module: implement NODE_COMPILE_CACHE for automatic on-disk code caching #52535

joyeecheung · 2024-04-15T05:42:39Z

This is the initial PR for implementing the idea proposed in #47472. It takes inspiration from ccache, the v8-compile-cache npm package (but faster, supports ESM and replaces CJS loader moneky-patching in user land) and v8 code caching in Blink. The first commit is backported from https://chromium-review.googlesource.com/c/v8/v8/+/5401780 which has already landed in the upstream (this is required for import() to work in deserialized code). For more motivation and background of this PR, check out #47472

Local numbers:

❯ hyperfine "./out/Release/node test/fixtures/snapshot/typescript.js" --warmup 5
Benchmark 1: ./out/Release/node test/fixtures/snapshot/typescript.js
  Time (mean ± σ):     124.3 ms ±  13.2 ms    [User: 109.6 ms, System: 11.6 ms]
  Range (min … max):   116.1 ms … 170.2 ms    21 runs

❯ export NODE_COMPILE_CACHE=/tmp/.node_compile_cache
❯ hyperfine "./out/Release/node test/fixtures/snapshot/typescript.js" --warmup 5
Benchmark 1: ./out/Release/node test/fixtures/snapshot/typescript.js
  Time (mean ± σ):      72.3 ms ±   3.2 ms    [User: 62.3 ms, System: 10.2 ms]
  Range (min … max):    70.5 ms …  87.2 ms    40 runs

