Revert "[off-thread] Allow off-thread top-level IIFE finalization"

This reverts commit 35a6eeec.

Reason for revert: TSAN failures like https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN/39084/overview

Original change's description:
> [off-thread] Allow off-thread top-level IIFE finalization
>
> Allow off-thread finalization for parallel compile tasks (i.e. for top-
> level IIFEs).
>
> This allows us to merge the code paths in BackgroundCompileTask, and
> re-enable the compiler dispatcher tests under the off-thread
> finalization flag. Indeed, we can simplify further and get rid of that
> flag entirely (it has been on-by-default for several releases now).
>
> Change-Id: I54f361997d651667fa813ec09790a6aab4d26774
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3226780
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#77615}

Change-Id: I6752470eebd594bad92c7cf4e58dbe5bac53598c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3255667Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Owners-Override: Shu-yu Guo <syg@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#77631}

src/codegen/compiler.h

@@ -8,7 +8,6 @@
 #include <forward_list>
 #include <memory>
 
-#include "src/ast/ast-value-factory.h"
 #include "src/base/platform/elapsed-timer.h"
 #include "src/codegen/bailout-reason.h"
 #include "src/common/globals.h"
@@ -496,7 +495,8 @@ using DeferredFinalizationJobDataList =
 class V8_EXPORT_PRIVATE BackgroundCompileTask {
  public:
   // Creates a new task that when run will parse and compile the streamed
-  // script associated with |data| and can be finalized with FinalizeScript.
+  // script associated with |data| and can be finalized with
+  // Compiler::GetSharedFunctionInfoForStreamedScript.
   // Note: does not take ownership of |data|.
   BackgroundCompileTask(ScriptStreamingData* data, Isolate* isolate,
                         v8::ScriptType type);
@@ -505,44 +505,68 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
   ~BackgroundCompileTask();
 
   // Creates a new task that when run will parse and compile the
-  // |function_literal| and can be finalized with FinalizeFunction
+  // |function_literal| and can be finalized with
   // Compiler::FinalizeBackgroundCompileTask.
   BackgroundCompileTask(
-      Isolate* isolate, const ParseInfo* outer_parse_info,
-      const AstRawString* function_name,
+      const ParseInfo* outer_parse_info, const AstRawString* function_name,
       const FunctionLiteral* function_literal,
       WorkerThreadRuntimeCallStats* worker_thread_runtime_stats,
       TimedHistogram* timer, int max_stack_size);
 
   void Run();
 
-  MaybeHandle<SharedFunctionInfo> FinalizeScript(
-      Isolate* isolate, Handle<String> source,
-      const ScriptDetails& script_details);
+  ParseInfo* info() {
+    DCHECK_NOT_NULL(info_);
+    return info_.get();
+  }
+  Parser* parser() { return parser_.get(); }
+  UnoptimizedCompilationJobList* compilation_jobs() {
+    return &compilation_jobs_;
+  }
+  UnoptimizedCompileFlags flags() const { return flags_; }
+  UnoptimizedCompileState* compile_state() { return &compile_state_; }
+  LanguageMode language_mode() { return language_mode_; }
+  FinalizeUnoptimizedCompilationDataList*
+  finalize_unoptimized_compilation_data() {
+    return &finalize_unoptimized_compilation_data_;
+  }
 
-  bool FinalizeFunction(Isolate* isolate,
-                        Handle<SharedFunctionInfo> shared_info,
-                        Compiler::ClearExceptionFlag flag);
+  int use_count(v8::Isolate::UseCounterFeature feature) const {
+    return use_counts_[static_cast<int>(feature)];
+  }
+  int total_preparse_skipped() const { return total_preparse_skipped_; }
 
-  UnoptimizedCompileFlags flags() const { return flags_; }
-  LanguageMode language_mode() const { return language_mode_; }
+  // Jobs which could not be finalized in the background task, and need to be
+  // finalized on the main thread.
+  DeferredFinalizationJobDataList* jobs_to_retry_finalization_on_main_thread() {
+    return &jobs_to_retry_finalization_on_main_thread_;
+  }
 
- private:
-  void ReportStatistics(Isolate* isolate);
+  // Getters for the off-thread finalization results, that create main-thread
+  // handles to the objects.
+  MaybeHandle<SharedFunctionInfo> GetOuterFunctionSfi(Isolate* isolate);
+  Handle<Script> GetScript(Isolate* isolate);
 
-  // Data needed for parsing and compilation. These need to be initialized
-  // before the compilation starts.
-  Isolate* isolate_for_local_isolate_;
+ private:
+  // Data needed for parsing, and data needed to to be passed between thread
+  // between parsing and compilation. These need to be initialized before the
+  // compilation starts.
   UnoptimizedCompileFlags flags_;
   UnoptimizedCompileState compile_state_;
   std::unique_ptr<ParseInfo> info_;
-  int stack_size_;
-  WorkerThreadRuntimeCallStats* worker_thread_runtime_call_stats_;
-  TimedHistogram* timer_;
+  std::unique_ptr<Parser> parser_;
 
-  // Data needed for merging onto the main thread.
+  // Data needed for finalizing compilation after background compilation.
+  UnoptimizedCompilationJobList compilation_jobs_;
+
+  // Data needed for merging onto the main thread after background finalization.
+  // TODO(leszeks): When these are available, the above fields are not. We
+  // should add some stricter type-safety or DCHECKs to ensure that the user of
+  // the task knows this.
+  Isolate* isolate_for_local_isolate_;
   std::unique_ptr<PersistentHandles> persistent_handles_;
   MaybeHandle<SharedFunctionInfo> outer_function_sfi_;
+  Handle<Script> script_;
   IsCompiledScope is_compiled_scope_;
   FinalizeUnoptimizedCompilationDataList finalize_unoptimized_compilation_data_;
   DeferredFinalizationJobDataList jobs_to_retry_finalization_on_main_thread_;
@@ -550,11 +574,13 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
   int total_preparse_skipped_ = 0;
 
   // Single function data for top-level function compilation.
-  Handle<Script> script_;
   int start_position_;
   int end_position_;
   int function_literal_id_;
 
+  int stack_size_;
+  WorkerThreadRuntimeCallStats* worker_thread_runtime_call_stats_;
+  TimedHistogram* timer_;
   LanguageMode language_mode_;
 };
 

