Skip to content

src: pre-emptively fix tests broken by V8 CL

Rodrigo Muino Tomonari requested to merge github/fork/kvakil/cp-v8-change into main

I put up a CL to V8 to fix the fact that kEagerCompile was being completely ignored when used as an input to CompileFunction. Turns out actually enabling kEagerCompile breaks a bunch of tests. Oops.

To stop us from getting the broken tests when we update V8, (1) stop using kEagerCompile and (2) cherry-pick the patch. The latter is not strictly necessary but will hopefully stop us from introducing new use-cases of kEagerCompile which would break once it's actually fixed.

Manually tested this by compiling before & after this PR and inspecting out/Release/gen/node_snapshot.cc for differences. There were no differences in the code cache but some minor differences in the snapshot (which makes sense since snapshotting is not deterministic).

src: remove kEagerCompile for CompileFunction

It wasn't doing anything, and actually enabling it would cause some tests to fail.

Refs: https://github.com/nodejs/node/pull/48576

deps: V8: cherry-pick cb00db4dba6c

Original commit message:

[compiler] fix CompileFunction ignoring kEagerCompile

v8::ScriptCompiler::CompileFunction was ignoring kEagerCompile. Unlike
the other functions in v8::ScriptCompiler, it was not actually
propagating kEagerCompile to the parser. The newly updated test fails
without this change.

I did some archeology and found that this was commented out since the
original CL in https://crrev.com/c/980944.

As far as I know Node.js is the main consumer of this particular API.
This CL speeds up Node.js's overall startup time by ~13%.

Change-Id: Ifc3cd6653555194d46ca48db14f7ba7a4afe0053
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4571822
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#87944}

Refs: https://github.com/v8/v8/commit/cb00db4dba6c4a1900700d134e2293c59155db28

Merge request reports