[Compile] Refactor UnoptimizedCompilationJob to use BackgroundCompilerTask

This CL makes UnoptimizedCompilationJob a simple proxy for
BackgroundCompilerTask. A follow-up CL will remove UnoptimizedCompilationJob
entirely and have CompilerDispatcher deal directly with BackgroundCompilerTasks

BUG=v8:8041, v8:8015

Change-Id: Ia53d05c015c4ca2ee32a4d1c5d0c65edb3caeda8
Reviewed-on: https://chromium-review.googlesource.com/1236257
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56212}

src/compiler-dispatcher/compiler-dispatcher.cc

@@ -146,6 +146,8 @@ CompilerDispatcher::CompilerDispatcher(Isolate* isolate, Platform* platform,
       allocator_(isolate->allocator()),
       worker_thread_runtime_call_stats_(
           isolate->counters()->worker_thread_runtime_call_stats()),
+      background_compile_timer_(
+          isolate->counters()->compile_function_on_background()),
       platform_(platform),
       max_stack_size_(max_stack_size),
       trace_compiler_dispatcher_(FLAG_trace_compiler_dispatcher),
@@ -200,7 +202,8 @@ base::Optional<CompilerDispatcher::JobId> CompilerDispatcher::Enqueue(
 
   std::unique_ptr<CompilerDispatcherJob> job(new UnoptimizedCompileJob(
       tracer_.get(), allocator_, outer_parse_info, function_name,
-      function_literal, worker_thread_runtime_call_stats_, max_stack_size_));
+      function_literal, worker_thread_runtime_call_stats_,
+      background_compile_timer_, max_stack_size_));
   JobMap::const_iterator it = InsertJob(std::move(job));
   JobId id = it->first;
   if (trace_compiler_dispatcher_) {

src/compiler-dispatcher/compiler-dispatcher.h

@@ -39,6 +39,7 @@ class FunctionLiteral;
 class Isolate;
 class ParseInfo;
 class SharedFunctionInfo;
+class TimedHistogram;
 class WorkerThreadRuntimeCallStats;
 class Zone;
 
@@ -153,6 +154,7 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
   Isolate* isolate_;
   AccountingAllocator* allocator_;
   WorkerThreadRuntimeCallStats* worker_thread_runtime_call_stats_;
+  TimedHistogram* background_compile_timer_;
   Platform* platform_;
   size_t max_stack_size_;
 

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

@@ -220,7 +220,7 @@ void OptimizingCompileDispatcher::InstallOptimizedFunctions() {
       }
       DisposeCompilationJob(job, false);
     } else {
-      Compiler::FinalizeCompilationJob(job, isolate_);
+      Compiler::FinalizeOptimizedCompilationJob(job, isolate_);
     }
   }
 }

src/compiler-dispatcher/unoptimized-compile-job.cc

