[compiler-dispatcher] make it so that we can always parse on bg threads

BUG=v8:5215
R=rmcilroy@chromium.org,hpayer@chromium.org,vogelheim@chromium.org,marja@chromium.org

Review-Url: https://codereview.chromium.org/2625413004
Cr-Commit-Position: refs/heads/master@{#42361}

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

@@ -22,6 +22,46 @@
 namespace v8 {
 namespace internal {
 
+namespace {
+
+class OneByteWrapper : public v8::String::ExternalOneByteStringResource {
+ public:
+  OneByteWrapper(const void* data, int length) : data_(data), length_(length) {}
+  ~OneByteWrapper() override = default;
+
+  const char* data() const override {
+    return reinterpret_cast<const char*>(data_);
+  }
+
+  size_t length() const override { return static_cast<size_t>(length_); }
+
+ private:
+  const void* data_;
+  int length_;
+
+  DISALLOW_COPY_AND_ASSIGN(OneByteWrapper);
+};
+
+class TwoByteWrapper : public v8::String::ExternalStringResource {
+ public:
+  TwoByteWrapper(const void* data, int length) : data_(data), length_(length) {}
+  ~TwoByteWrapper() override = default;
+
+  const uint16_t* data() const override {
+    return reinterpret_cast<const uint16_t*>(data_);
+  }
+
+  size_t length() const override { return static_cast<size_t>(length_); }
+
+ private:
+  const void* data_;
+  int length_;
+
+  DISALLOW_COPY_AND_ASSIGN(TwoByteWrapper);
+};
+
+}  // namespace
+
 CompilerDispatcherJob::CompilerDispatcherJob(Isolate* isolate,
                                              CompilerDispatcherTracer* tracer,
                                              Handle<SharedFunctionInfo> shared,
@@ -31,14 +71,11 @@ CompilerDispatcherJob::CompilerDispatcherJob(Isolate* isolate,
       shared_(Handle<SharedFunctionInfo>::cast(
           isolate_->global_handles()->Create(*shared))),
       max_stack_size_(max_stack_size),
-      can_compile_on_background_thread_(false),
       trace_compiler_dispatcher_jobs_(FLAG_trace_compiler_dispatcher_jobs) {
   HandleScope scope(isolate_);
   DCHECK(!shared_->outer_scope_info()->IsTheHole(isolate_));
   Handle<Script> script(Script::cast(shared_->script()), isolate_);
   Handle<String> source(String::cast(script->source()), isolate_);
-  can_parse_on_background_thread_ =
-      source->IsExternalTwoByteString() || source->IsExternalOneByteString();
   if (trace_compiler_dispatcher_jobs_) {
     PrintF("CompilerDispatcherJob[%p] created for ", static_cast<void*>(this));
     shared_->ShortPrint();
@@ -78,11 +115,68 @@ void CompilerDispatcherJob::PrepareToParseOnMainThread() {
         source, shared_->start_position(), shared_->end_position()));
   } else {
     source = String::Flatten(source);
-    // Have to globalize the reference here, so it survives between function
-    // calls.
-    source_ = Handle<String>::cast(isolate_->global_handles()->Create(*source));
-    character_stream_.reset(ScannerStream::For(
-        source_, shared_->start_position(), shared_->end_position()));
+    const void* data;
+    int offset = 0;
+    int length = source->length();
+
+    // Objects in lo_space don't move, so we can just read the contents from
+    // any thread.
+    if (isolate_->heap()->lo_space()->Contains(*source)) {
+      // We need to globalize the handle to the flattened string here, in
+      // case it's not referenced from anywhere else.
+      source_ =
+          Handle<String>::cast(isolate_->global_handles()->Create(*source));
+      DisallowHeapAllocation no_allocation;
+      String::FlatContent content = source->GetFlatContent();
+      DCHECK(content.IsFlat());
+      data =
+          content.IsOneByte()
+              ? reinterpret_cast<const void*>(content.ToOneByteVector().start())
+              : reinterpret_cast<const void*>(content.ToUC16Vector().start());
+    } else {
+      // Otherwise, create a copy of the part of the string we'll parse in the
+      // zone.
+      length = (shared_->end_position() - shared_->start_position());
+      offset = shared_->start_position();
+
+      int byte_len = length * (source->IsOneByteRepresentation() ? 1 : 2);
+      data = zone_->New(byte_len);
+
+      DisallowHeapAllocation no_allocation;
+      String::FlatContent content = source->GetFlatContent();
+      DCHECK(content.IsFlat());
+      if (content.IsOneByte()) {
+        MemCopy(const_cast<void*>(data),
+                &content.ToOneByteVector().at(shared_->start_position()),
+                byte_len);
+      } else {
+        MemCopy(const_cast<void*>(data),
+                &content.ToUC16Vector().at(shared_->start_position()),
+                byte_len);
+      }
+    }
+    Handle<String> wrapper;
+    if (source->IsOneByteRepresentation()) {
+      ExternalOneByteString::Resource* resource =
+          new OneByteWrapper(data, length);
+      source_wrapper_.reset(resource);
+      wrapper = isolate_->factory()
+                    ->NewExternalStringFromOneByte(resource)
+                    .ToHandleChecked();
+    } else {
+      ExternalTwoByteString::Resource* resource =
+          new TwoByteWrapper(data, length);
+      source_wrapper_.reset(resource);
+      wrapper = isolate_->factory()
+                    ->NewExternalStringFromTwoByte(resource)
+                    .ToHandleChecked();
+    }
+    wrapper_ =
+        Handle<String>::cast(isolate_->global_handles()->Create(*wrapper));
+
+    character_stream_.reset(
+        ScannerStream::For(wrapper_, shared_->start_position() - offset,
+                           shared_->end_position() - offset));
   }
   parse_info_.reset(new ParseInfo(zone_.get()));
   parse_info_->set_isolate(isolate_);
@@ -111,8 +205,6 @@ void CompilerDispatcherJob::PrepareToParseOnMainThread() {
 }
 
 void CompilerDispatcherJob::Parse() {
-  DCHECK(can_parse_on_background_thread_ ||
-         ThreadId::Current().Equals(isolate_->thread_id()));
   DCHECK(status() == CompileJobStatus::kReadyToParse);
   COMPILER_DISPATCHER_TRACE_SCOPE_WITH_NUM(
       tracer_, kParse,
@@ -123,12 +215,7 @@ void CompilerDispatcherJob::Parse() {
 
   DisallowHeapAllocation no_allocation;
   DisallowHandleAllocation no_handles;
-  std::unique_ptr<DisallowHandleDereference> no_deref;
-  // If we can't parse on a background thread, we need to be able to deref the
-  // source string.
-  if (can_parse_on_background_thread_) {
-    no_deref.reset(new DisallowHandleDereference());
-  }
+  DisallowHandleDereference no_deref;
 
   // Nullify the Isolate temporarily so that the parser doesn't accidentally
   // use it.
@@ -157,6 +244,10 @@ bool CompilerDispatcherJob::FinalizeParsingOnMainThread() {
     i::GlobalHandles::Destroy(Handle<Object>::cast(source_).location());
     source_ = Handle<String>::null();
   }
+  if (!wrapper_.is_null()) {
+    i::GlobalHandles::Destroy(Handle<Object>::cast(wrapper_).location());
+    wrapper_ = Handle<String>::null();
+  }
 
   if (parse_info_->literal() == nullptr) {
     status_ = CompileJobStatus::kFailed;
@@ -217,16 +308,13 @@ bool CompilerDispatcherJob::PrepareToCompileOnMainThread() {
     return false;
   }
 
-  can_compile_on_background_thread_ =
-      compile_job_->can_execute_on_background_thread();
+  CHECK(compile_job_->can_execute_on_background_thread());
   status_ = CompileJobStatus::kReadyToCompile;
   return true;
 }
 
 void CompilerDispatcherJob::Compile() {
   DCHECK(status() == CompileJobStatus::kReadyToCompile);
-  DCHECK(can_compile_on_background_thread_ ||
-         ThreadId::Current().Equals(isolate_->thread_id()));
   COMPILER_DISPATCHER_TRACE_SCOPE_WITH_NUM(
       tracer_, kCompile, parse_info_->literal()->ast_node_count());
   if (trace_compiler_dispatcher_jobs_) {
@@ -293,6 +381,10 @@ void CompilerDispatcherJob::ResetOnMainThread() {
     i::GlobalHandles::Destroy(Handle<Object>::cast(source_).location());
     source_ = Handle<String>::null();
   }
+  if (!wrapper_.is_null()) {
+    i::GlobalHandles::Destroy(Handle<Object>::cast(wrapper_).location());
+    wrapper_ = Handle<String>::null();
+  }
 
   status_ = CompileJobStatus::kInitial;
 }

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

@@ -7,6 +7,7 @@
 
 #include <memory>
 
+#include "include/v8.h"
 #include "src/base/macros.h"
 #include "src/globals.h"
 #include "src/handles.h"
@@ -46,14 +47,6 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
   ~CompilerDispatcherJob();
 
   CompileJobStatus status() const { return status_; }
-  bool can_parse_on_background_thread() const {
-    return can_parse_on_background_thread_;
-  }
-  // Should only be called after kReadyToCompile.
-  bool can_compile_on_background_thread() const {
-    DCHECK(compile_job_.get());
-    return can_compile_on_background_thread_;
-  }
 
   // Returns true if this CompilerDispatcherJob was created for the given
   // function.
@@ -98,6 +91,8 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
   CompilerDispatcherTracer* tracer_;
   Handle<SharedFunctionInfo> shared_;  // Global handle.
   Handle<String> source_;        // Global handle.
+  Handle<String> wrapper_;       // Global handle.
+  std::unique_ptr<v8::String::ExternalStringResourceBase> source_wrapper_;
   size_t max_stack_size_;
 
   // Members required for parsing.
@@ -112,9 +107,6 @@ class V8_EXPORT_PRIVATE CompilerDispatcherJob {
   std::unique_ptr<CompilationInfo> compile_info_;
   std::unique_ptr<CompilationJob> compile_job_;
 
-  bool can_parse_on_background_thread_;
-  bool can_compile_on_background_thread_;
-
   bool trace_compiler_dispatcher_jobs_;
 
   DISALLOW_COPY_AND_ASSIGN(CompilerDispatcherJob);

src/compiler-dispatcher/compiler-dispatcher.cc

@@ -68,10 +68,8 @@ bool IsFinished(CompilerDispatcherJob* job) {
 }
 
 bool CanRunOnAnyThread(CompilerDispatcherJob* job) {
-  return (job->status() == CompileJobStatus::kReadyToParse &&
-          job->can_parse_on_background_thread()) ||
-         (job->status() == CompileJobStatus::kReadyToCompile &&
-          job->can_compile_on_background_thread());
+  return job->status() == CompileJobStatus::kReadyToParse ||
+         job->status() == CompileJobStatus::kReadyToCompile;
 }
 
 void DoNextStepOnBackgroundThread(CompilerDispatcherJob* job) {
@@ -229,6 +227,8 @@ CompilerDispatcher::~CompilerDispatcher() {
 bool CompilerDispatcher::Enqueue(Handle<SharedFunctionInfo> function) {
   if (!IsEnabled()) return false;
 
+  DCHECK(FLAG_ignition);
+
   if (memory_pressure_level_.Value() != MemoryPressureLevel::kNone) {
     return false;
   }

src/compiler-dispatcher/compiler-dispatcher.h

@@ -96,13 +96,11 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
  private:
   FRIEND_TEST(CompilerDispatcherTest, EnqueueAndStep);
   FRIEND_TEST(CompilerDispatcherTest, IdleTaskSmallIdleTime);
-  FRIEND_TEST(IgnitionCompilerDispatcherTest, CompileOnBackgroundThread);
-  FRIEND_TEST(IgnitionCompilerDispatcherTest, FinishNowWithBackgroundTask);
-  FRIEND_TEST(IgnitionCompilerDispatcherTest,
-              AsyncAbortAllPendingBackgroundTask);
-  FRIEND_TEST(IgnitionCompilerDispatcherTest,
-              AsyncAbortAllRunningBackgroundTask);
-  FRIEND_TEST(IgnitionCompilerDispatcherTest, FinishNowDuringAbortAll);
+  FRIEND_TEST(CompilerDispatcherTest, CompileOnBackgroundThread);
+  FRIEND_TEST(CompilerDispatcherTest, FinishNowWithBackgroundTask);
+  FRIEND_TEST(CompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask);
+  FRIEND_TEST(CompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask);
+  FRIEND_TEST(CompilerDispatcherTest, FinishNowDuringAbortAll);
 
   typedef std::multimap<std::pair<int, int>,
                         std::unique_ptr<CompilerDispatcherJob>>

src/objects.h

@@ -9596,7 +9596,6 @@ class String: public Name {
   // returned structure will report so, and can't provide a vector of either
   // kind.
   FlatContent GetFlatContent();
-  FlatContent GetFlattenedContent();
 
   // Returns the parent of a sliced string or first part of a flat cons string.
   // Requires: StringShape(this).IsIndirect() && this->IsFlat()

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

@@ -29,17 +29,6 @@ class CompilerDispatcherJobTest : public TestWithContext {
 
   CompilerDispatcherTracer* tracer() { return &tracer_; }
 
- private:
-  CompilerDispatcherTracer tracer_;
-
-  DISALLOW_COPY_AND_ASSIGN(CompilerDispatcherJobTest);
-};
-
-class IgnitionCompilerDispatcherJobTest : public CompilerDispatcherJobTest {
- public:
-  IgnitionCompilerDispatcherJobTest() {}
-  ~IgnitionCompilerDispatcherJobTest() override {}
-
   static void SetUpTestCase() {
     old_flag_ = i::FLAG_ignition;
     i::FLAG_ignition = true;
@@ -52,11 +41,13 @@ class IgnitionCompilerDispatcherJobTest : public CompilerDispatcherJobTest {
   }
 
  private:
+  CompilerDispatcherTracer tracer_;
   static bool old_flag_;
-  DISALLOW_COPY_AND_ASSIGN(IgnitionCompilerDispatcherJobTest);
+
+  DISALLOW_COPY_AND_ASSIGN(CompilerDispatcherJobTest);
 };
 
-bool IgnitionCompilerDispatcherJobTest::old_flag_;
+bool CompilerDispatcherJobTest::old_flag_;
 
 namespace {
 
@@ -110,22 +101,6 @@ TEST_F(CompilerDispatcherJobTest, Construct) {
       FLAG_stack_size));
 }
 
-TEST_F(CompilerDispatcherJobTest, CanParseOnBackgroundThread) {
-  {
-    std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
-        i_isolate(), tracer(), CreateSharedFunctionInfo(i_isolate(), nullptr),
-        FLAG_stack_size));
-    ASSERT_FALSE(job->can_parse_on_background_thread());
-  }
-  {
-    ScriptResource script(test_script, strlen(test_script));
-    std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
-        i_isolate(), tracer(), CreateSharedFunctionInfo(i_isolate(), &script),
-        FLAG_stack_size));
-    ASSERT_TRUE(job->can_parse_on_background_thread());
-  }
-}
-
 TEST_F(CompilerDispatcherJobTest, StateTransitions) {
   std::unique_ptr<CompilerDispatcherJob> job(new CompilerDispatcherJob(
       i_isolate(), tracer(), CreateSharedFunctionInfo(i_isolate(), nullptr),
@@ -291,7 +266,7 @@ class CompileTask : public Task {
   DISALLOW_COPY_AND_ASSIGN(CompileTask);
 };
 
-TEST_F(IgnitionCompilerDispatcherJobTest, CompileOnBackgroundThread) {
+TEST_F(CompilerDispatcherJobTest, CompileOnBackgroundThread) {
   const char* raw_script =
       "(a, b) {\n"
       "  var c = a + b;\n"
@@ -308,7 +283,6 @@ TEST_F(IgnitionCompilerDispatcherJobTest, CompileOnBackgroundThread) {
   job->Parse();
   job->FinalizeParsingOnMainThread();
   job->PrepareToCompileOnMainThread();
-  ASSERT_TRUE(job->can_compile_on_background_thread());
 
   base::Semaphore semaphore(0);
   CompileTask* background_task = new CompileTask(job.get(), &semaphore);

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

@@ -27,44 +27,26 @@ class CompilerDispatcherTest : public TestWithContext {
   static void SetUpTestCase() {
     old_flag_ = i::FLAG_ignition;
     i::FLAG_compiler_dispatcher = true;
+    old_ignition_flag_ = i::FLAG_ignition;
+    i::FLAG_ignition = true;
     TestWithContext::SetUpTestCase();
   }
 
   static void TearDownTestCase() {
     TestWithContext::TearDownTestCase();
     i::FLAG_compiler_dispatcher = old_flag_;
+    i::FLAG_ignition = old_ignition_flag_;
   }
 
  private:
   static bool old_flag_;
+  static bool old_ignition_flag_;
 
   DISALLOW_COPY_AND_ASSIGN(CompilerDispatcherTest);
 };
 
 bool CompilerDispatcherTest::old_flag_;
-
-class IgnitionCompilerDispatcherTest : public CompilerDispatcherTest {
- public:
-  IgnitionCompilerDispatcherTest() = default;
-  ~IgnitionCompilerDispatcherTest() override = default;
-
-  static void SetUpTestCase() {
-    old_flag_ = i::FLAG_ignition;
-    i::FLAG_ignition = true;
-    CompilerDispatcherTest::SetUpTestCase();
-  }
-
-  static void TearDownTestCase() {
-    CompilerDispatcherTest::TearDownTestCase();
-    i::FLAG_ignition = old_flag_;
-  }
-
- private:
-  static bool old_flag_;
-  DISALLOW_COPY_AND_ASSIGN(IgnitionCompilerDispatcherTest);
-};
-
-bool IgnitionCompilerDispatcherTest::old_flag_;
+bool CompilerDispatcherTest::old_ignition_flag_;
 
 namespace {
 
@@ -373,7 +355,7 @@ TEST_F(CompilerDispatcherTest, IdleTaskException) {
   ASSERT_FALSE(i_isolate()->has_pending_exception());
 }
 
-TEST_F(IgnitionCompilerDispatcherTest, CompileOnBackgroundThread) {
+TEST_F(CompilerDispatcherTest, CompileOnBackgroundThread) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
 
@@ -418,7 +400,7 @@ TEST_F(IgnitionCompilerDispatcherTest, CompileOnBackgroundThread) {
   ASSERT_FALSE(platform.IdleTaskPending());
 }
 
-TEST_F(IgnitionCompilerDispatcherTest, FinishNowWithBackgroundTask) {
+TEST_F(CompilerDispatcherTest, FinishNowWithBackgroundTask) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
 
@@ -517,7 +499,7 @@ TEST_F(CompilerDispatcherTest, FinishNowException) {
   platform.ClearIdleTask();
 }
 
-TEST_F(IgnitionCompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask) {
+TEST_F(CompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
 
@@ -561,7 +543,7 @@ TEST_F(IgnitionCompilerDispatcherTest, AsyncAbortAllPendingBackgroundTask) {
   ASSERT_FALSE(platform.ForegroundTasksPending());
 }
 
-TEST_F(IgnitionCompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask) {
+TEST_F(CompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
 
@@ -645,7 +627,7 @@ TEST_F(IgnitionCompilerDispatcherTest, AsyncAbortAllRunningBackgroundTask) {
   platform.ClearIdleTask();
 }
 
-TEST_F(IgnitionCompilerDispatcherTest, FinishNowDuringAbortAll) {
+TEST_F(CompilerDispatcherTest, FinishNowDuringAbortAll) {
   MockPlatform platform;
   CompilerDispatcher dispatcher(i_isolate(), &platform, FLAG_stack_size);
 
@@ -816,7 +798,8 @@ TEST_F(CompilerDispatcherTest, EnqueueAndStep) {
 
   ASSERT_TRUE(platform.IdleTaskPending());
   platform.ClearIdleTask();
-  ASSERT_FALSE(platform.BackgroundTasksPending());
+  ASSERT_TRUE(platform.BackgroundTasksPending());
+  platform.ClearBackgroundTasks();
 }
 
 }  // namespace internal