[v18.x] backport startup performance patches by joyeecheung · Pull Request #46425 · nodejs/node · GitHub

This is a backport of #45659 and #45849, the first two commits are specifically added for v18.x with the second one taken from #44483.

Refs: #46425

Review requested:

@nodejs/loaders
@nodejs/modules
@nodejs/net
@nodejs/startup

CI: https://ci.nodejs.org/job/node-test-pull-request/49253/

Nice! Looks good, there’s just one typo in the extra commit messages (gloablThis) 🙂

lgtm

CI: https://ci.nodejs.org/job/node-test-pull-request/49281/

CI: https://ci.nodejs.org/job/node-test-pull-request/49282/

CI: https://ci.nodejs.org/job/node-test-pull-request/49415/

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.

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.

@joyeecheung It looks like this needs a rebase

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

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 <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im>

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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <jgruber@chromium.org> Commit-Queue: 王澳 <wangao.james@bytedance.com> Cr-Commit-Position: refs/heads/main@{#82254} Refs: v8/v8@d69c793 PR-URL: #46425 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

PR-URL: nodejs/node#46425 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

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 <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

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 <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Tierney Cyren <hello@bnb.im>

PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

So that the file can be snapshotted. PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

So that the file can be snapshotted PR-URL: nodejs/node#45849 Backport-PR-URL: nodejs/node#46425 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>

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 <jgruber@chromium.org> Commit-Queue: 王澳 <wangao.james@bytedance.com> Cr-Commit-Position: refs/heads/main@{#82254} Refs: v8/v8@d69c793 PR-URL: nodejs/node#46425 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>