[offthread] Unify compiler.cc finalization logic

This patch unfies the finalization logic between the various unoptimized
compilation paths in compiler.cc, taking the various post-processings and
fixups needed for off-thread finalization and performing them in the same
order for the other finalizations.

It also unifies the general compilation path between streaming script
compilation, main-thread script compilation, and main-thread lazy
compilation, making the main-thread paths both use an iterative execution
and finalization, and making all three use the same job helper methods
and overall finalization helper.

Bug: chromium:1011762
Change-Id: Ibe56f6d2f75a2deffbe9e0b600ded8a02293b722
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2172790
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67609}

src/asmjs/asm-js.cc

@@ -224,7 +224,7 @@ class AsmJsCompilationJob final : public UnoptimizedCompilationJob {
 
 UnoptimizedCompilationJob::Status AsmJsCompilationJob::ExecuteJobImpl() {
   // Step 1: Translate asm.js module to WebAssembly module.
-  Zone* compile_zone = compilation_info()->zone();
+  Zone* compile_zone = &zone_;
   Zone translate_zone(allocator_, ZONE_NAME);
 
   Utf16CharacterStream* stream = parse_info()->character_stream();

src/ast/ast-value-factory.cc

@@ -27,6 +27,7 @@
 
 #include "src/ast/ast-value-factory.h"
 
+#include "src/base/logging.h"
 #include "src/common/globals.h"
 #include "src/execution/off-thread-isolate.h"
 #include "src/heap/factory-inl.h"

src/ast/ast-value-factory.h

@@ -31,6 +31,7 @@
 #include <forward_list>
 
 #include "src/base/hashmap.h"
+#include "src/base/logging.h"
 #include "src/common/globals.h"
 #include "src/heap/factory.h"
 #include "src/numbers/conversions.h"

src/codegen/compiler.h

@@ -276,6 +276,13 @@ class UnoptimizedCompilationJob : public CompilationJob {
 
   uintptr_t stack_limit() const { return stack_limit_; }
 
+  base::TimeDelta time_taken_to_execute() const {
+    return time_taken_to_execute_;
+  }
+  base::TimeDelta time_taken_to_finalize() const {
+    return time_taken_to_finalize_;
+  }
+
   bool can_off_thread_finalize() const { return can_off_thread_finalize_; }
 
  protected:
@@ -354,6 +361,15 @@ class OptimizedCompilationJob : public CompilationJob {
   const char* compiler_name_;
 };
 
+struct FinalizeUnoptimizedCompilationData {
+  int function_literal_id;
+  base::TimeDelta time_taken_to_execute;
+  base::TimeDelta time_taken_to_finalize;
+};
+
+using FinalizeUnoptimizedCompilationDataList =
+    std::vector<FinalizeUnoptimizedCompilationData>;
+
 class V8_EXPORT_PRIVATE BackgroundCompileTask {
  public:
   // Creates a new task that when run will parse and compile the streamed
@@ -386,18 +402,14 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
     return &inner_function_jobs_;
   }
   UnoptimizedCompileFlags flags() const { return flags_; }
+  const UnoptimizedCompileState* compile_state() const {
+    return &compile_state_;
+  }
   LanguageMode language_mode() { return language_mode_; }
-  bool collected_source_positions() { return collected_source_positions_; }
   bool finalize_on_background_thread() {
     return finalize_on_background_thread_;
   }
   OffThreadIsolate* off_thread_isolate() { return off_thread_isolate_.get(); }
-  PendingCompilationErrorHandler* pending_error_handler() {
-    return compile_state_.pending_error_handler();
-  }
-  UnoptimizedCompileState::ParallelTasks* parallel_tasks() {
-    return compile_state_.parallel_tasks();
-  }
   MaybeHandle<SharedFunctionInfo> outer_function_sfi() {
     DCHECK_NOT_NULL(off_thread_isolate_);
     return outer_function_sfi_.ToHandle();
@@ -406,6 +418,10 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
     DCHECK_NOT_NULL(off_thread_isolate_);
     return script_.ToHandle();
   }
+  FinalizeUnoptimizedCompilationDataList*
+  finalize_unoptimized_compilation_data() {
+    return &finalize_unoptimized_compilation_data_;
+  }
 
  private:
   // Data needed for parsing, and data needed to to be passed between thread
@@ -427,6 +443,7 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
   std::unique_ptr<OffThreadIsolate> off_thread_isolate_;
   OffThreadTransferMaybeHandle<SharedFunctionInfo> outer_function_sfi_;
   OffThreadTransferHandle<Script> script_;
+  FinalizeUnoptimizedCompilationDataList finalize_unoptimized_compilation_data_;
 
   // Single function data for top-level function compilation.
   int start_position_;
@@ -437,7 +454,6 @@ class V8_EXPORT_PRIVATE BackgroundCompileTask {
   WorkerThreadRuntimeCallStats* worker_thread_runtime_call_stats_;
   TimedHistogram* timer_;
   LanguageMode language_mode_;
-  bool collected_source_positions_;
 
   // True if the background compilation should be finalized on the background
   // thread. When this is true, the ParseInfo, Parser and compilation jobs are

src/codegen/unoptimized-compilation-info.cc

@@ -18,7 +18,7 @@ namespace internal {
 UnoptimizedCompilationInfo::UnoptimizedCompilationInfo(Zone* zone,
                                                        ParseInfo* parse_info,
                                                        FunctionLiteral* literal)
-    : flags_(parse_info->flags()), zone_(zone), feedback_vector_spec_(zone) {
+    : flags_(parse_info->flags()), 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

@@ -34,8 +34,6 @@ class V8_EXPORT_PRIVATE UnoptimizedCompilationInfo final {
   UnoptimizedCompilationInfo(Zone* zone, ParseInfo* parse_info,
                              FunctionLiteral* literal);
 
-  Zone* zone() { return zone_; }
-
   const UnoptimizedCompileFlags& flags() const { return flags_; }
 
   // Accessors for the input data of the function being compiled.
@@ -87,10 +85,6 @@ class V8_EXPORT_PRIVATE UnoptimizedCompilationInfo final {
   // Compilation flags.
   const UnoptimizedCompileFlags flags_;
 
-  // The zone from which the compilation pipeline working on this
-  // OptimizedCompilationInfo allocates.
-  Zone* zone_;
-
   // The root AST node of the function literal being compiled.
   FunctionLiteral* literal_;
 

src/d8/d8.cc

@@ -504,10 +504,14 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
             i_isolate, true, i::construct_language_mode(i::FLAG_use_strict),
             i::REPLMode::kNo);
 
+    if (options.compile_options == v8::ScriptCompiler::kEagerCompile) {
+      flags.set_is_eager(true);
+    }
+
     i::ParseInfo parse_info(i_isolate, flags, &compile_state);
 
     i::Handle<i::Script> script = parse_info.CreateScript(
-        i_isolate, str, i::kNullMaybeHandle, options.compile_options);
+        i_isolate, str, i::kNullMaybeHandle, ScriptOriginOptions());
     if (!i::parsing::ParseProgram(&parse_info, script, i_isolate)) {
       fprintf(stderr, "Failed parsing\n");
       return false;

src/execution/off-thread-isolate.h

@@ -105,6 +105,8 @@ class V8_EXPORT_PRIVATE OffThreadIsolate final
   // on the main thread.
   void Publish(Isolate* isolate);
 
+  bool has_pending_exception() const { return false; }
+
   template <typename T>
   Handle<T> Throw(Handle<Object> exception) {
     UNREACHABLE();

src/interpreter/bytecode-generator.cc

@@ -1040,10 +1040,10 @@ static bool IsInEagerLiterals(
 #endif  // DEBUG
 
 BytecodeGenerator::BytecodeGenerator(
-    UnoptimizedCompilationInfo* info,
+    Zone* compile_zone, UnoptimizedCompilationInfo* info,
     const AstStringConstants* ast_string_constants,
     std::vector<FunctionLiteral*>* eager_inner_literals)
-    : zone_(info->zone()),
+    : zone_(compile_zone),
       builder_(zone(), info->num_parameters_including_this(),
                info->scope()->num_stack_slots(), info->feedback_vector_spec(),
                info->SourcePositionRecordingMode()),

src/interpreter/bytecode-generator.h

@@ -32,7 +32,7 @@ class BytecodeJumpTable;
 class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
  public:
   explicit BytecodeGenerator(
-      UnoptimizedCompilationInfo* info,
+      Zone* zone, UnoptimizedCompilationInfo* info,
       const AstStringConstants* ast_string_constants,
       std::vector<FunctionLiteral*>* eager_inner_literals);
 

src/interpreter/interpreter.cc

@@ -152,7 +152,7 @@ InterpreterCompilationJob::InterpreterCompilationJob(
                                 &compilation_info_, CanOffThreadFinalize::kYes),
       zone_(allocator, ZONE_NAME),
       compilation_info_(&zone_, parse_info, literal),
-      generator_(&compilation_info_, parse_info->ast_string_constants(),
+      generator_(&zone_, &compilation_info_, parse_info->ast_string_constants(),
                  eager_inner_literals) {}
 
 InterpreterCompilationJob::Status InterpreterCompilationJob::ExecuteJobImpl() {

src/objects/objects.cc

@@ -4907,14 +4907,14 @@ Object Script::GetNameOrSourceURL() {
 
 template <typename LocalIsolate>
 MaybeHandle<SharedFunctionInfo> Script::FindSharedFunctionInfo(
-    LocalIsolate* isolate, const FunctionLiteral* fun) {
-  CHECK_NE(fun->function_literal_id(), kFunctionLiteralIdInvalid);
+    LocalIsolate* isolate, int function_literal_id) {
+  CHECK_NE(function_literal_id, kFunctionLiteralIdInvalid);
   // If this check fails, the problem is most probably the function id
   // renumbering done by AstFunctionLiteralIdReindexer; in particular, that
   // AstTraversalVisitor doesn't recurse properly in the construct which
   // triggers the mismatch.
-  CHECK_LT(fun->function_literal_id(), shared_function_infos().length());
-  MaybeObject shared = shared_function_infos().Get(fun->function_literal_id());
+  CHECK_LT(function_literal_id, shared_function_infos().length());
+  MaybeObject shared = shared_function_infos().Get(function_literal_id);
   HeapObject heap_object;
   if (!shared->GetHeapObject(&heap_object) ||
       heap_object.IsUndefined(isolate)) {
@@ -4923,9 +4923,9 @@ MaybeHandle<SharedFunctionInfo> Script::FindSharedFunctionInfo(
   return handle(SharedFunctionInfo::cast(heap_object), isolate);
 }
 template MaybeHandle<SharedFunctionInfo> Script::FindSharedFunctionInfo(
-    Isolate* isolate, const FunctionLiteral* fun);
+    Isolate* isolate, int function_literal_id);
 template MaybeHandle<SharedFunctionInfo> Script::FindSharedFunctionInfo(
-    OffThreadIsolate* isolate, const FunctionLiteral* fun);
+    OffThreadIsolate* isolate, int function_literal_id);
 
 Script::Iterator::Iterator(Isolate* isolate)
     : iterator_(isolate->heap()->script_list()) {}

src/objects/script.h

@@ -201,7 +201,7 @@ class Script : public Struct {
   // that matches the function literal.  Return empty handle if not found.
   template <typename LocalIsolate>
   MaybeHandle<SharedFunctionInfo> FindSharedFunctionInfo(
-      LocalIsolate* isolate, const FunctionLiteral* fun);
+      LocalIsolate* isolate, int function_literal_id);
 
   // Iterate over all script objects on the heap.
   class V8_EXPORT_PRIVATE Iterator {

src/parsing/parse-info.h

@@ -188,6 +188,9 @@ class V8_EXPORT_PRIVATE UnoptimizedCompileState {
   PendingCompilationErrorHandler* pending_error_handler() {
     return &pending_error_handler_;
   }
+  const PendingCompilationErrorHandler* pending_error_handler() const {
+    return &pending_error_handler_;
+  }
   ParallelTasks* parallel_tasks() const { return parallel_tasks_.get(); }
 
  private:

src/parsing/parsing.cc

@@ -27,8 +27,9 @@ void MaybeReportErrorsAndStatistics(ParseInfo* info, Handle<Script> script,
                                     ReportErrorsAndStatisticsMode mode) {
   if (mode == ReportErrorsAndStatisticsMode::kYes) {
     if (info->literal() == nullptr) {
-      info->pending_error_handler()->ReportErrors(isolate, script,
+      info->pending_error_handler()->PrepareErrors(isolate,
                                                    info->ast_value_factory());
+      info->pending_error_handler()->ReportErrors(isolate, script);
     }
     parser->UpdateStatistics(isolate, script);
   }

src/parsing/pending-compilation-error-handler.cc

@@ -9,35 +9,63 @@
 #include "src/debug/debug.h"
 #include "src/execution/isolate.h"
 #include "src/execution/messages.h"
+#include "src/execution/off-thread-isolate.h"
 #include "src/handles/handles.h"
+#include "src/heap/off-thread-factory-inl.h"
 #include "src/objects/objects-inl.h"
 
 namespace v8 {
 namespace internal {
 
-void PendingCompilationErrorHandler::MessageDetails::TransferOffThreadHandle(
-    OffThreadIsolate* isolate) {
+void PendingCompilationErrorHandler::MessageDetails::SetString(
+    Handle<String> string, Isolate* isolate) {
   DCHECK_NE(type_, kMainThreadHandle);
-  if (type_ != kAstRawString) return;
-  arg_transfer_handle_ = isolate->TransferHandle(arg_->string());
+  DCHECK_NE(type_, kOffThreadTransferHandle);
+  type_ = kMainThreadHandle;
+  arg_handle_ = string;
+}
+
+void PendingCompilationErrorHandler::MessageDetails::SetString(
+    Handle<String> string, OffThreadIsolate* isolate) {
+  DCHECK_NE(type_, kMainThreadHandle);
+  DCHECK_NE(type_, kOffThreadTransferHandle);
   type_ = kOffThreadTransferHandle;
+  arg_transfer_handle_ = isolate->TransferHandle(string);
 }
 
-Handle<String> PendingCompilationErrorHandler::MessageDetails::ArgumentString(
-    Isolate* isolate) const {
+template <typename LocalIsolate>
+void PendingCompilationErrorHandler::MessageDetails::Prepare(
+    LocalIsolate* isolate) {
   switch (type_) {
     case kAstRawString:
-      return arg_->string();
+      return SetString(arg_->string(), isolate);
     case kNone:
-      return isolate->factory()->undefined_string();
     case kConstCharString:
-      return isolate->factory()
-          ->NewStringFromUtf8(CStrVector(char_arg_))
-          .ToHandleChecked();
+      // We can delay allocation until ArgumentString(isolate).
+      // TODO(leszeks): We don't actually have to transfer this string, since
+      // it's a root.
+      return;
+    case kMainThreadHandle:
+    case kOffThreadTransferHandle:
+      UNREACHABLE();
+  }
+}
+
+Handle<String> PendingCompilationErrorHandler::MessageDetails::ArgumentString(
+    Isolate* isolate) const {
+  switch (type_) {
     case kMainThreadHandle:
       return arg_handle_;
     case kOffThreadTransferHandle:
       return arg_transfer_handle_.ToHandle();
+    case kNone:
+      return isolate->factory()->undefined_string();
+    case kConstCharString:
+      return isolate->factory()
+          ->NewStringFromUtf8(CStrVector(char_arg_), AllocationType::kOld)
+          .ToHandleChecked();
+    case kAstRawString:
+      UNREACHABLE();
   }
 }
 
@@ -74,8 +102,20 @@ void PendingCompilationErrorHandler::ReportWarningAt(int start_position,
       MessageDetails(start_position, end_position, message, arg));
 }
 
-void PendingCompilationErrorHandler::ReportWarnings(Isolate* isolate,
-                                                    Handle<Script> script) {
+template <typename LocalIsolate>
+void PendingCompilationErrorHandler::PrepareWarnings(LocalIsolate* isolate) {
+  DCHECK(!has_pending_error());
+
+  for (MessageDetails& warning : warning_messages_) {
+    warning.Prepare(isolate);
+  }
+}
+template void PendingCompilationErrorHandler::PrepareWarnings(Isolate* isolate);
+template void PendingCompilationErrorHandler::PrepareWarnings(
+    OffThreadIsolate* isolate);
+
+void PendingCompilationErrorHandler::ReportWarnings(
+    Isolate* isolate, Handle<Script> script) const {
   DCHECK(!has_pending_error());
 
   for (const MessageDetails& warning : warning_messages_) {
@@ -89,53 +129,33 @@ void PendingCompilationErrorHandler::ReportWarnings(Isolate* isolate,
   }
 }
 
-void PendingCompilationErrorHandler::ReportWarnings(OffThreadIsolate* isolate,
-                                                    Handle<Script> script) {
-  // Change any AstRawStrings to raw object pointers before the Ast Zone dies,
-  // re-report later on the main thread.
-  DCHECK(!has_pending_error());
-  for (MessageDetails& warning : warning_messages_) {
-    warning.TransferOffThreadHandle(isolate);
-  }
-}
+template <typename LocalIsolate>
+void PendingCompilationErrorHandler::PrepareErrors(
+    LocalIsolate* isolate, AstValueFactory* ast_value_factory) {
+  if (stack_overflow()) return;
 
-void PendingCompilationErrorHandler::ReportErrors(
-    Isolate* isolate, Handle<Script> script,
-    AstValueFactory* ast_value_factory) {
-  if (stack_overflow()) {
-    isolate->StackOverflow();
-  } else {
   DCHECK(has_pending_error());
   // Internalize ast values for throwing the pending error.
   ast_value_factory->Internalize(isolate);
-    ThrowPendingError(isolate, script);
-  }
+  error_details_.Prepare(isolate);
 }
+template void PendingCompilationErrorHandler::PrepareErrors(
+    Isolate* isolate, AstValueFactory* ast_value_factory);
+template void PendingCompilationErrorHandler::PrepareErrors(
+    OffThreadIsolate* isolate, AstValueFactory* ast_value_factory);
 
-void PendingCompilationErrorHandler::PrepareErrorsOffThread(
-    OffThreadIsolate* isolate, Handle<Script> script,
-    AstValueFactory* ast_value_factory) {
-  if (!stack_overflow()) {
-    DCHECK(has_pending_error());
-    // Internalize ast values for later throwing the pending error.
-    ast_value_factory->Internalize(isolate);
-    error_details_.TransferOffThreadHandle(isolate);
-  }
-}
-
-void PendingCompilationErrorHandler::ReportErrorsAfterOffThreadFinalization(
-    Isolate* isolate, Handle<Script> script) {
+void PendingCompilationErrorHandler::ReportErrors(Isolate* isolate,
+                                                  Handle<Script> script) const {
   if (stack_overflow()) {
     isolate->StackOverflow();
   } else {
     DCHECK(has_pending_error());
-    // Ast values should already be internalized.
     ThrowPendingError(isolate, script);
   }
 }
 
-void PendingCompilationErrorHandler::ThrowPendingError(Isolate* isolate,
-                                                       Handle<Script> script) {
+void PendingCompilationErrorHandler::ThrowPendingError(
+    Isolate* isolate, Handle<Script> script) const {
   if (!has_pending_error_) return;
 
   MessageLocation location = error_details_.GetLocation(script);
@@ -149,7 +169,8 @@ void PendingCompilationErrorHandler::ThrowPendingError(Isolate* isolate,
 }
 
 Handle<String> PendingCompilationErrorHandler::FormatErrorMessageForTest(
-    Isolate* isolate) const {
+    Isolate* isolate) {
+  error_details_.Prepare(isolate);
   return MessageFormatter::Format(isolate, error_details_.message(),
                                   error_details_.ArgumentString(isolate));
 }

src/parsing/pending-compilation-error-handler.h

@@ -48,23 +48,16 @@ class PendingCompilationErrorHandler {
   bool has_pending_warnings() const { return !warning_messages_.empty(); }
 
   // Handle errors detected during parsing.
-  void ReportErrors(Isolate* isolate, Handle<Script> script,
-                    AstValueFactory* ast_value_factory);
-  // Prepare errors detected during off-thread parsing, to be reported later on
-  // the main thread.
-  void PrepareErrorsOffThread(OffThreadIsolate* isolate, Handle<Script> script,
-                              AstValueFactory* ast_value_factory);
-  // Report errors detected during off-thread parsing, which were prepared
-  // off-thread during finalization by the above method.
-  void ReportErrorsAfterOffThreadFinalization(Isolate* isolate,
-                                              Handle<Script> script);
+  template <typename LocalIsolate>
+  void PrepareErrors(LocalIsolate* isolate, AstValueFactory* ast_value_factory);
+  void ReportErrors(Isolate* isolate, Handle<Script> script) const;
 
   // Handle warnings detected during compilation.
-  void ReportWarnings(Isolate* isolate, Handle<Script> script);
-  void ReportWarnings(OffThreadIsolate* isolate, Handle<Script> script);
+  template <typename LocalIsolate>
+  void PrepareWarnings(LocalIsolate* isolate);
+  void ReportWarnings(Isolate* isolate, Handle<Script> script) const;
 
-  V8_EXPORT_PRIVATE Handle<String> FormatErrorMessageForTest(
-      Isolate* isolate) const;
+  V8_EXPORT_PRIVATE Handle<String> FormatErrorMessageForTest(Isolate* isolate);
 
   void set_unidentifiable_error() {
     has_pending_error_ = true;
@@ -106,32 +99,34 @@ class PendingCompilationErrorHandler {
     MessageLocation GetLocation(Handle<Script> script) const;
     MessageTemplate message() const { return message_; }
 
-    // After off-thread finalization, the Ast Zone will be deleted, so before
-    // that happens we have to transfer any string handles.
-    void TransferOffThreadHandle(OffThreadIsolate* isolate);
+    template <typename LocalIsolate>
+    void Prepare(LocalIsolate* isolate);
 
    private:
     enum Type {
       kNone,
       kAstRawString,
       kConstCharString,
-      kOffThreadTransferHandle,
-      kMainThreadHandle
+      kMainThreadHandle,
+      kOffThreadTransferHandle
     };
 
+    void SetString(Handle<String> string, Isolate* isolate);
+    void SetString(Handle<String> string, OffThreadIsolate* isolate);
+
     int start_position_;
     int end_position_;
     MessageTemplate message_;
     union {
       const AstRawString* arg_;
       const char* char_arg_;
-      OffThreadTransferHandle<String> arg_transfer_handle_;
       Handle<String> arg_handle_;
+      OffThreadTransferHandle<String> arg_transfer_handle_;
     };
     Type type_;
   };
 
-  void ThrowPendingError(Isolate* isolate, Handle<Script> script);
+  void ThrowPendingError(Isolate* isolate, Handle<Script> script) const;
 
   bool has_pending_error_;
   bool stack_overflow_;