@@ -21,83 +21,18 @@
 namespace v8 {
 namespace internal {
 
-namespace {
-
-// A scope object that ensures a parse info's runtime call stats field is set
-// correctly during worker-thread compile, and restores it after going out of
-// scope.
-class OffThreadRuntimeCallStatsScope {
- public:
-  OffThreadRuntimeCallStatsScope(
-      ParseInfo* parse_info,
-      WorkerThreadRuntimeCallStats* worker_thread_runtime_stats)
-      : parse_info_(parse_info),
-        original_runtime_call_stats_(parse_info_->runtime_call_stats()),
-        worker_thread_scope_(worker_thread_runtime_stats) {
-    parse_info_->set_runtime_call_stats(worker_thread_scope_.Get());
-  }
-
-  ~OffThreadRuntimeCallStatsScope() {
-    parse_info_->set_runtime_call_stats(original_runtime_call_stats_);
-  }
-
- private:
-  ParseInfo* parse_info_;
-  RuntimeCallStats* original_runtime_call_stats_;
-  WorkerThreadRuntimeCallStatsScope worker_thread_scope_;
-};
-
-}  // namespace
-
 UnoptimizedCompileJob::UnoptimizedCompileJob(
     CompilerDispatcherTracer* tracer, AccountingAllocator* allocator,
     const ParseInfo* outer_parse_info, const AstRawString* function_name,
     const FunctionLiteral* function_literal,
     WorkerThreadRuntimeCallStats* worker_thread_runtime_stats,
-    size_t max_stack_size)
+    TimedHistogram* timer, size_t max_stack_size)
     : CompilerDispatcherJob(Type::kUnoptimizedCompile),
       tracer_(tracer),
-      allocator_(allocator),
-      worker_thread_runtime_stats_(worker_thread_runtime_stats),
-      max_stack_size_(max_stack_size),
-      trace_compiler_dispatcher_jobs_(FLAG_trace_compiler_dispatcher_jobs) {
-  DCHECK(outer_parse_info->is_toplevel());
-  DCHECK(!function_literal->is_toplevel());
-
-  // Initialize parse_info for the given function details.
-  parse_info_ = ParseInfo::FromParent(outer_parse_info, allocator_,
-                                      function_literal, function_name);
-
-  // Clone the character stream so both can be accessed independently.
-  std::unique_ptr<Utf16CharacterStream> character_stream =
-      outer_parse_info->character_stream()->Clone();
-  character_stream->Seek(function_literal->start_position());
-  parse_info_->set_character_stream(std::move(character_stream));
-
-  // Get preparsed scope data from the function literal.
-  if (function_literal->produced_preparsed_scope_data()) {
-    DCHECK(FLAG_preparser_scope_analysis);
-    ZonePreParsedScopeData* serialized_data =
-        function_literal->produced_preparsed_scope_data()->Serialize(
-            parse_info_->zone());
-    parse_info_->set_consumed_preparsed_scope_data(
-        ConsumedPreParsedScopeData::For(parse_info_->zone(), serialized_data));
-  }
-
-  // Create a unicode cache for the parse-info.
-  // TODO(rmcilroy): Try to reuse an existing one.
-  unicode_cache_.reset(new UnicodeCache());
-  parse_info_->set_unicode_cache(unicode_cache_.get());
-
-  parser_.reset(new Parser(parse_info_.get()));
-
-  if (trace_compiler_dispatcher_jobs_) {
-    PrintF(
-        "UnoptimizedCompileJob[%p] created for function literal id %d in "
-        "initial state.\n",
-        static_cast<void*>(this), function_literal->function_literal_id());
-  }
-}
+      task_(new BackgroundCompileTask(allocator, outer_parse_info,
+                                      function_name, function_literal,
+                                      worker_thread_runtime_stats, timer,
+                                      static_cast<int>(max_stack_size))) {}
 
 UnoptimizedCompileJob::~UnoptimizedCompileJob() {
   DCHECK(status() == Status::kInitial || status() == Status::kDone);
@@ -107,69 +42,8 @@ void UnoptimizedCompileJob::Compile(bool on_background_thread) {
   DCHECK_EQ(status(), Status::kInitial);
   COMPILER_DISPATCHER_TRACE_SCOPE_WITH_NUM(
       tracer_, kCompile,
-      parse_info_->end_position() - parse_info_->start_position());
-  TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
-               "V8.UnoptimizedCompileJob::Compile");
-  if (trace_compiler_dispatcher_jobs_) {
-    PrintF("UnoptimizedCompileJob[%p]: Compiling\n", static_cast<void*>(this));
-  }
-  DisallowHeapAllocation no_allocation;
-  DisallowHandleAllocation no_handles;
-  DisallowHandleDereference no_deref;
-
-  base::Optional<OffThreadRuntimeCallStatsScope> runtime_call_stats_scope;
-  if (V8_UNLIKELY(FLAG_runtime_stats && on_background_thread)) {
-    runtime_call_stats_scope.emplace(parse_info_.get(),
-                                     worker_thread_runtime_stats_);
-  }
-
-  RuntimeCallTimerScope runtimeTimer(
-      parse_info_->runtime_call_stats(),
-      on_background_thread
-          ? RuntimeCallCounterId::kCompileBackgroundUnoptimizedCompileJob
-          : RuntimeCallCounterId::kCompileUnoptimizedCompileJob);
-
-  parse_info_->set_on_background_thread(on_background_thread);
-
-  uintptr_t stack_limit = GetCurrentStackPosition() - max_stack_size_ * KB;
-  parse_info_->set_stack_limit(stack_limit);
-
-  parser_.reset(new Parser(parse_info_.get()));
-  parser_->set_stack_limit(stack_limit);
-
-  // We only support compilation of functions with no outer scope info
-  // therefore it is correct to use an empty scope chain.
-  DCHECK(parse_info_->maybe_outer_scope_info().is_null());
-  parser_->InitializeEmptyScopeChain(parse_info_.get());
-  parser_->ParseOnBackground(parse_info_.get());
-
-  if (parse_info_->literal() == nullptr) {
-    // Parser sets error in pending error handler.
-    set_status(Status::kReadyToFinalize);
-    return;
-  }
-
-  if (!Compiler::Analyze(parse_info_.get())) {
-    parse_info_->pending_error_handler()->set_stack_overflow();
-    set_status(Status::kReadyToFinalize);
-    return;
-  }
-
-  compilation_job_.reset(interpreter::Interpreter::NewCompilationJob(
-      parse_info_.get(), parse_info_->literal(), allocator_, nullptr));
-
-  if (!compilation_job_.get()) {
-    parse_info_->pending_error_handler()->set_stack_overflow();
-    set_status(Status::kReadyToFinalize);
-    return;
-  }
-
-  if (compilation_job_->ExecuteJob() != CompilationJob::SUCCEEDED) {
-    parse_info_->pending_error_handler()->set_stack_overflow();
-    set_status(Status::kReadyToFinalize);
-    return;
-  }
-
+      task_->info()->end_position() - task_->info()->start_position());
+  task_->Run();
   set_status(Status::kReadyToFinalize);
 }
 
