wasmtime / PR #10285 Don't pad .text to page size for pulley · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #10285 Don't pad .text to page size for pulley

Wasmtime GitHub notifications bot (Feb 24 2025 at 21:16):

posborne requested pchickey for a review on PR #10285.

Wasmtime GitHub notifications bot (Feb 24 2025 at 21:16):

posborne opened PR #10285 from posborne:reduce-cwasm-padding-pulley to bytecodealliance:main:

When targeting pulley we aren't directly emitting executable code in the .text section and we don't have a good idea of the true target page size so we end up with ELF files that can have a significant amount of extra padding around the .text section with no benefit to the consumer.

The change here aligns with the already present section flag SH_WASMTIME_NOT_EXECUTED. Padding elimination for the .rodata.wasm section is already handled by the presence/absence of the memory-init-on-cow configuration.

For a basic wasip1 hello-world rust program with the combination of this patch and -O memory-init-cow=n I saw a reduction in the compiled wasm ELF from 421KB to 189KB with the sections no longer showing as being padded out significantly:
$ objdump --section-headers target/wasm32-wasip1/release/hello-wasm-world-0x00.cwasm

target/wasm32-wasip1/release/hello-wasm-world-0x00.cwasm:       file format elf64-littleriscv

Sections:
Idx Name              Size     VMA              Type
  0                   00000000 0000000000000000
  1 .wasmtime.engine  00000353 0000000000000000 DATA
  2 .wasmtime.bti     00000001 0000000000000000 DATA
  3 .text             00011bdc 0000000000000000
  4 .wasmtime.addrmap 00011c5c 0000000000000000 DATA
  5 .wasmtime.traps   00000ae0 0000000000000000 DATA
  6 .rodata.wasm      000019e8 0000000000000000 DATA
  7 .name.wasm        000027a6 0000000000000000 DATA
  8 .wasmtime.info    000010f9 0000000000000000 DATA
  9 .symtab           00001788 0000000000000000
 10 .strtab           000040f0 0000000000000000
 11 .shstrtab         00000089 0000000000000000
Addresses #10244, CC @ia0 @tschneidereit

Feedback very welcome as this is a bit of a starter issue for me within wasmtime. Things I considered but didn't end up going with with this patch:

Directly inlining the conditional logic for the text align in the two callsites.

Using the SH_WASMTIME_NOT_EXECUTED instead of looking at the triple again for the alignment decision.

Possibly looking to see if it made sense to reuse memory-init-on-cow or some other flagging for this decision rather than using the triple.

I'm also open to any feedback regarding if there is any additional testing that should be added in tree for this patch

Wasmtime GitHub notifications bot (Feb 24 2025 at 21:16):

posborne requested wasmtime-core-reviewers for a review on PR #10285.

Wasmtime GitHub notifications bot (Feb 24 2025 at 21:40):

alexcrichton submitted PR review:

This all looks great to me, thanks for working on this!

Using objdump --section-headers also makes me think that we should probably also provide a Config::* option for dropping the .symtab, .strtab, and .shstrtab sections. I don't think we have anything relying on that other than various debugging/perf utilities meaning it should be possible to strip them and still have a functioning *.cwasm binary.

I'll file an issue about this...

Last updated: Feb 28 2025 at 02:27 UTC