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

Lazy load ext/node source code on first use #19398

Closed
bartlomieju opened this issue Jun 7, 2023 · 3 comments
Closed

Lazy load ext/node source code on first use #19398

bartlomieju opened this issue Jun 7, 2023 · 3 comments
Assignees
Labels
deno_core Changes in "deno_core" crate are needed node compat perf performance related

Comments

@bartlomieju
Copy link
Member

Current we are snapshotting all internal code. One idea to improve the startup
time is to exclude "ext/node" code from the snapshot and instead load it on first
use.

The theory is that it would help us save about half of the size of the startup snapshot
and about half of the startup time.

This might require more involved changes in "core/extensions.rs" and the build
scripts.

@bartlomieju bartlomieju self-assigned this Jun 7, 2023
@bartlomieju bartlomieju added perf performance related deno_core Changes in "deno_core" crate are needed node compat labels Jun 7, 2023
mmastrac pushed a commit that referenced this issue Jun 13, 2023
Remove `ExtensionFileSourceCode::LoadedFromFsDuringSnapshot` and feature
`include_js_for_snapshotting` since they leak paths that are only
applicable in this repo to embedders. Replace with feature
`exclude_js_sources`. Additionally the feature
`force_include_js_sources` allows negating it, if both features are set.
We need both of these because features are additive and there must be a
way of force including sources for snapshot creation while still having
the `exclude_js_sources` feature. `force_include_js_sources` is only set
for build deps, so sources are still excluded from the final binary.

You can also specify `force_include_js_sources` on any extension to
override the above features for that extension. Towards #19398.

But there was still the snapshot-from-snapshot situation where code
could be executed twice, I addressed that by making `mod_evaluate()` and
scripts like `core/01_core.js` behave idempotently. This allowed
unifying `ext::init_ops()` and `ext::init_ops_and_esm()` into
`ext::init()`.
bartlomieju pushed a commit that referenced this issue Jun 15, 2023
Remove `ExtensionFileSourceCode::LoadedFromFsDuringSnapshot` and feature
`include_js_for_snapshotting` since they leak paths that are only
applicable in this repo to embedders. Replace with feature
`exclude_js_sources`. Additionally the feature
`force_include_js_sources` allows negating it, if both features are set.
We need both of these because features are additive and there must be a
way of force including sources for snapshot creation while still having
the `exclude_js_sources` feature. `force_include_js_sources` is only set
for build deps, so sources are still excluded from the final binary.

You can also specify `force_include_js_sources` on any extension to
override the above features for that extension. Towards #19398.

But there was still the snapshot-from-snapshot situation where code
could be executed twice, I addressed that by making `mod_evaluate()` and
scripts like `core/01_core.js` behave idempotently. This allowed
unifying `ext::init_ops()` and `ext::init_ops_and_esm()` into
`ext::init()`.
@bartlomieju
Copy link
Member Author

I'm gonna close this one for now. The trade-off between complexity and potential gains is currently not worth it to spend time on it.

@petamoriken
Copy link
Contributor

In my understanding, WebGPU is temporarily disabled due to the lack of lazy load, are there any plans to bring it back?
#18094 (review)

@bartlomieju
Copy link
Member Author

In my understanding, WebGPU is temporarily disabled due to the lack of lazy load, are there any plans to bring it back? #18094 (review)

These are not the same problem - for WebGPU it's about lazy loading the Rust code (there's a startup cost related to linking system libraries on startup), while this issue was about lazy loading JavaScript code for ext/node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno_core Changes in "deno_core" crate are needed node compat perf performance related
Projects
None yet
Development

No branches or pull requests

2 participants