@@ -178,64 +52,20 @@ void UnoptimizedCompileJob::FinalizeOnMainThread(
   DCHECK_EQ(ThreadId::Current().ToInteger(), isolate->thread_id().ToInteger());
   DCHECK_EQ(status(), Status::kReadyToFinalize);
   COMPILER_DISPATCHER_TRACE_SCOPE(tracer_, kFinalize);
-  TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
-               "V8.UnoptimizedCompileJob::FinalizeOnMainThread");
-  RuntimeCallTimerScope runtimeTimer(
-      isolate, RuntimeCallCounterId::kCompileFinalizeUnoptimizedCompileJob);
-
-  if (trace_compiler_dispatcher_jobs_) {
-    PrintF("UnoptimizedCompileJob[%p]: Finalizing compiling\n",
-           static_cast<void*>(this));
-  }
-
-  HandleScope scope(isolate);
-  Handle<Script> script(Script::cast(shared->script()), isolate);
-  parse_info_->set_script(script);
-
-  parser_->UpdateStatistics(isolate, script);
-  parser_->HandleSourceURLComments(isolate, script);
 
-  if (!parse_info_->literal() || !compilation_job_.get()) {
-    // Parse or compile failed on the main thread, report errors.
-    parse_info_->pending_error_handler()->ReportErrors(
-        isolate, script, parse_info_->ast_value_factory());
-    ResetDataOnMainThread(isolate);
-    set_status(Status::kFailed);
-    return;
-  }
-
-  // Internalize ast values onto the heap.
-  parse_info_->ast_value_factory()->Internalize(isolate);
-  // Allocate scope infos for the literal.
-  DeclarationScope::AllocateScopeInfos(parse_info_.get(), isolate);
-  if (compilation_job_->state() == CompilationJob::State::kFailed ||
-      !Compiler::FinalizeCompilationJob(compilation_job_.release(), shared,
-                                        isolate)) {
-    if (!isolate->has_pending_exception()) isolate->StackOverflow();
-    ResetDataOnMainThread(isolate);
-    set_status(Status::kFailed);
-    return;
-  }
+  bool succeeded = Compiler::FinalizeBackgroundCompileTask(
+      task_.get(), shared, isolate, Compiler::KEEP_EXCEPTION);
 
   ResetDataOnMainThread(isolate);
