Skip to content

Commit

Permalink
deps: V8: cherry-pick cb00db4dba6c
Browse files Browse the repository at this point in the history
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: v8/v8@cb00db4
PR-URL: #48671
Refs: #48576
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
kvakil authored and ruyadorno committed Sep 16, 2023
1 parent 3f65598 commit bcb255d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
2 changes: 1 addition & 1 deletion deps/v8/src/codegen/compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3199,7 +3199,7 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
// functions fully non-lazy instead thus preventing source positions from
// being omitted.
flags.set_collect_source_positions(true);
// flags.set_eager(compile_options == ScriptCompiler::kEagerCompile);
flags.set_is_eager(compile_options == ScriptCompiler::kEagerCompile);

UnoptimizedCompileState compile_state;
ReusableUnoptimizedCompileState reusable_state(isolate);
Expand Down
31 changes: 31 additions & 0 deletions deps/v8/test/cctest/test-serialize.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4937,6 +4937,37 @@ TEST(CachedCompileFunction) {
}
}

TEST(CachedCompileFunctionRespectsEager) {
DisableAlwaysOpt();
LocalContext env;
Isolate* isolate = CcTest::i_isolate();
isolate->compilation_cache()
->DisableScriptAndEval(); // Disable same-isolate code cache.

v8::HandleScope scope(CcTest::isolate());

v8::Local<v8::String> source = v8_str("return function() { return 42; }");
v8::ScriptCompiler::Source script_source(source);

for (bool eager_compile : {false, true}) {
v8::ScriptCompiler::CompileOptions options =
eager_compile ? v8::ScriptCompiler::kEagerCompile
: v8::ScriptCompiler::kNoCompileOptions;
v8::Local<v8::Value> fun =
v8::ScriptCompiler::CompileFunction(env.local(), &script_source, 0,
nullptr, 0, nullptr, options)
.ToLocalChecked()
.As<v8::Function>()
->Call(env.local(), v8::Undefined(CcTest::isolate()), 0, nullptr)
.ToLocalChecked();

auto i_fun = i::Handle<i::JSFunction>::cast(Utils::OpenHandle(*fun));

// Function should be compiled iff kEagerCompile was used.
CHECK_EQ(i_fun->shared().is_compiled(), eager_compile);
}
}

UNINITIALIZED_TEST(SnapshotCreatorAnonClassWithKeep) {
DisableAlwaysOpt();
v8::SnapshotCreator creator;
Expand Down

0 comments on commit bcb255d

Please sign in to comment.