Reuse existing Scripts during synchronous parsing

This is a partial reland of https://crrev.com/c/3597106

With this change, an existing Script from the compilation cache can be
reused after its top-level SharedFunctionInfo was discarded, but only if
the new script is parsed on the main thread (not deserialized from code
cache data, and not parsed on a background thread).

Bug: v8:12808
Change-Id: I1edaee2095306a89e2c3b91f2fd01ac053f3c770
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3689348Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#81472}

src/codegen/compiler.cc

@@ -1426,6 +1426,21 @@ Handle<SharedFunctionInfo> CreateTopLevelSharedFunctionInfo(
       parse_info->literal(), script, true);
 }
 
+Handle<SharedFunctionInfo> GetOrCreateTopLevelSharedFunctionInfo(
+    ParseInfo* parse_info, Handle<Script> script, Isolate* isolate,
+    IsCompiledScope* is_compiled_scope) {
+  EnsureSharedFunctionInfosArrayOnScript(script, parse_info, isolate);
+  MaybeHandle<SharedFunctionInfo> maybe_shared =
+      Script::FindSharedFunctionInfo(script, isolate, parse_info->literal());
+  if (Handle<SharedFunctionInfo> shared; maybe_shared.ToHandle(&shared)) {
+    DCHECK_EQ(shared->function_literal_id(),
+              parse_info->literal()->function_literal_id());
+    *is_compiled_scope = shared->is_compiled_scope(isolate);
+    return shared;
+  }
+  return CreateTopLevelSharedFunctionInfo(parse_info, script, isolate);
+}
+
 MaybeHandle<SharedFunctionInfo> CompileToplevel(
     ParseInfo* parse_info, Handle<Script> script,
     MaybeHandle<ScopeInfo> maybe_outer_scope_info, Isolate* isolate,
@@ -1459,7 +1474,8 @@ MaybeHandle<SharedFunctionInfo> CompileToplevel(
 
   // Create the SharedFunctionInfo and add it to the script's list.
   Handle<SharedFunctionInfo> shared_info =
-      CreateTopLevelSharedFunctionInfo(parse_info, script, isolate);
+      GetOrCreateTopLevelSharedFunctionInfo(parse_info, script, isolate,
+                                            is_compiled_scope);
 
   FinalizeUnoptimizedCompilationDataList
       finalize_unoptimized_compilation_data_list;
@@ -2822,14 +2838,16 @@ MaybeHandle<SharedFunctionInfo> CompileScriptOnMainThread(
     const UnoptimizedCompileFlags flags, Handle<String> source,
     const ScriptDetails& script_details, NativesFlag natives,
     v8::Extension* extension, Isolate* isolate,
-    IsCompiledScope* is_compiled_scope) {
+    MaybeHandle<Script> maybe_script, IsCompiledScope* is_compiled_scope) {
   UnoptimizedCompileState compile_state;
   ReusableUnoptimizedCompileState reusable_state(isolate);
   ParseInfo parse_info(isolate, flags, &compile_state, &reusable_state);
   parse_info.set_extension(extension);
 
-  Handle<Script> script =
-      NewScript(isolate, &parse_info, source, script_details, natives);
+  Handle<Script> script;
+  if (!maybe_script.ToHandle(&script)) {
+    script = NewScript(isolate, &parse_info, source, script_details, natives);
+  }
   DCHECK_IMPLIES(parse_info.flags().collect_type_profile(),
                  script->IsUserJavaScript());
   DCHECK_EQ(parse_info.flags().is_repl_mode(), script->is_repl_mode());
@@ -2910,8 +2928,10 @@ bool CompilationExceptionIsRangeError(Isolate* isolate, Handle<Object> obj) {
 
 MaybeHandle<SharedFunctionInfo> CompileScriptOnBothBackgroundAndMainThread(
     Handle<String> source, const ScriptDetails& script_details,
-    Isolate* isolate, IsCompiledScope* is_compiled_scope) {
+    MaybeHandle<Script> maybe_script, Isolate* isolate,
+    IsCompiledScope* is_compiled_scope) {
   // Start a background thread compiling the script.
+  // TODO(v8:12808): Use maybe_script for the background compilation.
   StressBackgroundCompileThread background_compile_thread(
       isolate, source,
       script_details.origin_options.IsModule() ? ScriptType::kModule
@@ -2934,7 +2954,7 @@ MaybeHandle<SharedFunctionInfo> CompileScriptOnBothBackgroundAndMainThread(
     flags_copy.set_script_id(Script::kTemporaryScriptId);
     main_thread_maybe_result = CompileScriptOnMainThread(
         flags_copy, source, script_details, NOT_NATIVES_CODE, nullptr, isolate,
-        &inner_is_compiled_scope);
+        MaybeHandle<Script>(), &inner_is_compiled_scope);
     if (main_thread_maybe_result.is_null()) {
       // Assume all range errors are stack overflows.
       main_thread_had_stack_overflow = CompilationExceptionIsRangeError(
@@ -3025,6 +3045,7 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
   const bool use_compilation_cache =
       extension == nullptr && script_details.repl_mode == REPLMode::kNo;
   MaybeHandle<SharedFunctionInfo> maybe_result;
+  MaybeHandle<Script> maybe_script;
   IsCompiledScope is_compiled_scope;
   if (use_compilation_cache) {
     bool can_consume_code_cache =
@@ -3034,9 +3055,11 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
     }
 
     // First check per-isolate compilation cache.
-    maybe_result =
-        compilation_cache->LookupScript(source, script_details, language_mode)
-            .toplevel_sfi();
+    CompilationCacheScript::LookupResult lookup_result =
+        compilation_cache->LookupScript(source, script_details, language_mode);
+    maybe_script = lookup_result.script();
+    maybe_result = lookup_result.toplevel_sfi();
+    is_compiled_scope = lookup_result.is_compiled_scope();
     if (!maybe_result.is_null()) {
       compile_timer.set_hit_isolate_cache();
     } else if (can_consume_code_cache) {
@@ -3047,6 +3070,10 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
       RCS_SCOPE(isolate, RuntimeCallCounterId::kCompileDeserialize);
       TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                    "V8.CompileDeserialize");
+      // TODO(v8:12808): If a Script was found in the compilation cache, then
+      // both of the code paths below (Finish and Deserialize) should make use
+      // of that Script to avoid duplicating the Script itself or any
+      // preexisting SharedFunctionInfos.
       if (deserialize_task) {
         // If there's a cache consume task, finish it.
         maybe_result = deserialize_task->Finish(isolate, source,
@@ -3081,7 +3108,7 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
       // If the --stress-background-compile flag is set, do the actual
       // compilation on a background thread, and wait for its result.
       maybe_result = CompileScriptOnBothBackgroundAndMainThread(
-          source, script_details, isolate, &is_compiled_scope);
+          source, script_details, maybe_script, isolate, &is_compiled_scope);
     } else {
       UnoptimizedCompileFlags flags =
           UnoptimizedCompileFlags::ForToplevelCompile(
@@ -3093,9 +3120,13 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
 
       flags.set_is_eager(compile_options == ScriptCompiler::kEagerCompile);
 
-      maybe_result =
-          CompileScriptOnMainThread(flags, source, script_details, natives,
-                                    extension, isolate, &is_compiled_scope);
+      if (Handle<Script> script; maybe_script.ToHandle(&script)) {
+        flags.set_script_id(script->id());
+      }
+
+      maybe_result = CompileScriptOnMainThread(
+          flags, source, script_details, natives, extension, isolate,
+          maybe_script, &is_compiled_scope);
     }
 
     // Add the result to the isolate cache.
@@ -3268,10 +3299,18 @@ Compiler::GetSharedFunctionInfoForStreamedScript(
   {
     TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                  "V8.StreamingFinalization.CheckCache");
-    maybe_result = compilation_cache
-                       ->LookupScript(source, script_details,
-                                      task->flags().outer_language_mode())
-                       .toplevel_sfi();
+    CompilationCacheScript::LookupResult lookup_result =
+        compilation_cache->LookupScript(source, script_details,
+                                        task->flags().outer_language_mode());
+
+    // TODO(v8:12808): Determine what to do if we finish streaming and find that
+    // another copy of the Script already exists but has no root
+    // SharedFunctionInfo or has an uncompiled SharedFunctionInfo. For now, we
+    // just ignore it and create a new Script.
+    if (!lookup_result.toplevel_sfi().is_null()) {
+      maybe_result = lookup_result.toplevel_sfi();
+    }
+
     if (!maybe_result.is_null()) {
       compile_timer.set_hit_isolate_cache();
     }

test/cctest/heap/test-heap.cc

@@ -1556,6 +1556,235 @@ TEST(CompilationCacheCachingBehaviorRetainScript) {
   CompilationCacheCachingBehavior(true);
 }
 
+namespace {
+
+template <typename T>
+Handle<SharedFunctionInfo> GetSharedFunctionInfo(
+    v8::Local<T> function_or_script) {
+  Handle<JSFunction> i_function =
+      Handle<JSFunction>::cast(v8::Utils::OpenHandle(*function_or_script));
+  return handle(i_function->shared(), CcTest::i_isolate());
+}
+
+template <typename T>
+void AgeBytecode(v8::Local<T> function_or_script) {
+  Handle<SharedFunctionInfo> shared = GetSharedFunctionInfo(function_or_script);
+  CHECK(shared->HasBytecodeArray());
+  const int kAgingThreshold = 6;
+  for (int i = 0; i < kAgingThreshold; i++) {
+    shared->GetBytecodeArray(CcTest::i_isolate()).MakeOlder();
+  }
+}
+
+void CompilationCacheRegeneration(bool retain_root_sfi, bool flush_root_sfi,
+                                  bool flush_eager_sfi) {
+  // If the compilation cache is turned off, this test is invalid.
+  if (!FLAG_compilation_cache) {
+    return;
+  }
+
+  // TODO(v8:12808): Remove this check once background compilation is capable of
+  // reusing an existing Script.
+  if (flush_root_sfi && FLAG_stress_background_compile) {
+    return;
+  }
+
+  // Some flags can prevent bytecode flushing, which affects this test.
+  bool flushing_disabled = !FLAG_flush_bytecode ||
+                           (FLAG_always_sparkplug && !FLAG_flush_baseline_code);
+
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+
+  const char* source =
+      "({"
+      "  lazyFunction: function () {"
+      "    var x = 42;"
+      "    var y = 42;"
+      "    var z = x + y;"
+      "  },"
+      "  eagerFunction: (function () {"
+      "    var x = 43;"
+      "    var y = 43;"
+      "    var z = x + y;"
+      "  })"
+      "})";
+
+  v8::Global<v8::Script> outer_function;
+  v8::Global<v8::Function> lazy_function;
+  v8::Global<v8::Function> eager_function;
+
+  {
+    v8::HandleScope scope(CcTest::isolate());
+    v8::Local<v8::Context> context =
+        v8::Isolate::GetCurrent()->GetCurrentContext();
+    v8::Local<v8::Script> script = v8_compile(v8_str(source));
+    outer_function.Reset(CcTest::isolate(), script);
+
+    // Even though the script has not executed, it should already be parsed.
+    Handle<SharedFunctionInfo> script_sfi = GetSharedFunctionInfo(script);
+    CHECK(script_sfi->is_compiled());
+
+    v8::Local<v8::Value> result = script->Run(context).ToLocalChecked();
+
+    // Now that the script has run, we can get references to the inner
+    // functions, and verify that the eager parsing heuristics are behaving as
+    // expected.
+    v8::Local<v8::Object> result_obj =
+        result->ToObject(context).ToLocalChecked();
+    v8::Local<v8::Value> lazy_function_value =
+        result_obj->GetRealNamedProperty(context, v8_str("lazyFunction"))
+            .ToLocalChecked();
+    CHECK(lazy_function_value->IsFunction());
+    CHECK(!GetSharedFunctionInfo(lazy_function_value)->is_compiled());
+    lazy_function.Reset(CcTest::isolate(),
+                        lazy_function_value.As<v8::Function>());
+    v8::Local<v8::Value> eager_function_value =
+        result_obj->GetRealNamedProperty(context, v8_str("eagerFunction"))
+            .ToLocalChecked();
+    CHECK(eager_function_value->IsFunction());
+    eager_function.Reset(CcTest::isolate(),
+                         eager_function_value.As<v8::Function>());
+    CHECK(GetSharedFunctionInfo(eager_function_value)->is_compiled());
+  }
+
+  {
+    v8::HandleScope scope(CcTest::isolate());
+
+    // Progress code age until it's old and ready for GC.
+    if (flush_root_sfi) {
+      v8::Local<v8::Script> outer_function_value =
+          outer_function.Get(CcTest::isolate());
+      AgeBytecode(outer_function_value);
+    }
+    if (flush_eager_sfi) {
+      v8::Local<v8::Function> eager_function_value =
+          eager_function.Get(CcTest::isolate());
+      AgeBytecode(eager_function_value);
+    }
+    if (!retain_root_sfi) {
+      outer_function.Reset();
+    }
+  }
+
+  CcTest::CollectAllGarbage();
+
+  if (FLAG_stress_incremental_marking) {
+    // If incremental marking could have started before the bytecode was aged,
+    // then we need a second collection to evict the cache entries.
+    CcTest::CollectAllGarbage();
+  }
+
+  // The root SharedFunctionInfo can be retained either by a Global in this
+  // function or by the compilation cache.
+  bool root_sfi_should_still_exist = retain_root_sfi || !flush_root_sfi;
+
+  {
+    v8::HandleScope scope(CcTest::isolate());
+
+    // The lazy function should still not be compiled.
+    Handle<SharedFunctionInfo> lazy_sfi =
+        GetSharedFunctionInfo(lazy_function.Get(CcTest::isolate()));
+    CHECK(!lazy_sfi->is_compiled());
+
+    // The eager function may have had its bytecode flushed.
+    Handle<SharedFunctionInfo> eager_sfi =
+        GetSharedFunctionInfo(eager_function.Get(CcTest::isolate()));
+    CHECK_EQ(!flush_eager_sfi || flushing_disabled, eager_sfi->is_compiled());
+
+    // Check whether the root SharedFunctionInfo is still reachable from the
+    // Script.
+    Handle<Script> script(Script::cast(lazy_sfi->script()), isolate);
+    bool root_sfi_still_exists = false;
+    MaybeObject maybe_root_sfi =
+        script->shared_function_infos().Get(kFunctionLiteralIdTopLevel);
+    if (HeapObject sfi_or_undefined;
+        maybe_root_sfi.GetHeapObject(&sfi_or_undefined)) {
+      root_sfi_still_exists = !sfi_or_undefined.IsUndefined();
+    }
+    CHECK_EQ(root_sfi_should_still_exist, root_sfi_still_exists);
+  }
+
+  {
+    // Run the script again and check that no SharedFunctionInfos were
+    // duplicated, and that the expected ones were compiled.
+    v8::HandleScope scope(CcTest::isolate());
+    v8::Local<v8::Context> context =
+        v8::Isolate::GetCurrent()->GetCurrentContext();
+    v8::Local<v8::Script> script = v8_compile(v8_str(source));
+
+    // The script should be compiled by now.
+    Handle<SharedFunctionInfo> script_sfi = GetSharedFunctionInfo(script);
+    CHECK(script_sfi->is_compiled());
+
+    // This compilation should not have created a new root SharedFunctionInfo if
+    // one already existed.
+    if (retain_root_sfi) {
+      Handle<SharedFunctionInfo> old_script_sfi =
+          GetSharedFunctionInfo(outer_function.Get(CcTest::isolate()));
+      CHECK_EQ(*old_script_sfi, *script_sfi);
+    }
+
+    Handle<SharedFunctionInfo> old_lazy_sfi =
+        GetSharedFunctionInfo(lazy_function.Get(CcTest::isolate()));
+    CHECK(!old_lazy_sfi->is_compiled());
+
+    // The only way for the eager function to be uncompiled at this point is if
+    // it was flushed but the root function was not.
+    Handle<SharedFunctionInfo> old_eager_sfi =
+        GetSharedFunctionInfo(eager_function.Get(CcTest::isolate()));
+    CHECK_EQ(!(flush_eager_sfi && !flush_root_sfi) || flushing_disabled,
+             old_eager_sfi->is_compiled());
+
+    v8::Local<v8::Value> result = script->Run(context).ToLocalChecked();
+
+    // Check that both functions reused the existing SharedFunctionInfos.
+    v8::Local<v8::Object> result_obj =
+        result->ToObject(context).ToLocalChecked();
+    v8::Local<v8::Value> lazy_function_value =
+        result_obj->GetRealNamedProperty(context, v8_str("lazyFunction"))
+            .ToLocalChecked();
+    CHECK(lazy_function_value->IsFunction());
+    Handle<SharedFunctionInfo> lazy_sfi =
+        GetSharedFunctionInfo(lazy_function_value);
+    CHECK_EQ(*old_lazy_sfi, *lazy_sfi);
+    v8::Local<v8::Value> eager_function_value =
+        result_obj->GetRealNamedProperty(context, v8_str("eagerFunction"))
+            .ToLocalChecked();
+    CHECK(eager_function_value->IsFunction());
+    Handle<SharedFunctionInfo> eager_sfi =
+        GetSharedFunctionInfo(eager_function_value);
+    CHECK_EQ(*old_eager_sfi, *eager_sfi);
+  }
+}
+
+}  // namespace
+
+TEST(CompilationCacheRegeneration0) {
+  CompilationCacheRegeneration(false, false, false);
+}
+TEST(CompilationCacheRegeneration1) {
+  CompilationCacheRegeneration(false, false, true);
+}
+TEST(CompilationCacheRegeneration2) {
+  CompilationCacheRegeneration(false, true, false);
+}
+TEST(CompilationCacheRegeneration3) {
+  CompilationCacheRegeneration(false, true, true);
+}
+TEST(CompilationCacheRegeneration4) {
+  CompilationCacheRegeneration(true, false, false);
+}
+TEST(CompilationCacheRegeneration5) {
+  CompilationCacheRegeneration(true, false, true);
+}
+TEST(CompilationCacheRegeneration6) {
+  CompilationCacheRegeneration(true, true, false);
+}
+TEST(CompilationCacheRegeneration7) {
+  CompilationCacheRegeneration(true, true, true);
+}
+
 static void OptimizeEmptyFunction(const char* name) {
   HandleScope scope(CcTest::i_isolate());
   base::EmbeddedVector<char, 256> source;

test/cctest/test-api.cc

@@ -23783,23 +23783,25 @@ TEST(StreamingWithHarmonyScopes) {
   delete[] full_source;
 }
 
-// Regression test for crbug.com/v8/12668. Verifies that after a streamed script
-// is inserted into the isolate script cache, a non-streamed script with
-// identical origin can reuse that data.
-TEST(StreamingWithIsolateScriptCache) {
+namespace {
+void StreamingWithIsolateScriptCache(bool run_gc) {
+  i::FLAG_expose_gc = true;
   const char* chunks[] = {"'use strict'; (function test() { return 13; })",
                           nullptr};
   const char* full_source = chunks[0];
   v8::Isolate* isolate = CcTest::isolate();
+  auto i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   v8::HandleScope scope(isolate);
   v8::ScriptOrigin origin(isolate, v8_str("http://foo.com"), 0, 0, false, -1,
                           v8::Local<v8::Value>(), false, false, false);
+  v8::Local<Value> first_function_untyped;
   i::Handle<i::JSFunction> first_function;
   i::Handle<i::JSFunction> second_function;
 
   // Run the script using streaming.
   {
     LocalContext env;
+    v8::EscapableHandleScope inner_scope(isolate);
 
     v8::ScriptCompiler::StreamedSource source(
         std::make_unique<TestSourceStream>(chunks),
@@ -23814,13 +23816,38 @@ TEST(StreamingWithIsolateScriptCache) {
                                     origin)
             .ToLocalChecked();
     v8::Local<Value> result(script->Run(env.local()).ToLocalChecked());
-    first_function =
-        i::Handle<i::JSFunction>::cast(v8::Utils::OpenHandle(*result));
+    first_function_untyped = inner_scope.Escape(result);
+
+    if (run_gc) {
+      // Age the top-level bytecode for the script to encourage the Isolate
+      // script cache to evict it. However, there are still active Handles
+      // referring to functions in that script, so the script itself should stay
+      // alive and reachable via the Isolate script cache.
+      i::Handle<i::JSFunction> script_function =
+          i::Handle<i::JSFunction>::cast(v8::Utils::OpenHandle(*script));
+      i::Handle<i::BytecodeArray> script_bytecode(
+          script_function->shared().GetBytecodeArray(i_isolate), i_isolate);
+      for (int i = 0; i < 5; ++i) {
+        script_bytecode->MakeOlder();
+      }
+    }
   }
 
+  first_function = i::Handle<i::JSFunction>::cast(
+      v8::Utils::OpenHandle(*first_function_untyped));
+
   // Run the same script in another Context without streaming.
   {
     LocalContext env;
+
+    if (run_gc) {
+      // Perform garbage collection, which should remove the top-level
+      // SharedFunctionInfo from the Isolate script cache. However, the
+      // corresponding Script is still reachable and therefore still present in
+      // the Isolate script cache.
+      CompileRun("gc();");
+    }
+
     v8::ScriptCompiler::Source script_source(v8_str(full_source), origin);
     Local<Script> script =
         v8::ScriptCompiler::Compile(env.local(), &script_source)
@@ -23834,6 +23861,26 @@ TEST(StreamingWithIsolateScriptCache) {
   // SharedFunctionInfo instance due to the isolate script cache.
   CHECK_EQ(first_function->shared(), second_function->shared());
 }
+}  // namespace
+
+// Regression test for crbug.com/v8/12668. Verifies that after a streamed script
+// is inserted into the isolate script cache, a non-streamed script with
+// identical origin can reuse that data.
+TEST(StreamingWithIsolateScriptCache) {
+  StreamingWithIsolateScriptCache(false);
+}
+
+// Variant of the above test which evicts the root SharedFunctionInfo from the
+// Isolate script cache but still reuses the same Script.
+TEST(StreamingWithIsolateScriptCacheClearingRootSFI) {
+  // TODO(v8:12808): Remove this check once background compilation is capable of
+  // reusing an existing Script.
+  if (v8::internal::FLAG_stress_background_compile) {
+    return;
+  }
+
+  StreamingWithIsolateScriptCache(true);
+}
 
 TEST(CodeCache) {
   v8::Isolate::CreateParams create_params;