[v18.x] backport startup performance patches#46425

I cannot reproduce the snapshot test failures locally on similar platforms. It could be a flake, or maybe something wasn't backported properly for v18.x. Will need to wait until the CI is unlocked to do a bit more runs to find out.

juanarbol · 2023-02-23T23:54:16Z

I would love to land this in the v18.15.0 release <3, I was going to kick a CI for this but it needs a rebase first.

danielleadams · 2023-04-02T21:18:02Z

@joyeecheung It looks like this needs a rebase

krk · 2023-04-26T12:49:19Z

Hi! What is the expected performance gain in startup with this PR?

ruyadorno · 2023-08-16T15:38:12Z

hi @joyeecheung this PR needs rebasing in case we want to land it in v18.x

So that we can add multiple initialization scripts in the test runner. Refs: nodejs#44483

It turns out that even with startup snapshots, there is a non-trivial overhead for loading internal modules. This patch makes the loading of the non-essential modules lazy again. Caveat: we have to make some of the globals lazily-loaded too, so the WPT runner is updated to test what the state of the global scope is after the globals are accessed (and replaced with the loaded value). PR-URL: nodejs#45659 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Daeyeon Jeong <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Tierney Cyren <[email protected]>

This patch makes the top-level access to runtime states in the CJS loader lazy, and move the side-effects into a initializeCJS() function that gets called during pre-execution. As a result the CJS loader can be included into the built-in snapshot. PR-URL: #45849 Backport-PR-URL: #46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Preload essential modules and lazy-load non-essential ones. After this patch, all modules listed by running this snippet: ``` const list = process.moduleLoadList.join('\n'); require('fs').writeSync(1, list, 'utf-8'); ``` (which is roughly the same list as the one in test-bootstrap-module.js for the main thread) are loaded from the snapshot so no additional compilation cost is incurred. PR-URL: #45849 Backport-PR-URL: #46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Original commit message: [snapshot] Dont defer ByteArray when serializing JSTypedArray needs the base_pointer ByteArray immediately if it's on heap. JSTypedArray's base_pointer was initialized to Smi::uninitialized_deserialization_value at first when deserializing, and if base_pointer was deferred, we will mistakenly check JSTypedArray not on heap. Bug: v8:13149 Change-Id: I104c83ff9a2017de1c8071a9e116baa602f6977d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3813068 Reviewed-by: Jakob Linke <[email protected]> Commit-Queue: 王澳 <[email protected]> Cr-Commit-Position: refs/heads/main@{#82254} Refs: v8/v8@d69c793 PR-URL: #46425 Reviewed-By: Joyee Cheung <[email protected]>

PR-URL: nodejs/node#46425 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

So that we can add multiple initialization scripts in the test runner. Refs: nodejs/node#44483 PR-URL: nodejs/node#46425 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

It turns out that even with startup snapshots, there is a non-trivial overhead for loading internal modules. This patch makes the loading of the non-essential modules lazy again. Caveat: we have to make some of the globals lazily-loaded too, so the WPT runner is updated to test what the state of the global scope is after the globals are accessed (and replaced with the loaded value). PR-URL: nodejs/node#45659 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Daeyeon Jeong <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Tierney Cyren <[email protected]>

PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Since the module has to be loaded during bootstrap anyway. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

The helpers are actually shared by the two loaders, so move them under modules/ directly. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

This patch adds a getLazy() method to facilitate initialize-once lazy loading in the internals. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

So that the file can be snapshotted. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

So that the file can be snapshotted PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

This moves the following utils into modules/esm/utils.js: - Code related to default conditions - The callbackMap (which is now created in the module instead of hanging off the module_wrap binding, since the C++ land does not need it). - Per-isolate module callbacks These are self-contained code that can be included into the built-in snapshot. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

