src: stop copying code cache, part 2 #47958
Merged
Conversation
This file contains 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
|
Review requested:
|
nodejs-github-bot
added
c++
This removes more copies of the code cache data. First: for the builtin snapshot, we were copying the code cache to create a `std::vector<uint8_t>`. This was slowing down static intialization. Change it to use a good old `uint8_t*` and `size_t` rather than a vector. For the case of embedder provided snapshots, we also add an `owning_ptr` so that we can properly cleanup owned values created from the snapshot. Second: whenever the code cache was hit, we would remove the bytecode from the code cache, and then reserialize it from the compiled function. This was pretty slow. Change the code so that we can reuse the same code cache multiple times. If the code cache is rejected (say, because the user added V8 options), then we need to generate the bytecode, in which case we again use `owning_ptr` to ensure that the underlying code cache is freed. Combined, these changes improve the misc/startup.js benchmarks significantly (p < 0.001): * process,benchmark/fixtures/require-builtins: 22.15% * process,test/fixtures/semicolon: 8.55% * worker,benchmark/fixtures/require-builtins: 26.52% * worker,test/fixtures/semicolon: 21.52%
kvakil
force-pushed
the
code-cache-nonowns
branch
from
ab7ac94
to
b43e828
Compare
github-actions
bot
removed
the
request-ci
joyeecheung
approved these changes
May 12, 2023
There was a problem hiding this comment.
Nice find, I think the generation logic came from a time when we didn't generate the code cache at build time (we always generated the code cache so that workers can reuse the cache compiled by the main thread), but it's obsolete now that we pre-generate the code cache at build time..
kvakil
added
the
author ready
JungMinu
approved these changes
May 15, 2023
kvakil
added
commit-queue-squash
nodejs-github-bot
removed
the
commit-queue
|
Landed in 3ef17b6 |
targos
pushed a commit
that referenced
this pull request
May 15, 2023
This removes more copies of the code cache data. First: for the builtin snapshot, we were copying the code cache to create a `std::vector<uint8_t>`. This was slowing down static intialization. Change it to use a good old `uint8_t*` and `size_t` rather than a vector. For the case of embedder provided snapshots, we also add an `owning_ptr` so that we can properly cleanup owned values created from the snapshot. Second: whenever the code cache was hit, we would remove the bytecode from the code cache, and then reserialize it from the compiled function. This was pretty slow. Change the code so that we can reuse the same code cache multiple times. If the code cache is rejected (say, because the user added V8 options), then we need to generate the bytecode, in which case we again use `owning_ptr` to ensure that the underlying code cache is freed. Combined, these changes improve the misc/startup.js benchmarks significantly (p < 0.001): * process,benchmark/fixtures/require-builtins: 22.15% * process,test/fixtures/semicolon: 8.55% * worker,benchmark/fixtures/require-builtins: 26.52% * worker,test/fixtures/semicolon: 21.52% PR-URL: #47958 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams
added
the
backport-requested-v18.x
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.
This removes more copies of the code cache data.
First: for the builtin snapshot, we were copying the code cache to create a
std::vector<uint8_t>. This was slowing down static intialization. Change it to use a good olduint8_t*andsize_trather than a vector. For the case of embedder provided snapshots, we also add anowning_ptrso that we can properly cleanup owned values created from the snapshot.Second: whenever the code cache was hit, we would remove the bytecode from the code cache, and then reserialize it from the compiled function. This was pretty slow. Change the code so that we can reuse the same code cache multiple times. If the code cache is rejected (say, because the user added V8 options), then we need to generate the bytecode, in which case we again use
owning_ptrto ensure that the underlying code cache is freed.Combined, these changes improve the misc/startup.js benchmarks significantly (p < 0.001):