-  set_status(Status::kDone);
+  set_status(succeeded ? Status::kDone : Status::kFailed);
 }
 
 void UnoptimizedCompileJob::ResetDataOnMainThread(Isolate* isolate) {
   DCHECK_EQ(ThreadId::Current().ToInteger(), isolate->thread_id().ToInteger());
-  TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
-               "V8.UnoptimizedCompileJob::ResetDataOnMainThread");
-  compilation_job_.reset();
-  parser_.reset();
-  unicode_cache_.reset();
-  parse_info_.reset();
+  task_.reset();
 }
 
 void UnoptimizedCompileJob::ResetOnMainThread(Isolate* isolate) {
-  if (trace_compiler_dispatcher_jobs_) {
-    PrintF("UnoptimizedCompileJob[%p]: Resetting\n", static_cast<void*>(this));
-  }
-
   ResetDataOnMainThread(isolate);
   set_status(Status::kInitial);
 }
@@ -243,8 +73,8 @@ void UnoptimizedCompileJob::ResetOnMainThread(Isolate* isolate) {
 double UnoptimizedCompileJob::EstimateRuntimeOfNextStepInMs() const {
   switch (status()) {
     case Status::kInitial:
-      return tracer_->EstimateCompileInMs(parse_info_->end_position() -
-                                          parse_info_->start_position());
+      return tracer_->EstimateCompileInMs(task_->info()->end_position() -
+                                          task_->info()->start_position());
     case Status::kReadyToFinalize:
       // TODO(rmcilroy): Pass size of bytecode to tracer to get better estimate.
       return tracer_->EstimateFinalizeInMs();

src/compiler-dispatcher/unoptimized-compile-job.h

@@ -19,6 +19,7 @@ class AccountingAllocator;
 class AstRawString;
 class AstValueFactory;
 class AstStringConstants;
+class BackgroundCompileTask;
 class CompilerDispatcherTracer;
 class DeferredHandles;
 class FunctionLiteral;
@@ -27,10 +28,13 @@ class ParseInfo;
 class Parser;
 class SharedFunctionInfo;
 class String;
+class TimedHistogram;
 class UnicodeCache;
 class UnoptimizedCompilationJob;
 class WorkerThreadRuntimeCallStats;
 
+// TODO(rmcilroy): Remove this class entirely and just have CompilerDispatcher
+// manage BackgroundCompileTasks.
 class V8_EXPORT_PRIVATE UnoptimizedCompileJob : public CompilerDispatcherJob {
  public:
   // Creates a UnoptimizedCompileJob in the initial state.
@@ -39,7 +43,7 @@ class V8_EXPORT_PRIVATE UnoptimizedCompileJob : public CompilerDispatcherJob {
       const ParseInfo* outer_parse_info, const AstRawString* function_name,
       const FunctionLiteral* function_literal,
       WorkerThreadRuntimeCallStats* worker_thread_runtime_stats,
-      size_t max_stack_size);
+      TimedHistogram* timer, size_t max_stack_size);
   ~UnoptimizedCompileJob() override;
 
   // CompilerDispatcherJob implementation.
@@ -56,19 +60,7 @@ class V8_EXPORT_PRIVATE UnoptimizedCompileJob : public CompilerDispatcherJob {
   void ResetDataOnMainThread(Isolate* isolate);
 
   CompilerDispatcherTracer* tracer_;
-  AccountingAllocator* allocator_;
-  WorkerThreadRuntimeCallStats* worker_thread_runtime_stats_;
-  size_t max_stack_size_;
-
-  // Members required for parsing.
-  std::unique_ptr<UnicodeCache> unicode_cache_;
-  std::unique_ptr<ParseInfo> parse_info_;
-  std::unique_ptr<Parser> parser_;
-
-  // Members required for compiling.
-  std::unique_ptr<UnoptimizedCompilationJob> compilation_job_;
-
-  bool trace_compiler_dispatcher_jobs_;
+  std::unique_ptr<BackgroundCompileTask> task_;
 
   DISALLOW_COPY_AND_ASSIGN(UnoptimizedCompileJob);
 };

src/compiler.h

@@ -20,6 +20,7 @@ namespace v8 {
 namespace internal {
 
 // Forward declarations.
+class AstRawString;
 class BackgroundCompileTask;
 class JavaScriptFrame;
 class OptimizedCompilationInfo;
@@ -64,12 +65,14 @@ class V8_EXPORT_PRIVATE Compiler : public AllStatic {
   V8_WARN_UNUSED_RESULT static MaybeHandle<SharedFunctionInfo>
   CompileForLiveEdit(ParseInfo* parse_info, Isolate* isolate);
 
-  // Generate and install code from previously queued compilation job.
-  static bool FinalizeCompilationJob(UnoptimizedCompilationJob* job,
-                                     Handle<SharedFunctionInfo> shared_info,
-                                     Isolate* isolate);
-  static bool FinalizeCompilationJob(OptimizedCompilationJob* job,
-                                     Isolate* isolate);
+  // Finalize and install code from previously run background compile task.
+  static bool FinalizeBackgroundCompileTask(
+      BackgroundCompileTask* task, Handle<SharedFunctionInfo> shared_info,
+      Isolate* isolate, ClearExceptionFlag flag);
+
+  // Finalize and install optimized code from previously run job.
+  static bool FinalizeOptimizedCompilationJob(OptimizedCompilationJob* job,
+                                              Isolate* isolate);
 
   // Give the compiler a chance to perform low-latency initialization tasks of
   // the given {function} on its instantiation. Note that only the runtime will
@@ -321,6 +324,16 @@ class BackgroundCompileTask {
   // Note: does not take ownership of |data|.
   BackgroundCompileTask(ScriptStreamingData* data, Isolate* isolate);
 
+  // Creates a new task that when run will parse and compile the
+  // |function_literal| and can be finalized with
+  // Compiler::FinalizeBackgroundCompileTask.
+  BackgroundCompileTask(
+      AccountingAllocator* allocator, 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();
 
   ParseInfo* info() { return info_.get(); }

src/counters.h

@@ -870,17 +870,18 @@ class RuntimeCallTimer final {
   V(BoundFunctionNameGetter)                   \
   V(BoundFunctionLengthGetter)                 \
   V(CompileBackgroundAnalyse)                  \
+  V(CompileBackgroundCompileTask)              \
   V(CompileBackgroundEval)                     \
+  V(CompileBackgroundFunction)                 \
   V(CompileBackgroundIgnition)                 \
   V(CompileBackgroundScript)                   \
   V(CompileBackgroundRewriteReturnResult)      \
   V(CompileBackgroundScopeAnalysis)            \
-  V(CompileBackgroundUnoptimizedCompileJob)    \
   V(CompileDeserialize)                        \
   V(CompileEval)                               \
   V(CompileAnalyse)                            \
   V(CompileEnqueueOnDispatcher)                \
-  V(CompileFinalizeUnoptimizedCompileJob)      \
+  V(CompileFinalizeBackgroundCompileTask)      \
   V(CompileFinishNowOnDispatcher)              \
   V(CompileFunction)                           \
   V(CompileGetFromOptimizedCodeMap)            \
@@ -890,7 +891,6 @@ class RuntimeCallTimer final {
   V(CompileScopeAnalysis)                      \
   V(CompileScript)                             \
   V(CompileSerialize)                          \
-  V(CompileUnoptimizedCompileJob)              \
   V(CompileWaitForDispatcher)                  \
   V(DeoptimizeCode)                            \
   V(FunctionCallback)                          \
@@ -1313,6 +1313,8 @@ class RuntimeCallTimerScope {
      V8.CompileScriptMicroSeconds.NoCache.CacheTooCold, 1000000, MICROSECOND)  \
   HT(compile_script_on_background,                                             \
      V8.CompileScriptMicroSeconds.BackgroundThread, 1000000, MICROSECOND)      \
+  HT(compile_function_on_background,                                           \
+     V8.CompileFunctionMicroSeconds.BackgroundThread, 1000000, MICROSECOND)    \
   HT(gc_parallel_task_latency, V8.GC.ParallelTaskLatencyMicroSeconds, 1000000, \
      MICROSECOND)
 

src/flag-definitions.h

@@ -929,11 +929,6 @@ DEFINE_BOOL(compiler_dispatcher, false, "enable compiler dispatcher")
 DEFINE_BOOL(trace_compiler_dispatcher, false,
             "trace compiler dispatcher activity")
 
-// compiler-dispatcher-job.cc
-DEFINE_BOOL(
-    trace_compiler_dispatcher_jobs, false,
-    "trace progress of individual jobs managed by the compiler dispatcher")
-
 // cpu-profiler.cc
 DEFINE_INT(cpu_profiler_sampling_interval, 1000,
            "CPU profiler sampling interval in microseconds")

test/unittests/compiler-dispatcher/unoptimized-compile-job-unittest.cc

@@ -47,13 +47,6 @@ class UnoptimizedCompileJobTest : public TestWithNativeContext {
     save_flags_ = nullptr;
   }
 
-  static Variable* LookupVariableByName(UnoptimizedCompileJob* job,
-                                        const char* name) {
-    const AstRawString* name_raw_string =
-        job->parse_info_->ast_value_factory()->GetOneByteString(name);
-    return job->parse_info_->literal()->scope()->Lookup(name_raw_string);
-  }
-
   UnoptimizedCompileJob* NewUnoptimizedCompileJob(
       Isolate* isolate, Handle<SharedFunctionInfo> shared,
       size_t stack_size = FLAG_stack_size) {
@@ -85,7 +78,7 @@ class UnoptimizedCompileJobTest : public TestWithNativeContext {
         tracer(), allocator(), outer_parse_info.get(), function_name,
         function_literal,
         isolate->counters()->worker_thread_runtime_call_stats(),
-        FLAG_stack_size);
+        isolate->counters()->compile_function_on_background(), FLAG_stack_size);
   }
 
  private:
@@ -255,11 +248,45 @@ TEST_F(UnoptimizedCompileJobTest, CompileOnBackgroundThread) {
   ASSERT_JOB_STATUS(CompilerDispatcherJob::Status::kInitial, job);
 }
 
+TEST_F(UnoptimizedCompileJobTest, EagerInnerFunctions) {
+  const char raw_script[] =
+      "function g() {\n"
+      "  f = function() {\n"
+      "    // Simulate an eager IIFE with brackets.\n "
+      "    var e = (function () { return 42; });\n"
+      "    return e;\n"
+      "  }\n"
+      "  return f;\n"
+      "}\n"
+      "g();";
+  test::ScriptResource* script =
+      new test::ScriptResource(raw_script, strlen(raw_script));
+  Handle<JSFunction> f = RunJS<JSFunction>(script);
+  Handle<SharedFunctionInfo> shared = handle(f->shared(), isolate());
+  ASSERT_FALSE(shared->is_compiled());
+  std::unique_ptr<UnoptimizedCompileJob> job(
+      NewUnoptimizedCompileJob(isolate(), shared));
+
+  job->Compile(false);
+  ASSERT_FALSE(job->IsFailed());
+  job->FinalizeOnMainThread(isolate(), shared);
+  ASSERT_FALSE(job->IsFailed());
+  ASSERT_JOB_STATUS(CompilerDispatcherJob::Status::kDone, job);
+  ASSERT_TRUE(shared->is_compiled());
+
+  Handle<JSFunction> e = RunJS<JSFunction>("f();");
+
+  ASSERT_TRUE(e->shared()->is_compiled());
+
+  job->ResetOnMainThread(isolate());
+  ASSERT_JOB_STATUS(CompilerDispatcherJob::Status::kInitial, job);
+}
+
 TEST_F(UnoptimizedCompileJobTest, LazyInnerFunctions) {
   const char raw_script[] =
       "function g() {\n"
       "  f = function() {\n"
-      "    e = (function() { return 42; });\n"
+      "    function e() { return 42; };\n"
       "    return e;\n"
       "  }\n"
       "  return f;\n"