Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add addAtPreRun function for code injection at preRun. #23451

Closed
wants to merge 3 commits into from

Conversation

brendandahl
Copy link
Collaborator

This is a first step in trying to unify more of the code used by minimal runtime and the normal runtime and also move all addOn* functions into a library. Similar to the other addAt* functions for injecting code, this adds addAtPreRun. addAtPreRun is used to have code that runs after the Wasm is loaded, but before ctors.

The new injection point allows us to move FS.init into atPreRun and also inline the call to __wasm_call_ctors at the same place in minimal runtime and the normal runtime.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. I assume there is no impact on code size?

@brendandahl
Copy link
Collaborator Author

Makes sense to me. I assume there is no impact on code size?

There should be a little improvement in some cases with this patch. Closure will be able to get rid of addOnInit since it won't always be used by default with normal runtime now.

@brendandahl
Copy link
Collaborator Author

One interesting test failure test_fs_no_main. It looks like this pr moved FS.init to atInit and added a test to ensure FS.init is not run before preRun (which is no longer the case).

@kripken
Copy link
Member

kripken commented Jan 17, 2025

Hmm, that order may be necessary. It is possible to keep the order unchanged?

This is a first step in trying to unify more of the code used by minimal
runtime and the normal runtime and also move all addOn* functions into
a library. Similar to the other addAt* functions for injecting code, this
adds `addAtPreRun`. `addAtPreRun` is used to have code that runs after the
Wasm is loaded, but before ctors.

The new injection point allows us to move `FS.init` into atPreRun and also
inline the call to `__wasm_call_ctors` at the same place in minimal runtime
and the normal runtime.
@brendandahl
Copy link
Collaborator Author

After discussing with Sam, I've swapped it so the addAtPreRun run after the addOnPreRuns. i.e. compile time code runs after runtime code. This seems to preserve the order we'll want.

export const ATPRERUN = [];

// Add code that will be run after the Wasm module is loaded, but before static
// constructors and main are run.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe clarify how it relates to addOnPreRun?

@brendandahl
Copy link
Collaborator Author

Closing this in favor of #23488.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants