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

lib: lazy load v8 in error-serdes #26689

Closed
wants to merge 1 commit into from
Closed

Conversation

richardlau
Copy link
Member

@richardlau richardlau commented Mar 15, 2019

Lazy loading v8 in lib/internal/error-serdes.js reduces the number
of loaded modules by the bootstrap code for Worker threads by seven.

Refs: #26501 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@richardlau

This comment has been minimized.

Lazy loading `v8` in `lib/internal/error-serdes.js` reduces the number
of loaded modules by the bootstrap code for Worker threads by seven.

Refs: nodejs#26501 (comment)
@richardlau
Copy link
Member Author

@addaleax
Copy link
Member

The code LGTM, but it might be worth pointing out that once we have snapshot support, we want as many modules loaded during bootstrap as possible, as opposed to as few as possible.

@richardlau
Copy link
Member Author

The code LGTM, but it might be worth pointing out that once we have snapshot support, we want as many modules loaded during bootstrap as possible, as opposed to as few as possible.

I have no objections to closing this if it's not worth it.

@BridgeAR
Copy link
Member

@addaleax I would like to experiment with the snapshots as soon as we fully implement them in a way to see what's best. If I am not mistaken we could still have a code cache for the lazy loaded modules which could be a good middle ground for all users.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 15, 2019
@richardlau
Copy link
Member Author

richardlau commented Mar 16, 2019

@danbev
Copy link
Contributor

danbev commented Mar 19, 2019

Landed in 927f29d.

@danbev danbev closed this Mar 19, 2019
pull bot pushed a commit to SimenB/node that referenced this pull request Mar 19, 2019
Lazy loading `v8` in `lib/internal/error-serdes.js` reduces the number
of loaded modules by the bootstrap code for Worker threads by seven.

PR-URL: nodejs#26689
Refs: nodejs#26501 (comment)
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
Lazy loading `v8` in `lib/internal/error-serdes.js` reduces the number
of loaded modules by the bootstrap code for Worker threads by seven.

PR-URL: #26689
Refs: #26501 (comment)
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants