build: create V8 code cache after script is run #21567
Conversation
|
From #21563
@hashseed from the logs (posted in the OP) it does not look like the size changes much. Some of them changed about 1-2KB, but most of them are unchanged. I excluded the scripts under |
|
Seems like most scripts run during bootstrapping are straight-line code that only set up things and do not often call inner functions, which is why not a lot of lazily compiled functions get included, and therefore there is no benefit. |
|
How come we don't see acorn being cached in the log for after? |
| (key) => NativeModule.isDepsModule(key) || key.startsWith('internal/deps') | ||
| ); | ||
|
|
||
| // Modules with source code compiled in js2c that |
There was a problem hiding this comment.
I'm not sure I understand this comment. Why can't these scripts be cached?
There was a problem hiding this comment.
configis a JSON string, it's not going to be wrappedsysis deprecated and is redirected fromutil(executing it would trigger a deprecation warning)internal/test/bindingtriggers a warning as well (also, it's not supposed to be loaded by users)internal/v8_prof_polyfillis not supposed to be required directly (see the comments there)internal/v8_prof_processoris an actual script (instead of a module) that is supposed to write a log file
The files in v8/tools are not really Node.js modules. They are mostly concatenated and run by internal/v8_prof_processor since they expose stuff to the global space and rely on that.
There was a problem hiding this comment.
I see. Thanks for the explanation.
I've removed the |
lib/internal/bootstrap/cache.js
Outdated
| if (cached && (cached.loaded || cached.loading)) { | ||
| const script = cached.script; | ||
| return script.createCachedData(); | ||
| } |
There was a problem hiding this comment.
^ unneeded var assignment return cached.script.createCachedData()
This patch makes it possible to generate the code cache for the builtins directly from the original script object (instead of compiling a new one) and after the script has been run (via `NativeModule.require`). Before this patch only the top level functions (the wrapped ones) are included in the cache, after this patch the inner functions in those modules will be included as well. Also blacklists modules from dependencies like V8 and node-inspect since we cannot guarantee that they are suitable to be executed directly.
|
Rebased and addressed review from @jdalton. I think this is worth landing even though it does not affect the performance since it cleans up the code a bit and we no longer need to recompile the source. |
|
Another CI is green: https://ci.nodejs.org/job/node-test-pull-request/16001/ |
|
Landed in 186c2fb, thanks! |
This patch makes it possible to generate the code cache for the builtins directly from the original script object (instead of compiling a new one) and after the script has been run (via `NativeModule.require`). Before this patch only the top level functions (the wrapped ones) are included in the cache, after this patch the inner functions in those modules will be included as well. Also blacklists modules from dependencies like V8 and node-inspect since we cannot guarantee that they are suitable to be executed directly. PR-URL: #21567 Refs: #21563 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This patch makes it possible to generate the code cache for the builtins directly from the original script object (instead of compiling a new one) and after the script has been run (via `NativeModule.require`). Before this patch only the top level functions (the wrapped ones) are included in the cache, after this patch the inner functions in those modules will be included as well. Also blacklists modules from dependencies like V8 and node-inspect since we cannot guarantee that they are suitable to be executed directly. PR-URL: #21567 Refs: #21563 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This patch makes it possible to generate the code cache
for the builtins directly from the original script object
(instead of compiling a new one) and after the script has
been run (via
NativeModule.require). Before this patchonly the top level functions (the wrapped ones)
are included in the cache, after this patch the inner
functions in those modules will be included as well.
Also blacklists modules from dependencies like V8 and
node-inspect since we cannot guarantee that they are suitable
to be executed directly.
Refs: #21563
Performance stats
Ideally this should improve the startup time now that we include more into the cache,
but here is what I get now:
Before this patch, the logs of
tools/generate_code_cache.jsSee logs
After:
See logs
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes