[compiler] Post compile tasks from ignition instead of the parser

Posting compile tasks from the parser has several issues:

  1. We don't know how many functions there will be total, so we can't
     yet allocate shared_function_infos array on the Script
  2. Without this array, inner function compiles can't look up their own
     inner functions during bytecode finalization, so we can't run that
     finalization before script parse completes
  3. Scope analysis can't have run yet, so we can only post top-level
     function tasks and if we allocate SharedFunctionInfos early they
     are forced into a bit of a limbo state without an outer ScopeInfo.

Instead, we can post compile tasks during bytecode generation. Then, the
script parse is guaranteed to have completed, so we'll have a
shared_function_infos array and we will have allocated ScopeInfos
already. This also opens the door for posting tasks for compiling more
inner functions than just top-level, as well as generating better code
for functions/methods that reference same-script top-level
let/const/class.

Bug: chromium:1267680
Change-Id: Ie1a3a3c6f1b264c4ef28cd4763bfc6dc08f45d4d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3277884
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77894}

src/ast/ast.h

@@ -2192,6 +2192,13 @@ class FunctionLiteral final : public Expression {
     return HasDuplicateParameters::decode(bit_field_);
   }
 
+  bool should_parallel_compile() const {
+    return ShouldParallelCompileField::decode(bit_field_);
+  }
+  void set_should_parallel_compile() {
+    bit_field_ = ShouldParallelCompileField::update(bit_field_, true);
+  }
+
   // This is used as a heuristic on when to eagerly compile a function
   // literal. We consider the following constructs as hints that the
   // function will be called immediately:
@@ -2281,7 +2288,8 @@ class FunctionLiteral final : public Expression {
                   HasDuplicateParameters::encode(has_duplicate_parameters ==
                                                  kHasDuplicateParameters) |
                   RequiresInstanceMembersInitializer::encode(false) |
-                  HasBracesField::encode(has_braces);
+                  HasBracesField::encode(has_braces) |
+                  ShouldParallelCompileField::encode(false);
     if (eager_compile_hint == kShouldEagerCompile) SetShouldEagerCompile();
   }
 
@@ -2296,6 +2304,7 @@ class FunctionLiteral final : public Expression {
   using HasStaticPrivateMethodsOrAccessorsField =
       ClassScopeHasPrivateBrandField::Next<bool, 1>;
   using HasBracesField = HasStaticPrivateMethodsOrAccessorsField::Next<bool, 1>;
+  using ShouldParallelCompileField = HasBracesField::Next<bool, 1>;
 
   // expected_property_count_ is the sum of instance fields and properties.
   // It can vary depending on whether a function is lazily or eagerly parsed.

src/codegen/compiler.cc

@@ -687,7 +687,7 @@ CompilationJob::Status FinalizeSingleUnoptimizedCompilationJob(
 
 std::unique_ptr<UnoptimizedCompilationJob>
 ExecuteSingleUnoptimizedCompilationJob(
-    ParseInfo* parse_info, FunctionLiteral* literal,
+    ParseInfo* parse_info, FunctionLiteral* literal, Handle<Script> script,
     AccountingAllocator* allocator,
     std::vector<FunctionLiteral*>* eager_inner_literals,
     LocalIsolate* local_isolate) {
@@ -707,7 +707,8 @@ ExecuteSingleUnoptimizedCompilationJob(
 #endif
   std::unique_ptr<UnoptimizedCompilationJob> job(
       interpreter::Interpreter::NewCompilationJob(
-          parse_info, literal, allocator, eager_inner_literals, local_isolate));
+          parse_info, literal, script, allocator, eager_inner_literals,
+          local_isolate));
 
   if (job->ExecuteJob() != CompilationJob::SUCCEEDED) {
     // Compilation failed, return null.
@@ -751,8 +752,8 @@ bool IterativelyExecuteAndFinalizeUnoptimizedCompilationJobs(
     if (shared_info->is_compiled()) continue;
 
     std::unique_ptr<UnoptimizedCompilationJob> job =
-        ExecuteSingleUnoptimizedCompilationJob(parse_info, literal, allocator,
-                                               &functions_to_compile,
+        ExecuteSingleUnoptimizedCompilationJob(parse_info, literal, script,
+                                               allocator, &functions_to_compile,
                                                isolate->AsLocalIsolate());
 
     if (!job) return false;
@@ -1493,7 +1494,8 @@ void BackgroundCompileTask::Run() {
 
   bool toplevel_script_compilation = flags_.is_toplevel();
 
-  LocalIsolate isolate(isolate_for_local_isolate_, ThreadKind::kBackground);
+  LocalIsolate isolate(isolate_for_local_isolate_, ThreadKind::kBackground,
+                       worker_thread_scope.Get());
   UnparkedScope unparked_scope(&isolate);
   LocalHandleScope handle_scope(&isolate);
 

src/codegen/unoptimized-compilation-info.cc

@@ -18,7 +18,10 @@ namespace internal {
 UnoptimizedCompilationInfo::UnoptimizedCompilationInfo(Zone* zone,
                                                        ParseInfo* parse_info,
                                                        FunctionLiteral* literal)
-    : flags_(parse_info->flags()), feedback_vector_spec_(zone) {
+    : flags_(parse_info->flags()),
+      state_(parse_info->state()),
+      character_stream_(parse_info->character_stream()),
+      feedback_vector_spec_(zone) {
   // NOTE: The parse_info passed here represents the global information gathered
   // during parsing, but does not represent specific details of the actual
   // function literal being compiled for this OptimizedCompilationInfo. As such,

src/codegen/unoptimized-compilation-info.h

@@ -35,6 +35,10 @@ class V8_EXPORT_PRIVATE UnoptimizedCompilationInfo final {
                              FunctionLiteral* literal);
 
   const UnoptimizedCompileFlags& flags() const { return flags_; }
+  const UnoptimizedCompileState* state() const { return state_; }
+  const Utf16CharacterStream* character_stream() const {
+    return character_stream_;
+  }
 
   // Accessors for the input data of the function being compiled.
 
@@ -86,6 +90,10 @@ class V8_EXPORT_PRIVATE UnoptimizedCompilationInfo final {
   // Compilation flags.
   const UnoptimizedCompileFlags flags_;
 
+  // Compilation state.
+  const UnoptimizedCompileState* state_;
+  const Utf16CharacterStream* character_stream_;
+
   // The root AST node of the function literal being compiled.
   FunctionLiteral* literal_;
 

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

@@ -78,12 +78,12 @@ LazyCompileDispatcher::~LazyCompileDispatcher() {
 }
 
 void LazyCompileDispatcher::Enqueue(
-    Handle<SharedFunctionInfo> shared_info,
+    LocalIsolate* isolate, Handle<SharedFunctionInfo> shared_info,
     std::unique_ptr<Utf16CharacterStream> character_stream,
     ProducedPreparseData* preparse_data) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                "V8.LazyCompilerDispatcherEnqueue");
-  RCS_SCOPE(isolate_, RuntimeCallCounterId::kCompileEnqueueOnDispatcher);
+  RCS_SCOPE(isolate, RuntimeCallCounterId::kCompileEnqueueOnDispatcher);
 
   std::unique_ptr<Job> job =
       std::make_unique<Job>(std::make_unique<BackgroundCompileTask>(

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

@@ -83,7 +83,7 @@ class V8_EXPORT_PRIVATE LazyCompileDispatcher {
   LazyCompileDispatcher& operator=(const LazyCompileDispatcher&) = delete;
   ~LazyCompileDispatcher();
 
-  void Enqueue(Handle<SharedFunctionInfo> shared_info,
+  void Enqueue(LocalIsolate* isolate, Handle<SharedFunctionInfo> shared_info,
                std::unique_ptr<Utf16CharacterStream> character_stream,
                ProducedPreparseData* preparse_data);
 

src/execution/isolate.cc

@@ -3210,7 +3210,6 @@ void Isolate::Deinit() {
 
   ReleaseSharedPtrs();
 
-  string_table_.reset();
   builtins_.TearDown();
   bootstrapper_->TearDown();
 
@@ -3227,6 +3226,8 @@ void Isolate::Deinit() {
     lazy_compile_dispatcher_.reset();
   }
 
+  string_table_.reset();
+
   delete baseline_batch_compiler_;
   baseline_batch_compiler_ = nullptr;
 

src/interpreter/bytecode-generator.cc

@@ -17,6 +17,8 @@
 #include "src/codegen/compiler.h"
 #include "src/codegen/unoptimized-compilation-info.h"
 #include "src/common/globals.h"
+#include "src/compiler-dispatcher/lazy-compile-dispatcher.h"
+#include "src/heap/parked-scope.h"
 #include "src/interpreter/bytecode-flags.h"
 #include "src/interpreter/bytecode-jump-table.h"
 #include "src/interpreter/bytecode-label.h"
@@ -1130,10 +1132,12 @@ static bool IsInEagerLiterals(
 #endif  // DEBUG
 
 BytecodeGenerator::BytecodeGenerator(
-    Zone* compile_zone, UnoptimizedCompilationInfo* info,
+    LocalIsolate* local_isolate, Zone* compile_zone,
+    UnoptimizedCompilationInfo* info,
     const AstStringConstants* ast_string_constants,
-    std::vector<FunctionLiteral*>* eager_inner_literals)
-    : zone_(compile_zone),
+    std::vector<FunctionLiteral*>* eager_inner_literals, Handle<Script> script)
+    : local_isolate_(local_isolate),
+      zone_(compile_zone),
       builder_(zone(), info->num_parameters_including_this(),
                info->scope()->num_stack_slots(), info->feedback_vector_spec(),
                info->SourcePositionRecordingMode()),
@@ -1142,6 +1146,7 @@ BytecodeGenerator::BytecodeGenerator(
       closure_scope_(info->scope()),
       current_scope_(info->scope()),
       eager_inner_literals_(eager_inner_literals),
+      script_(script),
       feedback_slot_cache_(zone()->New<FeedbackSlotCache>(zone())),
       top_level_builder_(zone()->New<TopLevelDeclarationsBuilder>()),
       block_coverage_builder_(nullptr),
@@ -1358,10 +1363,6 @@ bool NeedsContextInitialization(DeclarationScope* scope) {
 }  // namespace
 
 void BytecodeGenerator::GenerateBytecode(uintptr_t stack_limit) {
-  DisallowGarbageCollection no_gc;
-  DisallowHandleAllocation no_handles;
-  DisallowHandleDereference no_deref;
-
   InitializeAstVisitor(stack_limit);
 
   // Initialize the incoming context.
@@ -2503,8 +2504,34 @@ void BytecodeGenerator::VisitFunctionLiteral(FunctionLiteral* expr) {
 }
 
 void BytecodeGenerator::AddToEagerLiteralsIfEager(FunctionLiteral* literal) {
-  if (eager_inner_literals_ && literal->ShouldEagerCompile()) {
+  // Only parallel compile when there's a script (not the case for source
+  // position collection).
+  if (!script_.is_null() && literal->should_parallel_compile()) {
+    // If we are already eagerly compiling this function, it must be because of
+    // --parallel-compile-tasks.
+    DCHECK_IMPLIES(!literal->ShouldEagerCompile(), FLAG_parallel_compile_tasks);
+    // There exists a lazy compile dispatcher.
+    DCHECK(info()->state()->dispatcher());
+    // There exists a cloneable character stream.
+    DCHECK(info()->character_stream()->can_be_cloned_for_parallel_access());
+
+    UnparkedScope scope(local_isolate_);
+    // If there doesn't already exist a SharedFunctionInfo for this function,
+    // then create one and enqueue it. Otherwise, we're reparsing (e.g. for the
+    // debugger, source position collection, call printing, recompile after
+    // flushing, etc.) and don't want to over-compile.
+    Handle<SharedFunctionInfo> shared_info;
+    if (!Script::FindSharedFunctionInfo(script_, local_isolate_, literal)
+             .ToHandle(&shared_info)) {
+      shared_info =
+          Compiler::GetSharedFunctionInfo(literal, script_, local_isolate_);
+      info()->state()->dispatcher()->Enqueue(
+          local_isolate_, shared_info, info()->character_stream()->Clone(),
+          literal->produced_preparse_data());
+    }
+  } else if (eager_inner_literals_ && literal->ShouldEagerCompile()) {
     DCHECK(!IsInEagerLiterals(literal, *eager_inner_literals_));
+    DCHECK(!literal->should_parallel_compile());
     eager_inner_literals_->push_back(literal);
   }
 }

src/interpreter/bytecode-generator.h

@@ -32,9 +32,10 @@ class BytecodeJumpTable;
 class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
  public:
   explicit BytecodeGenerator(
-      Zone* zone, UnoptimizedCompilationInfo* info,
+      LocalIsolate* local_isolate, Zone* zone, UnoptimizedCompilationInfo* info,
       const AstStringConstants* ast_string_constants,
-      std::vector<FunctionLiteral*>* eager_inner_literals);
+      std::vector<FunctionLiteral*>* eager_inner_literals,
+      Handle<Script> script);
 
   void GenerateBytecode(uintptr_t stack_limit);
   template <typename IsolateT>
@@ -499,6 +500,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
     current_loop_scope_ = loop_scope;
   }
 
+  LocalIsolate* local_isolate_;
   Zone* zone_;
   BytecodeArrayBuilder builder_;
   UnoptimizedCompilationInfo* info_;
@@ -508,6 +510,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
 
   // External vector of literals to be eagerly compiled.
   std::vector<FunctionLiteral*>* eager_inner_literals_;
+  Handle<Script> script_;
 
   FeedbackSlotCache* feedback_slot_cache_;
 

src/interpreter/interpreter.cc

@@ -35,6 +35,7 @@ namespace interpreter {
 class InterpreterCompilationJob final : public UnoptimizedCompilationJob {
  public:
   InterpreterCompilationJob(ParseInfo* parse_info, FunctionLiteral* literal,
+                            Handle<Script> script,
                             AccountingAllocator* allocator,
                             std::vector<FunctionLiteral*>* eager_inner_literals,
                             LocalIsolate* local_isolate);
@@ -170,7 +171,7 @@ bool ShouldPrintBytecode(Handle<SharedFunctionInfo> shared) {
 }  // namespace
 
 InterpreterCompilationJob::InterpreterCompilationJob(
-    ParseInfo* parse_info, FunctionLiteral* literal,
+    ParseInfo* parse_info, FunctionLiteral* literal, Handle<Script> script,
     AccountingAllocator* allocator,
     std::vector<FunctionLiteral*>* eager_inner_literals,
     LocalIsolate* local_isolate)
@@ -179,8 +180,9 @@ InterpreterCompilationJob::InterpreterCompilationJob(
       zone_(allocator, ZONE_NAME),
       compilation_info_(&zone_, parse_info, literal),
       local_isolate_(local_isolate),
-      generator_(&zone_, &compilation_info_, parse_info->ast_string_constants(),
-                 eager_inner_literals) {}
+      generator_(local_isolate, &zone_, &compilation_info_,
+                 parse_info->ast_string_constants(), eager_inner_literals,
+                 script) {}
 
 InterpreterCompilationJob::Status InterpreterCompilationJob::ExecuteJobImpl() {
   RCS_SCOPE(parse_info()->runtime_call_stats(),
@@ -196,8 +198,7 @@ InterpreterCompilationJob::Status InterpreterCompilationJob::ExecuteJobImpl() {
     MaybePrintAst(parse_info(), compilation_info());
   }
 
-  base::Optional<ParkedScope> parked_scope;
-  if (local_isolate_) parked_scope.emplace(local_isolate_);
+  ParkedScope parked_scope(local_isolate_);
 
   generator()->GenerateBytecode(stack_limit());
 
@@ -303,12 +304,13 @@ InterpreterCompilationJob::Status InterpreterCompilationJob::DoFinalizeJobImpl(
 }
 
 std::unique_ptr<UnoptimizedCompilationJob> Interpreter::NewCompilationJob(
-    ParseInfo* parse_info, FunctionLiteral* literal,
+    ParseInfo* parse_info, FunctionLiteral* literal, Handle<Script> script,
     AccountingAllocator* allocator,
     std::vector<FunctionLiteral*>* eager_inner_literals,
     LocalIsolate* local_isolate) {
   return std::make_unique<InterpreterCompilationJob>(
-      parse_info, literal, allocator, eager_inner_literals, local_isolate);
+      parse_info, literal, script, allocator, eager_inner_literals,
+      local_isolate);
 }
 
 std::unique_ptr<UnoptimizedCompilationJob>
@@ -317,7 +319,7 @@ Interpreter::NewSourcePositionCollectionJob(
     Handle<BytecodeArray> existing_bytecode, AccountingAllocator* allocator,
     LocalIsolate* local_isolate) {
   auto job = std::make_unique<InterpreterCompilationJob>(
-      parse_info, literal, allocator, nullptr, local_isolate);
+      parse_info, literal, Handle<Script>(), allocator, nullptr, local_isolate);
   job->compilation_info()->SetBytecodeArray(existing_bytecode);
   return job;
 }

src/interpreter/interpreter.h

@@ -46,7 +46,7 @@ class Interpreter {
   // Additionally, if |eager_inner_literals| is not null, adds any eagerly
   // compilable inner FunctionLiterals to this list.
   static std::unique_ptr<UnoptimizedCompilationJob> NewCompilationJob(
-      ParseInfo* parse_info, FunctionLiteral* literal,
+      ParseInfo* parse_info, FunctionLiteral* literal, Handle<Script> script,
       AccountingAllocator* allocator,
       std::vector<FunctionLiteral*>* eager_inner_literals,
       LocalIsolate* local_isolate);

src/logging/runtime-call-stats-scope.h

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "src/execution/isolate.h"
+#include "src/execution/local-isolate.h"
 #include "src/logging/counters.h"
 #include "src/logging/runtime-call-stats.h"
 #include "src/logging/tracing-flags.h"
@@ -29,6 +30,14 @@ RuntimeCallTimerScope::RuntimeCallTimerScope(Isolate* isolate,
   stats_->Enter(&timer_, counter_id);
 }
 
+RuntimeCallTimerScope::RuntimeCallTimerScope(LocalIsolate* isolate,
+                                             RuntimeCallCounterId counter_id) {
+  DCHECK_NOT_NULL(isolate->runtime_call_stats());
+  if (V8_LIKELY(!TracingFlags::is_runtime_stats_enabled())) return;
+  stats_ = isolate->runtime_call_stats();
+  stats_->Enter(&timer_, counter_id);
+}
+
 #else  // RUNTIME_CALL_STATS
 
 #define RCS_SCOPE(...)

src/logging/runtime-call-stats.h

@@ -711,6 +711,8 @@ class V8_NODISCARD RuntimeCallTimerScope {
  public:
   inline RuntimeCallTimerScope(Isolate* isolate,
                                RuntimeCallCounterId counter_id);
+  inline RuntimeCallTimerScope(LocalIsolate* isolate,
+                               RuntimeCallCounterId counter_id);
   inline RuntimeCallTimerScope(RuntimeCallStats* stats,
                                RuntimeCallCounterId counter_id,
                                RuntimeCallStats::CounterMode mode =

src/parsing/parser.cc

@@ -494,18 +494,6 @@ void Parser::DeserializeScopeChain(
 
 namespace {
 
-void MaybeResetCharacterStream(ParseInfo* info, FunctionLiteral* literal) {
-#if V8_ENABLE_WEBASSEMBLY
-  // Don't reset the character stream if there is an asm.js module since it will
-  // be used again by the asm-parser.
-  if (info->contains_asm_module()) {
-    if (FLAG_stress_validate_asm) return;
-    if (literal != nullptr && literal->scope()->ContainsAsmModule()) return;
-  }
-#endif  // V8_ENABLE_WEBASSEMBLY
-  info->ResetCharacterStream();
-}
-
 void MaybeProcessSourceRanges(ParseInfo* parse_info, Expression* root,
                               uintptr_t stack_limit_) {
   if (root != nullptr && parse_info->source_range_map() != nullptr) {
@@ -543,7 +531,6 @@ void Parser::ParseProgram(Isolate* isolate, Handle<Script> script,
 
   scanner_.Initialize();
   FunctionLiteral* result = DoParseProgram(isolate, info);
-  MaybeResetCharacterStream(info, result);
   MaybeProcessSourceRanges(info, result, stack_limit_);
   PostProcessParseResult(isolate, info, result);
 
@@ -874,7 +861,6 @@ void Parser::ParseFunction(Isolate* isolate, ParseInfo* info,
     result = DoParseFunction(isolate, info, start_position, end_position,
                              function_literal_id, info->function_name());
   }
-  MaybeResetCharacterStream(info, result);
   MaybeProcessSourceRanges(info, result, stack_limit_);
   if (result != nullptr) {
     Handle<String> inferred_name(shared_info->inferred_name(), isolate);
@@ -2702,13 +2688,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   RecordFunctionLiteralSourceRange(function_literal);
 
   if (should_post_parallel_task && !has_error()) {
-    // Start a parallel parse / compile task on the compiler dispatcher.
-    Handle<SharedFunctionInfo> shared_info =
-        local_isolate_->factory()->NewSharedFunctionInfoForLiteral(
-            function_literal, script_, false);
-    info()->dispatcher()->Enqueue(shared_info,
-                                  info()->character_stream()->Clone(),
-                                  function_literal->produced_preparse_data());
+    function_literal->set_should_parallel_compile();
   }
 
   if (should_infer_name) {
@@ -3331,7 +3311,6 @@ void Parser::ParseOnBackground(LocalIsolate* isolate, ParseInfo* info,
                                end_position, function_literal_id,
                                info->function_name());
     }
-    MaybeResetCharacterStream(info, result);
     MaybeProcessSourceRanges(info, result, stack_limit_);
   }
   // We need to unpark by now though, to be able to internalize.

src/parsing/preparse-data.cc

@@ -553,7 +553,11 @@ class ZoneProducedPreparseData final : public ProducedPreparseData {
     return data_->Serialize(isolate);
   }
 
-  ZonePreparseData* Serialize(Zone* zone) final { return data_; }
+  ZonePreparseData* Serialize(Zone* zone) final {
+    base::Vector<uint8_t> data(data_->byte_data()->data(),
+                               data_->byte_data()->size());
+    return zone->New<ZonePreparseData>(zone, &data, data_->children_length());
+  }
 
  private:
   ZonePreparseData* data_;

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

@@ -77,8 +77,9 @@ class LazyCompileDispatcherTest : public TestWithNativeContext {
     UnoptimizedCompileState state(isolate);
     std::unique_ptr<ParseInfo> outer_parse_info =
         test::OuterParseInfoForShared(isolate, shared, &state);
-    dispatcher->Enqueue(shared, outer_parse_info->character_stream()->Clone(),
-                        nullptr);
+    if (dispatcher->IsEnqueued(shared)) return;
+    dispatcher->Enqueue(isolate->main_thread_local_isolate(), shared,
+                        outer_parse_info->character_stream()->Clone(), nullptr);
   }
 };
 

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

@@ -258,6 +258,8 @@ TEST_F(BackgroundCompileTaskTest, LazyInnerFunctions) {
   std::unique_ptr<BackgroundCompileTask> task(
       NewBackgroundCompileTask(isolate(), shared));
 
+  // There's already a task for this SFI.
+
   task->Run();
   ASSERT_TRUE(Compiler::FinalizeBackgroundCompileTask(
       task.get(), isolate(), Compiler::KEEP_EXCEPTION));