There are more numbers from folks trying on an earlier iteration of this work (although this branch now hashes the code and the cache as an extra check, but crc32 should be fast enough that it's not too big of an overhead). For example from #47472 (comment) tsc --version can go from ~90ms to ~40ms, yarn --version can go from ~190ms to ~135ms, from #47472 (comment) npm run echo with a ncc-bundled npm may go from ~150ms to ~110ms

summary

This patch implements automatic on-disk code caching that can be enabled
via an environment variable NODE_COMPILE_CACHE.

When set, whenever Node.js compiles a CommonJS or a ECMAScript Module,
it will use on-disk V8 code cache persisted in the specified directory
to speed up the compilation. This may slow down the first load of a
module graph, but subsequent loads of the same module graph may get
a significant speedup if the contents of the modules do not change.
Locally, this speeds up loading of test/fixtures/snapshot/typescript.js
from ~130ms to ~80ms.

To clean up the generated code cache, simply remove the directory.
It will be recreated the next time the same directory is used for
NODE_COMPILE_CACHE.

Compilation cache generated by one version of Node.js may not be used
by a different version of Node.js. Cache generated by different versions
of Node.js will be stored separately if the same directory is used
to persist the cache, so they can co-exist.

Caveat: currently when using this with V8 JavaScript code coverage, the
coverage being collected by V8 may be less precise in functions that are
deserialized from the code cache. It's recommended to turn this off when
running tests to generate precise coverage.

Implementation details:

There is one cache file per module on disk. The directory layout
is:

Compile cache directory (from NODE_COMPILE_CACHE)
- 8b23c8fe: CRC32 hash of CachedDataVersionTag + NODE_VERESION
- 2ea3424d:
  - 10860e5a: CRC32 hash of filename + module type
  - 431e9adc: ...
  - ...

Inside the cache file, there is a header followed by the actual
cache content:

[uint32_t] code size
[uint32_t] code hash
[uint32_t] cache size
[uint32_t] cache hash
... compile cache content ...

When reading the cache file, we'll also check if the code size
and code hash match the code that the module loader is loading
and whether the cache size and cache hash match the file content
read. If they don't match, or if V8 rejects the cache passed,
we'll ignore the mismatch cache, and regenerate the cache after
compilation succeeds and rewrite it to disk.

There are still a bunch of TODOs for this feature, some of them are documented as TODO comments. In addition we can look into idle-time code cache serialization/writing which is also done by Blink to reduce the impact on first load. But this should be good enough as an initial iteration.

Note: why CRC32? Because we already have it in the dependencies (in zlib.h), it's fast and can avoid enough collisions for this use case. We do have some existing hashing functions available via openssl but those won't be available on no-crypto builds. We can revisit if CRC32 turns out to be inadequate for this feature. We can also revisit the directory structure or use an actual db, but for now the file I/O cost doesn't really show up when I profile it locally. The remaining bottleneck is the repeated UTF8 encoding, which I plan to get rid of in a follow-up (because it needs to dance with monkey-patching).

Refs: #47472

Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7

This patch implements automatic on-disk code caching that can be enabled via an environment variable NODE_COMPILE_CACHE. When set, whenever Node.js compiles a CommonJS or a ECMAScript Module, it will use on-disk [V8 code cache][] persisted in the specified directory to speed up the compilation. This may slow down the first load of a module graph, but subsequent loads of the same module graph may get a significant speedup if the contents of the modules do not change. Locally, this speeds up loading of test/fixtures/snapshot/typescript.js from ~130ms to ~80ms. To clean up the generated code cache, simply remove the directory. It will be recreated the next time the same directory is used for `NODE_COMPILE_CACHE`. Compilation cache generated by one version of Node.js may not be used by a different version of Node.js. Cache generated by different versions of Node.js will be stored separately if the same directory is used to persist the cache, so they can co-exist. Caveat: currently when using this with V8 JavaScript code coverage, the coverage being collected by V8 may be less precise in functions that are deserialized from the code cache. It's recommended to turn this off when running tests to generate precise coverage. Implementation details: There is one cache file per module on disk. The directory layout is: - Compile cache directory (from NODE_COMPILE_CACHE) - 8b23c8fe: CRC32 hash of CachedDataVersionTag + NODE_VERESION - 2ea3424d: - 10860e5a: CRC32 hash of filename + module type - 431e9adc: ... - ... Inside the cache file, there is a header followed by the actual cache content: ``` [uint32_t] code size [uint32_t] code hash [uint32_t] cache size [uint32_t] cache hash ... compile cache content ... ``` When reading the cache file, we'll also check if the code size and code hash match the code that the module loader is loading and whether the cache size and cache hash match the file content read. If they don't match, or if V8 rejects the cache passed, we'll ignore the mismatch cache, and regenerate the cache after compilation succeeds and rewrite it to disk.

nodejs-github-bot · 2024-04-15T05:42:44Z

nodejs-github-bot · 2024-04-15T05:44:48Z

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

joyeecheung · 2024-04-15T05:54:59Z

doc/api/cli.md

+of Node.js will be stored separately if the same directory is used
+to persist the cache, so they can co-exist.
+
+Caveat: currently when using this with [V8 JavaScript code coverage][], the


When enabling NODE_COMPILE_CACHE and running tools/test.py parallel locally, three tests would fail:

Failed tests: out/Release/node /Users/joyee/projects/node/test/parallel/test-runner-coverage.js out/Release/node /Users/joyee/projects/node/test/parallel/test-runner-output.mjs out/Release/node /Users/joyee/projects/node/test/parallel/test-v8-coverage.js

The failed tests are all related to less precise coverage which is currently a known caveat of code cache + v8 native coverage collection (e.g. see #51672). So I just documented that a bit in the docs that users should not use the two together for now. I could spare some time later to fix it in V8, but I don't think it's a blocker for this PR because this feature is currently opt-in. User-land code caching can run into this problem too, and it's technically a V8 bug.

merceyz · 2024-04-15T09:18:59Z

There are more numbers from folks trying on an earlier iteration of this work

Some up-to-date results with f7806b3:

$ hyperfine -w 10 "NODE_COMPILE_CACHE='' ./node ./.yarn/releases/yarn-4.1.1.cjs --version" "NODE_COMPILE_CACHE='/tmp/node-cache' ./node ./.yarn/releases/yarn-4.1.1.cjs --version"
Benchmark 1: NODE_COMPILE_CACHE='' ./node ./.yarn/releases/yarn-4.1.1.cjs --version
  Time (mean ± σ):     165.5 ms ±   1.9 ms    [User: 156.5 ms, System: 25.4 ms]
  Range (min … max):   162.2 ms … 169.3 ms    18 runs

Benchmark 2: NODE_COMPILE_CACHE='/tmp/node-cache' ./node ./.yarn/releases/yarn-4.1.1.cjs --version
  Time (mean ± σ):     116.0 ms ±   1.3 ms    [User: 109.2 ms, System: 23.5 ms]
  Range (min … max):   113.8 ms … 119.7 ms    25 runs

Summary
  NODE_COMPILE_CACHE='/tmp/node-cache' ./node ./.yarn/releases/yarn-4.1.1.cjs --version ran
    1.43 ± 0.02 times faster than NODE_COMPILE_CACHE='' ./node ./.yarn/releases/yarn-4.1.1.cjs --version

src/compile_cache.cc

jdalton · 2024-04-15T13:31:49Z

This is so rad! Having had to manage and hook into systems for this having it built-in is very nice! ❤️ it.

src/compile_cache.cc

benjamingr · 2024-04-16T09:35:20Z

doc/api/cli.md

+
+> Stability: 1.1 - Active Development
+
+When set, whenever Node.js compiles a CommonJS or a ECMAScript Module,


This description is very technical - I would use more layman terms for the first paragraph.

Do you have specific suggestions? I don't want to make it sound too magical to avoid false advertisement.

i'll do a follow up PR after this lands to not hold it but ideally something that focuses more on what it does and not the technical implementation

benjamingr

LGTM as experimental/active development. Have a few questions:

how does this interact with SEA (would it make sense or be possible to only eventually bundle the code cache and not the code?)
What happens on overflows or tampering with the cache file? Is there a security concern here regarding passing different code through the cache file (since the hash is just a crc32)?
How does this feature interact with loaders/require hooks that modify the file contents and in particular passing them in different order? (reading the code it should work but want to make sure)

benjamingr · 2024-04-16T09:51:53Z

src/compile_cache.cc

+//     - <cache_file_1>: a hash of filename + module type
+//     - <cache_file_2>
+//     - <cache_file_3>
+bool CompileCacheHandler::InitializeDirectory(const std::string& dir) {


(Not blocking for this PR - we should have better user facing errors if there are permission issues creating/reading/writing the cache)

This should not error at all, the idea is that if caching cannot be done, this is just a no-op. There can be some logging to help users figure out why the cache isn't hit to ensure better performance, but the cache not being hit only leads to "not speeding up module loading", and it's not an error (just like when you write code that V8 cannot optimize or have to deopt with, there is no error either).

It's literally caching I get that but when it won't work for users and they'll want to figure out why they'll ask us. It would be useful to at least trace it as as. warning or add DEBUG* like logs.

There are already traces. The handful of tests here are testing against the traces .

joyeecheung · 2024-04-16T14:31:18Z

how does this interact with SEA (would it make sense or be possible to only eventually bundle the code cache and not the code?)

Currently SEA only supports bundling a single script and it's not compiled using the normal CJS loader, but there's already a dedicated option in the SEA config to enable code cache for that one file ("useCodeCache": true). Neither static import nor dynamic import is not currently supported in SEA. Users can load on-disk CJS files via createRequire() and in that case if the environment variable is enabled, the cache of on-disk modules loaded through the created require() will be put in that directory. This is all separated from the SEA itself.

What happens on overflows or tampering with the cache file? Is there a security concern here regarding passing different code through the cache file (since the hash is just a crc32)?

The cache will be silently ignored if the hash doesn't match. Under the Node.js threat model, code loaded from the file system is already trusted. If you can gain access to the file system, you might as well just replace the source file on disk that'll be executed and do whatever you want in that replaced file, instead of wasting all your time figuring out how to mutate the code cache that meets all the requirement below:

Can do what you need (this means you need to know how to hand-write V8 bytecode to do what you need. Also, it must be run with the right V8 version since the bytecode format changes all the time)
The mutated code cache is still of the same length of the original code cache (so if you want to do a lot of additional stuff in that bytecode, this is unlikely to pass)
It passes the additional checks done by V8, and V8 can actually run it just fine (and once the V8 version in Node.js is upgraded this is likely to stop working)
The CRC32 of this new code cache must also conflict with the original code cache (finding CRC32 conflicts isn't that hard, but finding CRC32 conflicts that meet 1-3 is hard)

TBH it seems a bit silly for an attacker to waste all their effort on this when they can already just replace the source code on disk if they have access to the file system. Also, this is already what people have been doing in the wild via e.g. https://www.npmjs.com/package/v8-compile-cache with 12M downloads/week, so if there are any security concerns, well it's already out there for many years, and it's unlikely that the ecosystem will just stop relying on a capability like this.

How does this feature interact with loaders/require hooks that modify the file contents and in particular passing them in different order? (reading the code it should work but want to make sure)

If they are using the "official" hooks, e.g. module.register the modules will be cached and the resource name & module content cached match the results coming out of the loaders. If it's monkey-patching CJS hooks this will be ignored if they patch Module.wrap, Module.wrapper or Module.prototype._compile (without calling the original version, this means it's a no-op if v8-compile-cache is used since that package does this), but otherwise it should work fine.

merceyz · 2024-04-16T15:38:27Z

Would it be feasible to add a way to enable this for the current process only, perhaps as a follow-up?

For context Corepack uses v8-compile-cache to speed up the loading of package managers but doesn't modify process.env to avoid affecting anything said package managers might spawn.

joyeecheung · 2024-04-16T15:58:53Z

Would it be feasible to add a way to enable this for the current process only, perhaps as a follow-up?

Yeah I think we can discuss other ways to enable this (or a possible solution to enable it by default and pointing it to somewhere that's less likely to fill up the disk e.g. the tmpdir) in a follow-up. For the initial PR an opt-in environment variable should be enough.

…e caching

nodejs-github-bot · 2024-04-17T02:45:44Z

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

github-actions · 2024-04-17T15:17:27Z

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @joyeecheung.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

Joyee: See the PR description

nodejs-github-bot · 2024-04-17T21:15:14Z

Commit Queue failed

- Loading data for nodejs/node/pull/52535 ✔ Done loading data for nodejs/node/pull/52535 ----------------------------------- PR info ------------------------------------ Title module: implement NODE_COMPILE_CACHE for automatic on-disk code caching (#52535) Author Joyee Cheung (@joyeecheung) Branch joyeecheung:node-compile-cache -> nodejs:main Labels semver-minor, notable-change, commit-queue-rebase Commits 3 - deps: V8: cherry-pick cd10ad7cdbe5 - module: implement NODE_COMPILE_CACHE for automatic on-disk code caching - fixup! module: implement NODE_COMPILE_CACHE for automatic on-disk cod… Committers 1 - Joyee Cheung PR-URL: https://github.com/nodejs/node/pull/52535 Refs: https://github.com/nodejs/node/issues/47472 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Yagiz Nizipli Reviewed-By: Mohammed Keyvanzadeh ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/52535 Refs: https://github.com/nodejs/node/issues/47472 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Yagiz Nizipli Reviewed-By: Mohammed Keyvanzadeh -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 15 Apr 2024 05:42:39 GMT ✔ Approvals: 3 ✔ - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/52535#pullrequestreview-2006838532 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/52535#pullrequestreview-2006412328 ✔ - Mohammed Keyvanzadeh (@VoltrexKeyva): https://github.com/nodejs/node/pull/52535#pullrequestreview-2006844212 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-04-17T02:45:44Z: https://ci.nodejs.org/job/node-test-pull-request/58449/ - Querying data for job/node-test-pull-request/58449/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 52535 From https://github.com/nodejs/node * branch refs/pull/52535/merge -> FETCH_HEAD ✔ Fetched commits as 3d4c5207d10b..b45062b1ba1c -------------------------------------------------------------------------------- [main 5eb35177af] deps: V8: cherry-pick cd10ad7cdbe5 Author: Joyee Cheung Date: Sun Apr 14 22:59:45 2024 +0200 7 files changed, 247 insertions(+), 27 deletions(-) Auto-merging doc/api/cli.md [main bfccaa6b24] module: implement NODE_COMPILE_CACHE for automatic on-disk code caching Author: Joyee Cheung Date: Thu Feb 29 02:09:16 2024 +0100 22 files changed, 962 insertions(+), 10 deletions(-) create mode 100644 src/compile_cache.cc create mode 100644 src/compile_cache.h create mode 100644 test/fixtures/es-modules/dynamic-import/import.cjs create mode 100644 test/fixtures/es-modules/dynamic-import/import.mjs create mode 100644 test/fixtures/es-modules/dynamic-import/node_modules/deps/mod.js create mode 100644 test/fixtures/es-modules/dynamic-import/node_modules/deps/package.json create mode 100644 test/parallel/test-compile-cache-dynamic-import.js create mode 100644 test/parallel/test-compile-cache-esm.js create mode 100644 test/parallel/test-compile-cache-existing-directory.js create mode 100644 test/parallel/test-compile-cache-success.js create mode 100644 test/parallel/test-compile-cache-updated-file.js [main 3fa6489ea9] fixup! module: implement NODE_COMPILE_CACHE for automatic on-disk code caching Author: Joyee Cheung Date: Wed Apr 17 00:36:45 2024 +0200 6 files changed, 264 insertions(+), 23 deletions(-) create mode 100644 test/parallel/test-compile-cache-bad-syntax.js create mode 100644 test/parallel/test-compile-cache-permission-allowed.js create mode 100644 test/parallel/test-compile-cache-permission-disallowed.js ✔ Patches applied There are 3 commits in the PR. Attempting autorebase. Rebasing (2/5)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
deps: V8: cherry-pick cd10ad7cdbe5

Original commit message:

[compiler] reset script details in functions deserialized from code cache

During the serialization of the code cache, V8 would wipe out the
host-defined options, so after a script id deserialized from the
code cache, the host-defined options need to be reset on the script
using what's provided by the embedder when doing the deserializing
compilation, otherwise the HostImportModuleDynamically callbacks
can't get the data it needs to implement dynamic import().

Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/main@{#93323}

Refs: v8/v8@`cd10ad7`
PR-URL: #52535
Refs: #47472
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io
Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com

[detached HEAD cacfdc620f] deps: V8: cherry-pick cd10ad7cdbe5
Author: Joyee Cheung joyeec9h3@gmail.com
Date: Sun Apr 14 22:59:45 2024 +0200
7 files changed, 247 insertions(+), 27 deletions(-)
Rebasing (3/5)
Rebasing (4/5)
Rebasing (5/5)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
module: implement NODE_COMPILE_CACHE for automatic on-disk code caching

This patch implements automatic on-disk code caching that can be enabled
via an environment variable NODE_COMPILE_CACHE.

When set, whenever Node.js compiles a CommonJS or a ECMAScript Module,
it will use on-disk [V8 code cache][] persisted in the specified directory
to speed up the compilation. This may slow down the first load of a
module graph, but subsequent loads of the same module graph may get
a significant speedup if the contents of the modules do not change.
Locally, this speeds up loading of test/fixtures/snapshot/typescript.js
from ~130ms to ~80ms.

To clean up the generated code cache, simply remove the directory.
It will be recreated the next time the same directory is used for
NODE_COMPILE_CACHE.

Compilation cache generated by one version of Node.js may not be used
by a different version of Node.js. Cache generated by different versions
of Node.js will be stored separately if the same directory is used
to persist the cache, so they can co-exist.

Caveat: currently when using this with V8 JavaScript code coverage, the
coverage being collected by V8 may be less precise in functions that are
deserialized from the code cache. It's recommended to turn this off when
running tests to generate precise coverage.

Implementation details:

There is one cache file per module on disk. The directory layout
is:

Compile cache directory (from NODE_COMPILE_CACHE)
- 8b23c8fe: CRC32 hash of CachedDataVersionTag + NODE_VERESION
- 2ea3424d:
  - 10860e5a: CRC32 hash of filename + module type
  - 431e9adc: ...
  - ...

Inside the cache file, there is a header followed by the actual
cache content:

[uint32_t] code size
[uint32_t] code hash
[uint32_t] cache size
[uint32_t] cache hash
... compile cache content ...

When reading the cache file, we'll also check if the code size
and code hash match the code that the module loader is loading
and whether the cache size and cache hash match the file content
read. If they don't match, or if V8 rejects the cache passed,
we'll ignore the mismatch cache, and regenerate the cache after
compilation succeeds and rewrite it to disk.

PR-URL: #52535
Refs: #47472
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io
Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com

[detached HEAD 870f6d953d] module: implement NODE_COMPILE_CACHE for automatic on-disk code caching
Author: Joyee Cheung joyeec9h3@gmail.com
Date: Thu Feb 29 02:09:16 2024 +0100
25 files changed, 1203 insertions(+), 10 deletions(-)
create mode 100644 src/compile_cache.cc
create mode 100644 src/compile_cache.h
create mode 100644 test/fixtures/es-modules/dynamic-import/import.cjs
create mode 100644 test/fixtures/es-modules/dynamic-import/import.mjs
create mode 100644 test/fixtures/es-modules/dynamic-import/node_modules/deps/mod.js
create mode 100644 test/fixtures/es-modules/dynamic-import/node_modules/deps/package.json
create mode 100644 test/parallel/test-compile-cache-bad-syntax.js
create mode 100644 test/parallel/test-compile-cache-dynamic-import.js
create mode 100644 test/parallel/test-compile-cache-esm.js
create mode 100644 test/parallel/test-compile-cache-existing-directory.js
create mode 100644 test/parallel/test-compile-cache-permission-allowed.js
create mode 100644 test/parallel/test-compile-cache-permission-disallowed.js
create mode 100644 test/parallel/test-compile-cache-success.js
create mode 100644 test/parallel/test-compile-cache-updated-file.js

Successfully rebased and updated refs/heads/main.

✔ cacfdc620fc1913538ebbe3c48ffab814dd60c04
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✔ 0:0 skipping fixes-url fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 19:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length
✖ 870f6d953dd934cb1fc5dce52ba5c84dbcebd2b5
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✔ 0:0 skipping fixes-url fixes-url
✔ 0:0 blank line after title line-after-title
✖ 5:72 Line should be <= 72 columns. line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 56:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
⚠ 0:50 Title should be <= 50 columns. title-length

ℹ Please fix the commit message and try again.
Please manually ammend the commit message, by running
git commit --amend
Once commit message is fixed, finish the landing command running
git node land --continue

https://github.com/nodejs/node/actions/runs/8728758377

Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

This patch implements automatic on-disk code caching that can be enabled via an environment variable NODE_COMPILE_CACHE. When set, whenever Node.js compiles a CommonJS or a ECMAScript Module, it will use on-disk [V8 code cache][] persisted in the specified directory to speed up the compilation. This may slow down the first load of a module graph, but subsequent loads of the same module graph may get a significant speedup if the contents of the modules do not change. Locally, this speeds up loading of test/fixtures/snapshot/typescript.js from ~130ms to ~80ms. To clean up the generated code cache, simply remove the directory. It will be recreated the next time the same directory is used for `NODE_COMPILE_CACHE`. Compilation cache generated by one version of Node.js may not be used by a different version of Node.js. Cache generated by different versions of Node.js will be stored separately if the same directory is used to persist the cache, so they can co-exist. Caveat: currently when using this with V8 JavaScript code coverage, the coverage being collected by V8 may be less precise in functions that are deserialized from the code cache. It's recommended to turn this off when running tests to generate precise coverage. Implementation details: There is one cache file per module on disk. The directory layout is: - Compile cache directory (from NODE_COMPILE_CACHE) - 8b23c8fe: CRC32 hash of CachedDataVersionTag + NODE_VERESION - 2ea3424d: - 10860e5a: CRC32 hash of filename + module type - 431e9adc: ... - ... Inside the cache file, there is a header followed by the actual cache content: ``` [uint32_t] code size [uint32_t] code hash [uint32_t] cache size [uint32_t] cache hash ... compile cache content ... ``` When reading the cache file, we'll also check if the code size and code hash match the code that the module loader is loading and whether the cache size and cache hash match the file content read. If they don't match, or if V8 rejects the cache passed, we'll ignore the mismatch cache, and regenerate the cache after compilation succeeds and rewrite it to disk. PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

joyeecheung · 2024-04-17T22:10:01Z

Landed in 3d4c520...87abd02 (with commit messages edited to wrap around 72 characters)

Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 PR-URL: nodejs#52535 Refs: nodejs#47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

lirantal · 2024-04-18T12:53:51Z

Good discussion and a cool performance improvement @joyeecheung, congrats!

Expressing some of my security concerns thoughts:

What happens on overflows or tampering with the cache file? Is there a security concern here regarding passing different code through the cache file (since the hash is just a crc32)?

The cache will be silently ignored if the hash doesn't match

It would be helpful for security audit trail to record in some way (warning, trace, etc) that a cache entry failed matching.

Other aspects worth considering, even if only through mentions in the documentation so that these risks are well communicated:

Would child processes also inherit the enabled cache mode?
It seems like this shouldn't conflict with experimental policies but raising just in case. For example, would the experimental policy throw an error for loaded cached code because there's no integrity specified/calculated for it?
What are the security controls guarding against a denial of service? for example, attackers create large code files that would cause more cached code files to be created. Since the threat model already trusts the OS I get this is not in regular scope but potentially be in effect if child processes are inheriting code cache and in between an attacker creates this scenario (still out of scope I guess?)
Where are the code cache files stored and how are they protected? If they are created in a predictable path and a vulnerability such as path traversal exists in the application, it could be used to exfiltrate the compiled code.

joyeecheung · 2024-04-18T14:57:22Z

It would be helpful for security audit trail to record in some way (warning, trace, etc) that a cache entry failed matching.

It can be traced using NODE_DEBUG_NATIVE=COMPILE_CACHE, see the tests

Would child processes also inherit the enabled cache mode?

Only if they inherit the environment variables (child process env options are defaulted to process.env, but once you specify it you need to copy process.env into the new options).

For example, would the experimental policy throw an error for loaded cached code because there's no integrity specified/calculated for it?

We can just skip it #52577 - the cache should be quiet by default, failures would just lead to no-ops.

What are the security controls guarding against a denial of service? for example, attackers create large code files that would cause more cached code files to be created.

If they already have access to the file system they might as well just modify the original module to run an infinite loop?

Where are the code cache files stored and how are they protected?

This is up to the user, it's currently opt-in via the environment variable NODE_COMPILE_CACHE, and the names of subdirectories change depending on the Node.js version and the filename and the module type. If an attacker can already get the filename of the module, they might as well just...modify the module?

lirantal · 2024-04-18T20:02:34Z

Thanks Joyee, appreciate all the input.

Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> PR-URL: #52293 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>

Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 PR-URL: nodejs#52535 Refs: nodejs#47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> PR-URL: #52293 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>

Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 PR-URL: nodejs#52535 Refs: nodejs#47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

Original commit message: [compiler] reset script details in functions deserialized from code cache During the serialization of the code cache, V8 would wipe out the host-defined options, so after a script id deserialized from the code cache, the host-defined options need to be reset on the script using what's provided by the embedder when doing the deserializing compilation, otherwise the HostImportModuleDynamically callbacks can't get the data it needs to implement dynamic import(). Change-Id: I33cc6a5e43b6469d3527242e083f7ae6d8ed0c6a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401780 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#93323} Refs: v8/v8@cd10ad7 PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> PR-URL: #52465 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>

This patch implements automatic on-disk code caching that can be enabled via an environment variable NODE_COMPILE_CACHE. When set, whenever Node.js compiles a CommonJS or a ECMAScript Module, it will use on-disk [V8 code cache][] persisted in the specified directory to speed up the compilation. This may slow down the first load of a module graph, but subsequent loads of the same module graph may get a significant speedup if the contents of the modules do not change. Locally, this speeds up loading of test/fixtures/snapshot/typescript.js from ~130ms to ~80ms. To clean up the generated code cache, simply remove the directory. It will be recreated the next time the same directory is used for `NODE_COMPILE_CACHE`. Compilation cache generated by one version of Node.js may not be used by a different version of Node.js. Cache generated by different versions of Node.js will be stored separately if the same directory is used to persist the cache, so they can co-exist. Caveat: currently when using this with V8 JavaScript code coverage, the coverage being collected by V8 may be less precise in functions that are deserialized from the code cache. It's recommended to turn this off when running tests to generate precise coverage. Implementation details: There is one cache file per module on disk. The directory layout is: - Compile cache directory (from NODE_COMPILE_CACHE) - 8b23c8fe: CRC32 hash of CachedDataVersionTag + NODE_VERESION - 2ea3424d: - 10860e5a: CRC32 hash of filename + module type - 431e9adc: ... - ... Inside the cache file, there is a header followed by the actual cache content: ``` [uint32_t] code size [uint32_t] code hash [uint32_t] cache size [uint32_t] cache hash ... compile cache content ... ``` When reading the cache file, we'll also check if the code size and code hash match the code that the module loader is loading and whether the cache size and cache hash match the file content read. If they don't match, or if V8 rejects the cache passed, we'll ignore the mismatch cache, and regenerate the cache after compilation succeeds and rewrite it to disk. PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>

Notable changes: buffer: * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428 dns: * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492 events,doc: * mark CustomEvent as stable (Daeyeon Jeong) #52618 lib, url: * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509 module: * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535 net: * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474 src: * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595 src,permission: * throw async errors on async APIs (Rafael Gonzaga) #52730 test_runner: * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529 url: * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280 PR-URL: TODO

Notable changes: buffer: * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428 dns: * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492 events,doc: * mark CustomEvent as stable (Daeyeon Jeong) #52618 lib, url: * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509 module: * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) #52535 net: * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474 src: * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595 src,permission: * throw async errors on async APIs (Rafael Gonzaga) #52730 test_runner: * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) #52529 url: * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) #52280 PR-URL: #52768

Notable changes: buffer: * improve `base64` and `base64url` performance (Yagiz Nizipli) nodejs#52428 dns: * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) nodejs#52492 events,doc: * mark CustomEvent as stable (Daeyeon Jeong) nodejs#52618 lib, url: * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) nodejs#52509 module: * (SEMVER-MINOR) implement NODE_COMPILE_CACHE for automatic on-disk code caching (Joyee Cheung) nodejs#52535 net: * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) nodejs#52474 src: * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) nodejs#52595 src,permission: * throw async errors on async APIs (Rafael Gonzaga) nodejs#52730 test_runner: * (SEMVER-MINOR) add --test-skip-pattern cli option (Aviv Keller) nodejs#52529 url: * (SEMVER-MINOR) implement parse method for safer URL parsing (Ali Hassan) nodejs#52280 PR-URL: nodejs#52768

joyeecheung added 2 commits April 14, 2024 23:00

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. labels Apr 15, 2024

joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. and removed lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 15, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 15, 2024

joyeecheung mentioned this pull request Apr 15, 2024

Support for file-system based persistent code cache in user-land module loaders #47472

Closed

joyeecheung commented Apr 15, 2024

View reviewed changes

richardlau added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 15, 2024

anonrig reviewed Apr 15, 2024

View reviewed changes

src/compile_cache.cc Outdated Show resolved Hide resolved

VoltrexKeyva reviewed Apr 15, 2024

View reviewed changes

src/compile_cache.cc Outdated Show resolved Hide resolved

benjamingr reviewed Apr 16, 2024

View reviewed changes

benjamingr approved these changes Apr 16, 2024

View reviewed changes

benjamingr reviewed Apr 16, 2024

View reviewed changes

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 17, 2024

fixup! module: implement NODE_COMPILE_CACHE for automatic on-disk cod…

b45062b

…e caching

joyeecheung force-pushed the node-compile-cache branch from 9a7e1ed to b45062b Compare April 17, 2024 02:44

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 17, 2024

joyeecheung added the notable-change PRs with changes that should be highlighted in changelogs. label Apr 17, 2024

joyeecheung added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Apr 17, 2024

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 17, 2024

joyeecheung closed this Apr 17, 2024

prabhu mentioned this pull request Apr 18, 2024

[perf] Investigate NODE_COMPILE_CACHE CycloneDX/cdxgen#996

Open

joyeecheung mentioned this pull request Apr 25, 2024

Tracking issue: compile cache #52696

Open

7 tasks

aduh95 added the backported-to-v22.x PRs backported to the v22.x-staging branch. label Apr 30, 2024

aduh95 mentioned this pull request Apr 30, 2024

2024-05-02, Version 22.1.0 (Current) #52768

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

module: implement NODE_COMPILE_CACHE for automatic on-disk code caching #52535

module: implement NODE_COMPILE_CACHE for automatic on-disk code caching #52535

joyeecheung commented Apr 15, 2024 •

edited

nodejs-github-bot commented Apr 15, 2024

nodejs-github-bot commented Apr 15, 2024

joyeecheung Apr 15, 2024 •

edited

merceyz commented Apr 15, 2024 •

edited

jdalton commented Apr 15, 2024

benjamingr Apr 16, 2024

joyeecheung Apr 16, 2024

benjamingr Apr 17, 2024

benjamingr left a comment

benjamingr Apr 16, 2024

joyeecheung Apr 16, 2024 •

edited

benjamingr Apr 17, 2024

joyeecheung Apr 17, 2024

joyeecheung commented Apr 16, 2024 •

edited

merceyz commented Apr 16, 2024

joyeecheung commented Apr 16, 2024

nodejs-github-bot commented Apr 17, 2024

github-actions bot commented Apr 17, 2024 •

edited by joyeecheung

nodejs-github-bot commented Apr 17, 2024

Refs: v8/v8@`cd10ad7`
PR-URL: #52535
Refs: #47472
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io
Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com

PR-URL: #52535
Refs: #47472
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io
Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com

Successfully rebased and updated refs/heads/main.

joyeecheung commented Apr 17, 2024

lirantal commented Apr 18, 2024

joyeecheung commented Apr 18, 2024 •

edited

lirantal commented Apr 18, 2024


		> Stability: 1.1 - Active Development

		When set, whenever Node.js compiles a CommonJS or a ECMAScript Module,

module: implement NODE_COMPILE_CACHE for automatic on-disk code caching #52535

module: implement NODE_COMPILE_CACHE for automatic on-disk code caching #52535

Conversation

joyeecheung commented Apr 15, 2024 • edited

summary

nodejs-github-bot commented Apr 15, 2024

nodejs-github-bot commented Apr 15, 2024

joyeecheung Apr 15, 2024 • edited

Choose a reason for hiding this comment

merceyz commented Apr 15, 2024 • edited

jdalton commented Apr 15, 2024

benjamingr Apr 16, 2024

Choose a reason for hiding this comment

joyeecheung Apr 16, 2024

Choose a reason for hiding this comment

benjamingr Apr 17, 2024

Choose a reason for hiding this comment

benjamingr left a comment

Choose a reason for hiding this comment

benjamingr Apr 16, 2024

Choose a reason for hiding this comment

joyeecheung Apr 16, 2024 • edited

Choose a reason for hiding this comment

benjamingr Apr 17, 2024

Choose a reason for hiding this comment

joyeecheung Apr 17, 2024

Choose a reason for hiding this comment

joyeecheung commented Apr 16, 2024 • edited

merceyz commented Apr 16, 2024

joyeecheung commented Apr 16, 2024

nodejs-github-bot commented Apr 17, 2024

github-actions bot commented Apr 17, 2024 • edited by joyeecheung

nodejs-github-bot commented Apr 17, 2024

Refs: v8/v8@cd10ad7 PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com

PR-URL: #52535 Refs: #47472 Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com

Successfully rebased and updated refs/heads/main.

joyeecheung commented Apr 17, 2024

lirantal commented Apr 18, 2024

joyeecheung commented Apr 18, 2024 • edited

lirantal commented Apr 18, 2024

joyeecheung commented Apr 15, 2024 •

edited

joyeecheung Apr 15, 2024 •

edited

merceyz commented Apr 15, 2024 •

edited

joyeecheung Apr 16, 2024 •

edited

joyeecheung commented Apr 16, 2024 •

edited

github-actions bot commented Apr 17, 2024 •

edited by joyeecheung

Refs: v8/v8@`cd10ad7`
PR-URL: #52535
Refs: #47472
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io
Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com

PR-URL: #52535
Refs: #47472
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz.nizipli@sentry.io
Reviewed-By: Mohammed Keyvanzadeh mohammadkeyvanzade94@gmail.com

joyeecheung commented Apr 18, 2024 •

edited