Repository navigation
Read exported global mutability from the side module binary - #28006
rustyconover wants to merge 1 commit into
Conversation
dba1a61 to
66bc16e
Compare
relocateExports used isImmutableGlobal on every exported global, which detects an immutable global by trying to write it and catching the exception. Side modules export each data symbol as an immutable global, so loading one throws and catches an exception per data export (over a thousand for some real libraries). getDylinkMetadata now also reads the import, global and export sections when it has the module bytes, and records which exported globals are mutable. relocateExports uses that instead of probing. When only a compiled WebAssembly.Module is available, or the sections contain something the parser doesn't handle, it falls back to isImmutableGlobal. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
66bc16e to
005a883
Compare
sbc100
left a comment
sbc100
left a comment
There was a problem hiding this comment.
The fact that we need to do this type of thing is probably a good argument for a some kind of new/improve type-reflection proposal. https://github.com/WebAssembly/js-types is currently not making much progress. @stevenfontanella might be interesting in this.
I'm a little wary of adding this extra JS code. Maybe we could put it behind the SHRINK_LEVEL setting since its trading codesize for perf? Maybe its doesn't matter since dynamic linking is already fairly heavy weight?
| $relocateExports__deps: ['$isImmutableGlobal'], | ||
| $relocateExports: (exports, memoryBase = 0) => { | ||
| $relocateExports__docs: '/** @param {Set<string>=} mutableGlobals */', | ||
| $relocateExports: (exports, memoryBase = 0, mutableGlobals) => { |
There was a problem hiding this comment.
Having non-default argument after the default ones looks odd to me. Can we make mutableGlobals the second argument?
| // which are relative to `memoryBase`. `mutableGlobals`, when known, | ||
| // saves probing each global. | ||
| var immutable = mutableGlobals | ||
| ? value instanceof WebAssembly.Global && !mutableGlobals.has(name) |
There was a problem hiding this comment.
Do we need value instanceof WebAssembly.Global here?
| // This saves relocateExports from probing each exported global with | ||
| // isImmutableGlobal, which throws for every immutable one. Returns | ||
| // undefined if it finds something it doesn't handle. | ||
| function getMutableGlobals() { |
There was a problem hiding this comment.
This function seems long enough that we should perhaps split it out into its own library function?
Also since it itself include more sub-functions.
But I guess since the subfunctions also close over other helper like getU8 we might need a bigger refactor here?
This branch has not been deployed
2 participants
dlopengets slower with every data symbol a side module exports.relocateExportspasses each exported global toisImmutableGlobal, which tests for an immutable global by writing to it and catching the exception. Side modules export every data symbol as an immutable global, so loading one throws and catches an exception for each data export. This PR reads mutability from the module bytes instead, so nothing is thrown.Benchmark
A side module with 2,000 exported data symbols (
int d0 = 0; ... int d1999 = 1999;), built at-O2and loaded withdlopenfrom aMAIN_MODULE=2program. Each build was timed in 15 fresh Node processes, interleaved, on aarch64 (Graviton) EC2. Medians, IQR in brackets:maindlopenThat's about 3.5 µs per exported global in Node. A throw costs more inside a browser worker that is running deep wasm stacks: 25–45 µs per global in Chrome. In DuckDB-wasm in Chrome, removing the probe cut 30–60 ms from loading an extension that has about 1,300 data exports.
Change
getDylinkMetadataalready parses the start of the binary. When it has the bytes, it now continues through the import, global and export sections and records which exported globals are mutable.relocateExportsuses that set instead of probing. The parse takes 0.1–3 ms even for modules with several thousand imports and exports.It falls back to
isImmutableGlobalin two cases:WebAssembly.Moduleis available, with no bytes. This happens for preloaded modules, and for workers started after thedlopen, which get the module fromsharedModules. Threads that are already running when thedlopenhappens load from the bytes thatdynlink.ckeeps infile_data, so they take the fast path too.Assuming that every side-module export is immutable would be smaller, but #25530 made exported mutable globals a supported case.
Size
codesize.test_codesize_hello_dylinkcodesize.test_codesize_hello_dylink_allBuilds without dynamic linking don't change.
Testing
New test:
other.test_dlopen_exported_globals. It covers a side module that exports both a data symbol and a mutable global. It checks two things. The data symbol must be relocated, so thatdlsymmatches the address the side module sees. The mutable global must be passed through, so that a write from JS is visible to the wasm code. It also checks thatgetDylinkMetadatareturns exactlyside_globalas mutable, so the test fails if the parser silently falls back to probing. I checked that it fails in that case, and also when the parser reports no globals as mutable or all of them as mutable.The dylink, dlfcn, dlopen, dlsym, main_module and side_module selectors pass across
core0,core2,wasm64,core_2gbandother: 751 tests, the same set asmain, with 33 skipped on both.🤖 Generated with Claude Code