Enable background merging when --stress-background-compile

This change adds new functions to BackgroundCompileTask which closely
match those in BackgroundDeserializeTask. These functions allow a caller
to manage background merging of newly compiled content into an existing
Script from the Isolate compilation cache. These functions are not yet
exposed via the API; instead, StressBackgroundCompileThread uses them to
increase test coverage of the merging logic.

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

src/codegen/background-merge-task.h

@@ -25,6 +25,8 @@ struct ScriptDetails;
 // compilation cache.
 class V8_EXPORT_PRIVATE BackgroundMergeTask {
  public:
+  ~BackgroundMergeTask();
+
   // Step 1: on the main thread, check whether the Isolate compilation cache
   // contains the script.
   void SetUpOnMainThread(Isolate* isolate, Handle<String> source_text,

src/codegen/compiler.cc

@@ -1925,6 +1925,10 @@ class ConstantPoolPointerForwarder {
   std::unordered_map<int, Handle<SharedFunctionInfo>> forwarding_table_;
 };
 
+BackgroundMergeTask::~BackgroundMergeTask() {
+  DCHECK(!HasPendingForegroundWork());
+}
+
 void BackgroundMergeTask::SetUpOnMainThread(Isolate* isolate,
                                             Handle<String> source_text,
                                             const ScriptDetails& script_details,
@@ -2084,6 +2088,12 @@ Handle<SharedFunctionInfo> BackgroundMergeTask::CompleteMergeInForeground(
   Handle<SharedFunctionInfo> result = handle(
       SharedFunctionInfo::cast(maybe_toplevel_sfi.GetHeapObjectAssumeWeak()),
       isolate);
+
+  // Abandon the persistent handles from the background thread, so that
+  // future calls to HasPendingForegroundWork return false.
+  used_new_sfis_.clear();
+  new_compiled_data_for_cached_sfis_.clear();
+
   return handle_scope.CloseAndEscape(result);
 }
 
@@ -2096,44 +2106,56 @@ MaybeHandle<SharedFunctionInfo> BackgroundCompileTask::FinalizeScript(
   DCHECK_EQ(flags_.is_module(), origin_options.IsModule());
 
   MaybeHandle<SharedFunctionInfo> maybe_result;
+  Handle<Script> script = script_;
 
   // We might not have been able to finalize all jobs on the background
   // thread (e.g. asm.js jobs), so finalize those deferred jobs now.
   if (FinalizeDeferredUnoptimizedCompilationJobs(
-          isolate, script_, &jobs_to_retry_finalization_on_main_thread_,
+          isolate, script, &jobs_to_retry_finalization_on_main_thread_,
           compile_state_.pending_error_handler(),
           &finalize_unoptimized_compilation_data_)) {
     maybe_result = outer_function_sfi_;
   }
 
-  script_->set_source(*source);
-  script_->set_origin_options(origin_options);
+  if (!maybe_result.is_null() &&
+      background_merge_task_.HasPendingForegroundWork()) {
+    DCHECK(flags().is_toplevel());
+    Handle<SharedFunctionInfo> result =
+        background_merge_task_.CompleteMergeInForeground(isolate, script);
+    maybe_result = result;
+    script = handle(Script::cast(result->script()), isolate);
+    DCHECK(script->source().StrictEquals(*source));
+    DCHECK(isolate->factory()->script_list()->Contains(
+        MaybeObject::MakeWeak(MaybeObject::FromObject(*script))));
+  } else {
+    script->set_source(*source);
+    script->set_origin_options(origin_options);
 
-  // The one post-hoc fix-up: Add the script to the script list.
-  Handle<WeakArrayList> scripts = isolate->factory()->script_list();
-  scripts =
-      WeakArrayList::Append(isolate, scripts, MaybeObjectHandle::Weak(script_));
-  isolate->heap()->SetRootScriptList(*scripts);
+    // The one post-hoc fix-up: Add the script to the script list.
+    Handle<WeakArrayList> scripts = isolate->factory()->script_list();
+    scripts = WeakArrayList::Append(isolate, scripts,
+                                    MaybeObjectHandle::Weak(script));
+    isolate->heap()->SetRootScriptList(*scripts);
 
-  // Set the script fields after finalization, to keep this path the same
-  // between main-thread and off-thread finalization.
-  {
-    DisallowGarbageCollection no_gc;
-    SetScriptFieldsFromDetails(isolate, *script_, script_details, &no_gc);
-    LOG(isolate, ScriptDetails(*script_));
+    // Set the script fields after finalization, to keep this path the same
+    // between main-thread and off-thread finalization.
+    {
+      DisallowGarbageCollection no_gc;
+      SetScriptFieldsFromDetails(isolate, *script, script_details, &no_gc);
+      LOG(isolate, ScriptDetails(*script));
+    }
   }
 
   ReportStatistics(isolate);
 
   Handle<SharedFunctionInfo> result;
   if (!maybe_result.ToHandle(&result)) {
-    FailWithPreparedPendingException(isolate, script_,
+    FailWithPreparedPendingException(isolate, script,
                                      compile_state_.pending_error_handler());
     return kNullMaybeHandle;
   }
 
-  FinalizeUnoptimizedScriptCompilation(isolate, script_, flags_,
-                                       &compile_state_,
+  FinalizeUnoptimizedScriptCompilation(isolate, script, flags_, &compile_state_,
                                        finalize_unoptimized_compilation_data_);
 
   return handle(*result, isolate);
@@ -2142,6 +2164,7 @@ MaybeHandle<SharedFunctionInfo> BackgroundCompileTask::FinalizeScript(
 bool BackgroundCompileTask::FinalizeFunction(
     Isolate* isolate, Compiler::ClearExceptionFlag flag) {
   DCHECK(!flags_.is_toplevel());
+  DCHECK(!background_merge_task_.HasPendingForegroundWork());
 
   MaybeHandle<SharedFunctionInfo> maybe_result;
   Handle<SharedFunctionInfo> input_shared_info =
@@ -2233,14 +2256,35 @@ void BackgroundDeserializeTask::SourceTextAvailable(
                                            language_mode);
 }
 
+void BackgroundCompileTask::SourceTextAvailable(
+    Isolate* isolate, Handle<String> source_text,
+    const ScriptDetails& script_details) {
+  DCHECK_EQ(isolate, isolate_for_local_isolate_);
+
+  // Non-toplevel compilations already refer to an existing Script; there is no
+  // need to look anything up in the compilation cache.
+  if (!flags().is_toplevel()) return;
+
+  LanguageMode language_mode = flags().outer_language_mode();
+  background_merge_task_.SetUpOnMainThread(isolate, source_text, script_details,
+                                           language_mode);
+}
+
 bool BackgroundDeserializeTask::ShouldMergeWithExistingScript() const {
   DCHECK(FLAG_merge_background_deserialized_script_with_compilation_cache);
   return background_merge_task_.HasCachedScript() &&
          off_thread_data_.HasResult();
 }
 
+bool BackgroundCompileTask::ShouldMergeWithExistingScript() const {
+  DCHECK(FLAG_stress_background_compile);
+  DCHECK(!script_.is_null());
+  return background_merge_task_.HasCachedScript() &&
+         jobs_to_retry_finalization_on_main_thread_.empty();
+}
+
 void BackgroundDeserializeTask::MergeWithExistingScript() {
-  DCHECK(FLAG_merge_background_deserialized_script_with_compilation_cache);
+  DCHECK(ShouldMergeWithExistingScript());
 
   LocalIsolate isolate(isolate_for_local_isolate_, ThreadKind::kBackground);
   UnparkedScope unparked_scope(&isolate);
@@ -2250,6 +2294,21 @@ void BackgroundDeserializeTask::MergeWithExistingScript() {
       &isolate, off_thread_data_.GetOnlyScript(isolate.heap()));
 }
 
+void BackgroundCompileTask::MergeWithExistingScript() {
+  DCHECK(ShouldMergeWithExistingScript());
+
+  LocalIsolate isolate(isolate_for_local_isolate_, ThreadKind::kBackground);
+  UnparkedScope unparked_scope(&isolate);
+  LocalHandleScope handle_scope(isolate.heap());
+
+  // Get a non-persistent handle to the newly compiled script.
+  isolate.heap()->AttachPersistentHandles(std::move(persistent_handles_));
+  Handle<Script> script = handle(*script_, &isolate);
+  persistent_handles_ = isolate.heap()->DetachPersistentHandles();
+
+  background_merge_task_.BeginMergeInBackground(&isolate, script);
+}
+
 MaybeHandle<SharedFunctionInfo> BackgroundDeserializeTask::Finish(
     Isolate* isolate, Handle<String> source,
     ScriptOriginOptions origin_options) {
@@ -3214,18 +3273,27 @@ MaybeHandle<SharedFunctionInfo> CompileScriptOnMainThread(
 class StressBackgroundCompileThread : public base::Thread {
  public:
   StressBackgroundCompileThread(Isolate* isolate, Handle<String> source,
-                                ScriptType type)
+                                const ScriptDetails& script_details)
       : base::Thread(
             base::Thread::Options("StressBackgroundCompileThread", 2 * i::MB)),
         source_(source),
         streamed_source_(std::make_unique<SourceStream>(source, isolate),
                          v8::ScriptCompiler::StreamedSource::UTF8) {
+    ScriptType type = script_details.origin_options.IsModule()
+                          ? ScriptType::kModule
+                          : ScriptType::kClassic;
     data()->task = std::make_unique<i::BackgroundCompileTask>(
         data(), isolate, type,
         ScriptCompiler::CompileOptions::kNoCompileOptions);
+    data()->task->SourceTextAvailable(isolate, source, script_details);
   }
 
-  void Run() override { data()->task->Run(); }
+  void Run() override {
+    data()->task->Run();
+    if (data()->task->ShouldMergeWithExistingScript()) {
+      data()->task->MergeWithExistingScript();
+    }
+  }
 
   ScriptStreamingData* data() { return streamed_source_.impl(); }
 
@@ -3283,14 +3351,10 @@ bool CompilationExceptionIsRangeError(Isolate* isolate, Handle<Object> obj) {
 
 MaybeHandle<SharedFunctionInfo> CompileScriptOnBothBackgroundAndMainThread(
     Handle<String> source, const ScriptDetails& script_details,
-    MaybeHandle<Script> maybe_script, Isolate* isolate,
-    IsCompiledScope* is_compiled_scope) {
+    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
-                                               : ScriptType::kClassic);
+  StressBackgroundCompileThread background_compile_thread(isolate, source,
+                                                          script_details);
 
   UnoptimizedCompileFlags flags_copy =
       background_compile_thread.data()->task->flags();
@@ -3479,7 +3543,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, maybe_script, isolate, &is_compiled_scope);
+          source, script_details, isolate, &is_compiled_scope);
     } else {
       UnoptimizedCompileFlags flags =
           UnoptimizedCompileFlags::ForToplevelCompile(

src/codegen/compiler.h

@@ -538,6 +538,25 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
   void Run(LocalIsolate* isolate,
            ReusableUnoptimizedCompileState* reusable_state);
 
+  // Checks the Isolate compilation cache to see whether it will be necessary to
+  // merge the newly compiled objects into an existing Script. This can change
+  // the value of ShouldMergeWithExistingScript, and embedders should check the
+  // latter after calling this. May only be called on a thread where the Isolate
+  // is currently entered.
+  void SourceTextAvailable(Isolate* isolate, Handle<String> source_text,
+                           const ScriptDetails& script_details);
+
+  // Returns whether the embedder should call MergeWithExistingScript. This
+  // function may be called from any thread, any number of times, but its return
+  // value is only meaningful after SourceTextAvailable has completed.
+  bool ShouldMergeWithExistingScript() const;
+
+  // Partially merges newly compiled objects into an existing Script with the
+  // same source, and generates a list of follow-up work for the main thread.
+  // May be called from any thread, only once, and only if
+  // ShouldMergeWithExistingScript returned true.
+  void MergeWithExistingScript();
+
   MaybeHandle<SharedFunctionInfo> FinalizeScript(
       Isolate* isolate, Handle<String> source,
       const ScriptDetails& script_details);
@@ -578,6 +597,10 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
   int start_position_;
   int end_position_;
   int function_literal_id_;
+
+  // Task that merges newly compiled content into an existing Script from the
+  // Isolate compilation cache, if necessary.
+  BackgroundMergeTask background_merge_task_;
 };
 
 // Contains all data which needs to be transmitted between threads for

test/cctest/heap/test-heap.cc

@@ -1583,12 +1583,6 @@ void CompilationCacheRegeneration(bool retain_root_sfi, bool flush_root_sfi,
     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);