src/compiler-dispatcher/lazy-compile-dispatcher.cc

@@ -86,7 +86,7 @@ base::Optional<LazyCompileDispatcher::JobId> LazyCompileDispatcher::Enqueue(
   RCS_SCOPE(isolate_, RuntimeCallCounterId::kCompileEnqueueOnDispatcher);
 
   std::unique_ptr<Job> job = std::make_unique<Job>(new BackgroundCompileTask(
-      isolate_, outer_parse_info, function_name, function_literal,
+      outer_parse_info, function_name, function_literal,
       worker_thread_runtime_call_stats_, background_compile_timer_,
       static_cast<int>(max_stack_size_)));
   JobMap::const_iterator it = InsertJob(std::move(job));

src/d8/d8.cc

@@ -668,9 +668,8 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
 
     i::ParseInfo parse_info(i_isolate, flags, &compile_state);
 
-    parse_info.InitializeScript(i_isolate, str, i::kNullMaybeHandle,
-                                ScriptOriginOptions());
-    i::Handle<i::Script> script = parse_info.script();
+    i::Handle<i::Script> script = parse_info.CreateScript(
+        i_isolate, str, i::kNullMaybeHandle, ScriptOriginOptions());
     if (!i::parsing::ParseProgram(&parse_info, script, i_isolate,
                                   i::parsing::ReportStatisticsMode::kYes)) {
       parse_info.pending_error_handler()->PrepareErrors(

src/flags/flag-definitions.h

@@ -445,8 +445,8 @@ DEFINE_NEG_IMPLICATION(enable_third_party_heap, allocation_site_pretenuring)
 DEFINE_NEG_IMPLICATION(enable_third_party_heap, turbo_allocation_folding)
 DEFINE_NEG_IMPLICATION(enable_third_party_heap, concurrent_recompilation)
 DEFINE_NEG_IMPLICATION(enable_third_party_heap, concurrent_inlining)
-DEFINE_NEG_IMPLICATION(enable_third_party_heap, script_streaming)
-DEFINE_NEG_IMPLICATION(enable_third_party_heap, parallel_compile_tasks)
+DEFINE_NEG_IMPLICATION(enable_third_party_heap,
+                       finalize_streaming_on_background)
 DEFINE_NEG_IMPLICATION(enable_third_party_heap, use_marking_progress_bar)
 DEFINE_NEG_IMPLICATION(enable_third_party_heap, move_object_start)
 DEFINE_NEG_IMPLICATION(enable_third_party_heap, concurrent_marking)
@@ -1479,8 +1479,14 @@ DEFINE_BOOL(enable_regexp_unaligned_accesses, true,
 DEFINE_BOOL(script_streaming, true, "enable parsing on background")
 DEFINE_BOOL(stress_background_compile, false,
             "stress test parsing on background")
+DEFINE_BOOL(
+    finalize_streaming_on_background, true,
+    "perform the script streaming finalization on the background thread")
 DEFINE_BOOL(concurrent_cache_deserialization, true,
             "enable deserializing code caches on background")
+// TODO(leszeks): Parallel compile tasks currently don't support off-thread
+// finalization.
+DEFINE_NEG_IMPLICATION(parallel_compile_tasks, finalize_streaming_on_background)
 DEFINE_BOOL(disable_old_api_accessors, false,
             "Disable old-style API accessors whose setters trigger through the "
             "prototype chain")

src/heap/factory-base.cc

@@ -305,24 +305,6 @@ Handle<SharedFunctionInfo> FactoryBase<Impl>::NewSharedFunctionInfoForLiteral(
   return shared;
 }
 
-template <typename Impl>
-Handle<SharedFunctionInfo>
-FactoryBase<Impl>::NewPlaceholderSharedFunctionInfoForLazyLiteral(
-    FunctionLiteral* literal, Handle<Script> script) {
-  FunctionKind kind = literal->kind();
-  Handle<SharedFunctionInfo> shared =
-      NewSharedFunctionInfo(literal->GetName(isolate()), MaybeHandle<Code>(),
-                            Builtin::kCompileLazy, kind);
-  // Don't fully initialise the SFI from the function literal, since we e.g.
-  // might not have the scope info, but initialise just enough to work for
-  // compilation/finalization.
-  shared->set_function_literal_id(literal->function_literal_id());
-  // Set the script on the SFI, but don't make the script's SFI list point back
-  // to this SFI.
-  shared->set_script(*script);
-  return shared;
-}
-
 template <typename Impl>
 Handle<PreparseData> FactoryBase<Impl>::NewPreparseData(int data_length,
                                                         int children_length) {

src/heap/factory-base.h

@@ -162,9 +162,6 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) FactoryBase
   Handle<SharedFunctionInfo> NewSharedFunctionInfoForLiteral(
       FunctionLiteral* literal, Handle<Script> script, bool is_toplevel);
 
-  Handle<SharedFunctionInfo> NewPlaceholderSharedFunctionInfoForLazyLiteral(
-      FunctionLiteral* literal, Handle<Script> script);
-
   Handle<PreparseData> NewPreparseData(int data_length, int children_length);
 
   Handle<UncompiledDataWithoutPreparseData>

src/parsing/parse-info.cc

@@ -240,7 +240,7 @@ ParseInfo::~ParseInfo() = default;
 DeclarationScope* ParseInfo::scope() const { return literal()->scope(); }
 
 template <typename IsolateT>
-void ParseInfo::InitializeScript(
+Handle<Script> ParseInfo::CreateScript(
     IsolateT* isolate, Handle<String> source,
     MaybeHandle<FixedArray> maybe_wrapped_arguments,
     ScriptOriginOptions origin_options, NativesFlag natives) {
@@ -271,18 +271,19 @@ void ParseInfo::InitializeScript(
 
   CheckFlagsForToplevelCompileFromScript(*script,
                                          isolate->is_collecting_type_profile());
-
-  script_ = script;
+  return script;
 }
 
-template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE) void ParseInfo::
-    InitializeScript(Isolate* isolate, Handle<String> source,
-                     MaybeHandle<FixedArray> maybe_wrapped_arguments,
-                     ScriptOriginOptions origin_options, NativesFlag natives);
-template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE) void ParseInfo::
-    InitializeScript(LocalIsolate* isolate, Handle<String> source,
-                     MaybeHandle<FixedArray> maybe_wrapped_arguments,
-                     ScriptOriginOptions origin_options, NativesFlag natives);
+template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE)
+    Handle<Script> ParseInfo::CreateScript(
+        Isolate* isolate, Handle<String> source,
+        MaybeHandle<FixedArray> maybe_wrapped_arguments,
+        ScriptOriginOptions origin_options, NativesFlag natives);
+template EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE)
+    Handle<Script> ParseInfo::CreateScript(
+        LocalIsolate* isolate, Handle<String> source,
+        MaybeHandle<FixedArray> maybe_wrapped_arguments,
+        ScriptOriginOptions origin_options, NativesFlag natives);
 
 AstValueFactory* ParseInfo::GetOrCreateAstValueFactory() {
   if (!ast_value_factory_.get()) {

src/parsing/parse-info.h

@@ -225,10 +225,10 @@ class V8_EXPORT_PRIVATE ParseInfo {
 
   template <typename IsolateT>
   EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE)
-  void InitializeScript(IsolateT* isolate, Handle<String> source,
-                        MaybeHandle<FixedArray> maybe_wrapped_arguments,
-                        ScriptOriginOptions origin_options,
-                        NativesFlag natives = NOT_NATIVES_CODE);
+  Handle<Script> CreateScript(IsolateT* isolate, Handle<String> source,
+                              MaybeHandle<FixedArray> maybe_wrapped_arguments,
+                              ScriptOriginOptions origin_options,
+                              NativesFlag natives = NOT_NATIVES_CODE);
 
   // Either returns the ast-value-factory associcated with this ParseInfo, or
   // creates and returns a new factory if none exists.
@@ -274,9 +274,6 @@ class V8_EXPORT_PRIVATE ParseInfo {
   LanguageMode language_mode() const { return language_mode_; }
   void set_language_mode(LanguageMode value) { language_mode_ = value; }
 
-  Handle<Script> script() const { return script_; }
-  void set_script(Handle<Script> script) { script_ = script; }
-
   Utf16CharacterStream* character_stream() const {
     return character_stream_.get();
   }
@@ -355,7 +352,6 @@ class V8_EXPORT_PRIVATE ParseInfo {
   int max_function_literal_id_;
 
   //----------- Inputs+Outputs of parsing and scope analysis -----------------
-  Handle<Script> script_;
   std::unique_ptr<Utf16CharacterStream> character_stream_;
   std::unique_ptr<ConsumedPreparseData> consumed_preparse_data_;
   std::unique_ptr<AstValueFactory> ast_value_factory_;

test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc

@@ -39,6 +39,7 @@ class LazyCompilerDispatcherTestFlags {
     FLAG_single_threaded = true;
     FlagList::EnforceFlagImplications();
     FLAG_lazy_compile_dispatcher = true;
+    FLAG_finalize_streaming_on_background = false;
   }
 
   static void RestoreFlags() {
@@ -106,6 +107,15 @@ class LazyCompilerDispatcherTest : public TestWithNativeContext {
     return dispatcher->Enqueue(outer_parse_info.get(), function_name,
                                function_literal);
   }
+
+ protected:
+  void SetUp() override {
+    // TODO(leszeks): Support background finalization in compiler dispatcher.
+    if (FLAG_finalize_streaming_on_background) {
+      GTEST_SKIP_(
+          "Parallel compile tasks don't yet support background finalization");
+    }
+  }
 };
 
 namespace {

test/unittests/heap/local-factory-unittest.cc

@@ -88,10 +88,9 @@ class LocalFactoryTest : public TestWithIsolateAndZone {
     parse_info()->ast_value_factory()->Internalize(local_isolate());
     DeclarationScope::AllocateScopeInfos(parse_info(), local_isolate());
 
-    parse_info_.InitializeScript(local_isolate(),
-                                 local_factory()->empty_string(),
-                                 kNullMaybeHandle, ScriptOriginOptions());
-    script_ = parse_info_.script();
+    script_ = parse_info_.CreateScript(local_isolate(),
+                                       local_factory()->empty_string(),
+                                       kNullMaybeHandle, ScriptOriginOptions());
 
     // Create the SFI list on the script so that SFI SetScript works.
     Handle<WeakFixedArray> infos = local_factory()->NewWeakFixedArray(

test/unittests/tasks/background-compile-task-unittest.cc

@@ -80,11 +80,20 @@ class BackgroundCompileTaskTest : public TestWithNativeContext {
             shared->function_literal_id(), nullptr);
 
     return new BackgroundCompileTask(
-        isolate, outer_parse_info.get(), function_name, function_literal,
+        outer_parse_info.get(), function_name, function_literal,
         isolate->counters()->worker_thread_runtime_call_stats(),
         isolate->counters()->compile_function_on_background(), FLAG_stack_size);
   }
 
+ protected:
+  void SetUp() override {
+    // TODO(leszeks): Support background finalization in compiler dispatcher.
+    if (FLAG_finalize_streaming_on_background) {
+      GTEST_SKIP_(
+          "Parallel compile tasks don't yet support background finalization");
+    }
+  }
+
  private:
   AccountingAllocator* allocator_;
   static SaveFlags* save_flags_;

test/unittests/test-helpers.cc

@@ -67,8 +67,6 @@ std::unique_ptr<ParseInfo> OuterParseInfoForShared(
       ScannerStream::For(isolate, source));
   result->set_character_stream(std::move(stream));
 
-  result->set_script(handle(script, isolate));
-
   return result;
 }