Repository navigation
Don't walk the main module's table in pthread workers - #28105
Open
clementperon wants to merge 2 commits into
Open
clementperon wants to merge 2 commits into
clementperon wants to merge 2 commits into
Conversation
The first call to getFunctionAddress walks the whole table, which creates a JS function object for every entry. With MAIN_MODULE and pthreads, every worker does this when it instantiates the main module, and keeps those objects for its lifetime: about 8 MiB per worker for a 50k-entry table. The dynamic linker only looks up the main module's exported functions, so the main thread now records their table slots and passes them to workers, which add just those to the map. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request optimizes worker initialization in pthreads builds with MAIN_MODULE by passing only the table slots of the main module's exported functions to workers, rather than having them walk the entire function table. Feedback suggests adding a defensive check in src/lib/libaddfunction.js to ensure getWasmTableEntry(slot) returns a valid object before adding it to the WeakMap, preventing potential runtime crashes in workers.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first call to
getFunctionAddresswalks the whole table and keeps a JS function object for every entry. WithMAIN_MODULEand pthreads, every worker makes that call when it instantiates the main module (updateGOT), so each worker holds a wrapper for every address-taken function of the main module for its whole lifetime.The dynamic linker only looks up the main module's exported functions, so the main thread now records their table slots, before
updateGOT, and passes them to workers in theloadmessage next tosharedModules. A worker adds just those slots to its map, plus anything beyond the main module's initial table (side modules, lateraddFunctionslots).Impact
Kodi's WebAssembly port links Kodi as
MAIN_MODULE=2so binary add-ons can bedlopened. Its table has 50,551 slots, of which 102 hold exported functions. Measured in Chrome with the same wasm, applying this change to the Emscripten 6.0.9 glue:Behavior
Lookups in workers return the same slots as before:
addFunctionwrapped is still found through the entryaddFunctionrecords.The difference is a non-exported main-module function passed to
addFunctionin a worker. That function is only reachable from JS through the table itself. Before, the walk found its existing slot; now it would get a new slot. WithASSERTIONS=2,addFunctionalready checks for this case ("function in Table but not functionsInTableMap").The main thread still walks the table once. #27993 creates the main module's GOT entries lazily and doesn't change the walk, so the two changes are independent.
Testing
test_pthread_dylinknow asserts that a side module sees the same&bar_mainas the main module, on the main thread and in a worker. That assertion fails if workers don't add the recorded slots.Passing: all
test_pthread_dl*,test_pthread_dylink*andtest_pthread_tls_dylinktests incore0,core2andwasm64,other.test_dylink_pthread*, and the dylink codesize tests (unchanged). Thecore0tests also pass with-sASSERTIONS=2.🤖 Generated with Claude Code