This patch makes the top-level access to runtime states in the CJS loader lazy, and move the side-effects into a initializeCJS() function that gets called during pre-execution. As a result the CJS loader can be included into the built-in snapshot. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Preload essential modules and lazy-load non-essential ones. After this patch, all modules listed by running this snippet: ``` const list = process.moduleLoadList.join('\n'); require('fs').writeSync(1, list, 'utf-8'); ``` (which is roughly the same list as the one in test-bootstrap-module.js for the main thread) are loaded from the snapshot so no additional compilation cost is incurred. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Original commit message: [snapshot] Dont defer ByteArray when serializing JSTypedArray needs the base_pointer ByteArray immediately if it's on heap. JSTypedArray's base_pointer was initialized to Smi::uninitialized_deserialization_value at first when deserializing, and if base_pointer was deferred, we will mistakenly check JSTypedArray not on heap. Bug: v8:13149 Change-Id: I104c83ff9a2017de1c8071a9e116baa602f6977d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3813068 Reviewed-by: Jakob Linke <[email protected]> Commit-Queue: 王澳 <[email protected]> Cr-Commit-Position: refs/heads/main@{#82254} Refs: v8/v8@d69c793 PR-URL: nodejs/node#46425 Reviewed-By: Joyee Cheung <[email protected]>

PR-URL: nodejs/node#46425 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

So that we can add multiple initialization scripts in the test runner. Refs: nodejs/node#44483 PR-URL: nodejs/node#46425 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>

It turns out that even with startup snapshots, there is a non-trivial overhead for loading internal modules. This patch makes the loading of the non-essential modules lazy again. Caveat: we have to make some of the globals lazily-loaded too, so the WPT runner is updated to test what the state of the global scope is after the globals are accessed (and replaced with the loaded value). PR-URL: nodejs/node#45659 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Daeyeon Jeong <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: Tierney Cyren <[email protected]>

PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Since the module has to be loaded during bootstrap anyway. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

The helpers are actually shared by the two loaders, so move them under modules/ directly. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

This patch adds a getLazy() method to facilitate initialize-once lazy loading in the internals. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

So that the file can be snapshotted. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

So that the file can be snapshotted PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

This moves the following utils into modules/esm/utils.js: - Code related to default conditions - The callbackMap (which is now created in the module instead of hanging off the module_wrap binding, since the C++ land does not need it). - Per-isolate module callbacks These are self-contained code that can be included into the built-in snapshot. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

This patch makes the top-level access to runtime states in the CJS loader lazy, and move the side-effects into a initializeCJS() function that gets called during pre-execution. As a result the CJS loader can be included into the built-in snapshot. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Preload essential modules and lazy-load non-essential ones. After this patch, all modules listed by running this snippet: ``` const list = process.moduleLoadList.join('\n'); require('fs').writeSync(1, list, 'utf-8'); ``` (which is roughly the same list as the one in test-bootstrap-module.js for the main thread) are loaded from the snapshot so no additional compilation cost is incurred. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Original commit message: [snapshot] Dont defer ByteArray when serializing JSTypedArray needs the base_pointer ByteArray immediately if it's on heap. JSTypedArray's base_pointer was initialized to Smi::uninitialized_deserialization_value at first when deserializing, and if base_pointer was deferred, we will mistakenly check JSTypedArray not on heap. Bug: v8:13149 Change-Id: I104c83ff9a2017de1c8071a9e116baa602f6977d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3813068 Reviewed-by: Jakob Linke <[email protected]> Commit-Queue: 王澳 <[email protected]> Cr-Commit-Position: refs/heads/main@{#82254} Refs: v8/v8@d69c793 PR-URL: nodejs/node#46425 Reviewed-By: Joyee Cheung <[email protected]>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. v18.x Issues that can be reproduced on v18.x or PRs targeting the v18.x-staging branch. labels Jan 30, 2023

addaleax approved these changes Jan 30, 2023
View reviewed changes

mcollina approved these changes Jan 30, 2023
View reviewed changes

anonrig approved these changes Jan 31, 2023
View reviewed changes

juanarbol force-pushed the v18.x-staging branch from 7351221 to fcfde34Compare January 31, 2023 13:17

panva force-pushed the bp-start-perf branch from d93d8ad to df6ce31Compare January 31, 2023 15:08

panva force-pushed the bp-start-perf branch from df6ce31 to b5fb3fbCompare January 31, 2023 15:17

panva added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 31, 2023

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 31, 2023

github-actionsbot mentioned this pull request Feb 7, 2023
CI Reliability 2023-02-07 nodejs/reliability#498
Open
18 tasks

juanarbol force-pushed the v18.x-staging branch from e17d857 to de64315Compare March 5, 2023 04:04

panva mentioned this pull request Apr 11, 2023
lib: fix DOMException property descriptors after being lazy loaded #46799
Merged

krk mentioned this pull request May 2, 2023
Release plan - v18.x Maintenance LTS nodejs/Release#737
Closed

krk mentioned this pull request May 10, 2023
Node.js Performance Team Meeting 2023-05-15 nodejs/performance#78
Closed

ruyadorno force-pushed the v18.x-staging branch from ef85828 to 490fc00Compare September 17, 2023 02:48

joyeecheung added 3 commits October 28, 2023 13:59

bootstrap: lazy-load Performance.prototype.timeOrigin
10f5974

test: use an array for WPT gloablThis initialization scripts
3076119
So that we can add multiple initialization scripts in the test runner. Refs: nodejs#44483

targos mentioned this pull request Nov 28, 2023
v18.19.0 release proposal #50953
Merged

Uh oh!

[v18.x] backport startup performance patches#46425

[v18.x] backport startup performance patches #46425

Uh oh!

Conversation

joyeecheung commented Jan 30, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jan 30, 2023

Uh oh!

nodejs-github-bot commented Jan 30, 2023

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jan 31, 2023

Uh oh!

nodejs-github-bot commented Jan 31, 2023

Uh oh!

nodejs-github-bot commented Feb 5, 2023

Uh oh!

joyeecheung commented Feb 9, 2023

Uh oh!

juanarbol commented Feb 23, 2023

Uh oh!

danielleadams commented Apr 2, 2023

Uh oh!

krk commented Apr 26, 2023

Uh oh!

ruyadorno commented Aug 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

joyeecheung commented Jan 30, 2023•
edited
Loading