Repository navigation
fix(cli): relocate data into split modules in optimized builds - #60
Conversation
|
Note that when I wrote this code I did not (and still do not now) fully understand how llvm intends to use the data segments. My current model is that each data segment corresponds to a section in elf, but I didn't investigate a clear mapping. Somewhere in the wasm is a memory address that serves as "start of heap" and "start of stack". There are also instructions that take the immediate index of a data segment as argument, but these do for some reason not get relocations. I don't want to scan the whole wasm for these instructions, and llvm might not even emit them for any Rust code. My conclusion was not to move, or add, additional segments to the wasm so that indices do not change. Instead, each split module gets a slice of the original address space of each segment to initialize, without overlaps so that loading a split module doesn't reinitialize read/write memory which might have been modified. I will review the code in the coming days, especially to consider if easing the restriction to append extra segments needs a few more safeguards. |
There was a problem hiding this comment.
Good approach, and takes care to keep existing data segment indices, thanks for that. Would hold off with merging until #59 is done, since there will be one (easily merged but still) conflicts and I think this order is easier.
Adding additional data segments at the end is a nice idea that I missed!
1815ff1 to
c475495
Compare
|
Addressed all comments, one commit each. Will wait for #59 and rebase once it lands. One extra commit, c475495, for a gap in the overlap safeguard: a segment with a non-constant base address had an unknown extent and was skipped by the overlap check, so a neighbouring constant-address segment could still be fragmented and its appended fragments could initialize after it. Such a segment now counts as overlapping every constant-address segment, and they all stay in main. Regression test included. On your note about segment indices: every input segment keeps its slot, passive segments are copied unchanged, and only extra fragments are appended, so |
wasm-ld deduplicates and tail-merges strings when optimizing, so several data symbols can share bytes. These symbols do not necessarily end up in the same output module. The data layout was computed per module, so the shared bytes were copied once per module, the segment grew past its input length and the whole segment fell back into the main module. In release builds this kept practically all `.rodata` in the main wasm and nothing was relocated into split modules (leptos-rs/cargo-leptos#679). Compute the layout over the symbols of all modules at once: sort them by input position, merge overlapping symbols into one range, and emit a range that is needed by more than one module from the main module, which is always loaded first. A merged range keeps the strictest alignment of its symbols and is placed congruent to its input start, so every symbol in it stays aligned. Pad each module's part of a segment only to the alignment its ranges need instead of the segment's alignment; with many output modules that padding alone overflowed the segment. Add end-to-end tests built from hand-encoded linking and relocation sections: a tail-merged symbol shared between the main and a split module, which fails with the previous layout, and an aligned word overlapped by a longer string.
A data symbol contained in another symbol never became a candidate target for relocations, so a relocation inside it was attributed to its container. A module that only used the inner symbol, typically the main module using the tail of a split's symbol, then missed the relocation's target and could read it before the split was loaded. The whole-segment fallback used to hide this by copying the entire segment into main. Keep the inner symbol as a candidate so relocations go to the smallest symbol containing them, as the containment dependency already assumes.
Bytes needed by more than one output module were always emitted from the main module. When none of those modules is main, a chunk that every requiring split loads is a tighter place: it keeps the bytes out of the initial download. Pick the chunk shared by exactly the requiring splits, or else the smallest chunk loaded by all of them, and only fall back to main when no such chunk exists. Track the modules that actually need a range, separately from the module chosen to emit it. Choosing from the emitting module instead would carry the extra splits of a superset chunk into later requirements and could send a range to main although a chunk for its final requirement exists. Add end-to-end tests for two splits sharing a tail-merged string, and for a range whose requirement grows through three contained symbols.
wasm-ld's --gc-sections leaves little unreferenced data, so a relocated segment has almost no slack for the padding between the parts of the output modules. When it overflowed, the whole segment was copied into the main module unrelocated, which again kept all data out of the split modules. Order the parts by decreasing alignment so padding is only needed after a part that mixes alignments, and when the segment still overflows, move the data of the smallest non-main module into the main module and retry. Merged ranges can need more padding than the input had, so the copy of the whole segment remains as the last resort. Add an end-to-end test where padding overflows the segment by one byte and only the smaller split's data is folded into main.
Each output module got one contiguous part of every input data segment. A part mixing aligned tables with strings forced padding before the next part, and with many output modules that padding exceeded the slack left by --gc-sections, so data had to be folded into the main module. Place the ranges of all modules by decreasing alignment instead. A range's length is a multiple of its alignment, so ordinary ranges pack with no padding at all; only merged ranges, which keep their input residue, can still need some. Each module then emits one data segment per run of its ranges, in practice one per alignment its data uses. The input's data segments keep their indices in every module and hold the module's first fragment, further fragments are appended and the data count and names follow. Indices in the code, passive segments and the names subsection therefore stay valid without any guard. Appending only preserves the initialization order if the input segments do not overlap in memory, which wasm-ld guarantees; segments of a hand-made input that do overlap are kept as they are, in their slots. Folds that were still needed are now reported at info level, once per segment, so that users notice when lazy data is moved into main.
Folding a module's data into the main module changes what ends up in main and is worth acting on, the same as the overlong segment fallback that follows it. Report it at the same level.
…wners A range's emitting module is a function of the modules that need it. Computing it on every merge scanned the output modules once per shared symbol and let a range migrate several times. Collect the owners first, then place every range in one pass.
Keep the how out of the changelog; it is not actionable when upgrading. The pull request carries the details.
A merged range can start at a less aligned offset than a symbol inside it, so it is placed congruent to its input start modulo its alignment rather than at the next multiple. Add a unit test for the placement.
…nown Extra data segments are appended after the input's segments, which only preserves initialization order when the segments do not overlap in memory. A segment whose base address is not a constant has an unknown extent, so a segment that is fragmented next to it could initialize after it and overwrite its bytes. Treat such a segment as overlapping every constant-address segment and keep them all in the main module.
c475495 to
39f435e
Compare
|
I just rebased. |
|
I'm satisfied with the implementation part, just trying to thoroughly get through the test file, which is a bit dense to read. Trying to get some debugging going to visualize the data ranges, too. Nothing to do from your side at this point, just wanted to give a heads-up 👍 |
WorldSEnder
left a comment
There was a problem hiding this comment.
Got through it. Before any code changes, please answer the questions in the review comments.
In the best world, the tests would somehow differentiate between properties that the current approach takes by implementation detail and which properties we absolutely expect. For example, a lot of the tests for specific memory contents might be overly strict.
| assert_eq!( | ||
| single_data_segment(&output.main), | ||
| (SEGMENT_BASE, SHARED.to_vec()), | ||
| "main module should contain exactly the shared bytes at the segment base", |
There was a problem hiding this comment.
This assertion is a bit strict. I could imagine a future version that breaks this by initializing the prefix in the split module, so that
- main contains
b"bytes\0"atSEGMENT_BASE + SHARED_TAIL_OFFSET - the split contains
b"shared-"atSEGMENT_BASEandb"split-only\0"atSEGMENT_BASE + SHARED.len()
There was a problem hiding this comment.
Agreed, it pins which module emits the shared bytes, which is a choice, not a requirement. The property is that every module reads correct bytes at the addresses it references and no module's segments overlap another's. I'd rewrite the layout assertions in this file around that once the questions below are settled.
| let (split_only_addr, split_only_bytes) = single_data_segment(split_module); | ||
| assert_eq!(split_only_bytes, SPLIT_ONLY); | ||
| assert!( | ||
| split_only_addr >= SEGMENT_BASE + SHARED.len() as u32, |
There was a problem hiding this comment.
Not quite clear to me that the split_only_addr is not allowed to come before the shared data in memory, these two should be non-overlapping and not dependent on one-another.
There was a problem hiding this comment.
Right, only non-overlap is required. Order in the segment is an implementation choice.
Two of them change code: capping contained symbols by their container's alignment, which simplifies |
|
It seems we are aligned on the layout issues. We have two cases of overlap
Hopefully that reasoning makes sense to you. |
|
It does make sense and I checked it against a real build (a leptos frontend, 33k data symbols in one One thing does not work: the STRINGS flag. wasm-ld merges Here is the plan, unless you say otherwise:
|
|
Plan sounds good.
Ah yeah I at least assumed that some llvm versions omit this flag, glad to see it confirmed that we can't trust it to exist. Nevermind that flag then. |
A symbol contained in another one is a struct field or a merged string suffix, so it cannot be aligned stricter than its container and no longer raises the range's alignment. Only a partially overlapping symbol does, and the range start must remain a multiple of the result; otherwise the segment is kept whole in the main module. Linker output never produces such overlaps, so they are reported. Every range now starts at a multiple of its alignment and is placed at the next such multiple, which needs no padding, so the fold of modules into main is removed and the whole-segment fallback remains as the safety net.
A relocation that starts in one symbol and ends in another is not something the linker produces. Report it as an invalid input instead of attributing it to the containing symbol. Inner symbols stay candidates so that a relocation inside them is attributed to them.
Assert what a layout must satisfy instead of where the current layout puts things: every function reads the input's bytes at the addresses it refers to once its module and the modules loaded with it are initialized, the segments of modules loaded together do not overlap, every segment stays inside its input segment, and shared bytes are emitted once. Exact addresses remain only where the layout itself is under test.
WorldSEnder
left a comment
There was a problem hiding this comment.
Thanks a lot.
Will probably still change things for overlapping ranges but do so outside the PR with some more experience and debug output setup.
In release builds practically all
.rodatastayed in the main wasm and nothing wasrelocated into split modules (leptos-rs/cargo-leptos#679).
wasm-ld deduplicates and tail-merges strings when optimizing, so several data
symbols share bytes, and those symbols do not always land in the same output
module. The layout was computed per module, so shared bytes were copied once per
module, the segment grew past its input and the whole segment fell back into main.
Changes, one per commit:
into one range, keeping every symbol's alignment. Bytes shared with main are
emitted from main.
using only an inner symbol used to miss the relocation's target; the old
fallback hid this.
module's data into main and retry instead of giving up on the whole segment.
module emits one data segment per run of its ranges; input segments keep their
indices, extra segments are appended, data count and names follow.
Measured on the leptos
lazy_routesexample (release): main's data sectionshrinks from 74,106 to 69,547 bytes, every split and chunk now carries its own
data, shipped gzip sizes are within 0.2% of before, and no
--no-merge-data-segmentslinker flag is needed.