Revert "[compiler, heap] Create LocalHeap outside of ExecuteJob"

This reverts commit 44708a5b6f.

Reason for revert: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/33692

Original change's description:
> [compiler, heap] Create LocalHeap outside of ExecuteJob
>
> Create LocalHeap directly in the Task or in GetOptimizedCodeNow and
> pass its reference as argument to ExecuteJob. This allows us to create
> LocalHeap differently for the main and background thread, e.g. by
> passing an additional argument to the constructor in the future.
> It will be required in the future anyways when the main thread will
> have its own LocalHeap/LocalIsolate.
>
> Extending the scope of LocalHeap, also made
> HandleBase::IsDereferenceAllowed more precise and uncovered two
> potential issues: heap accesses in
> OptimizingCompileDispatcher::CompileNext and PipelineImpl::AssembleCode
> with --code-comments.
>
> LocalHeap can now be created in the parked state. Also fixed a data
> race with LocalHeap's destructor publishing write barrier entries
> without holding the lock.
>
> Bug: v8:10315
> Change-Id: I9226972601a07b87108cd66efbbb6a0d118af58d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2460818
> Commit-Queue: Georg Neis <neis@chromium.org>
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Santiago Aboy Solanes <solanes@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70521}

TBR=ulan@chromium.org,neis@chromium.org,leszeks@chromium.org,solanes@chromium.org,dinfuehr@chromium.org

Change-Id: I9dd1f8ca6237d5716b6d8938cef0ee3f642f3166
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10315
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474118
Reviewed-by: Georg Neis <neis@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70522}

src/codegen/compiler.cc

@@ -36,7 +36,6 @@
 #include "src/heap/heap-inl.h"
 #include "src/heap/local-factory-inl.h"
 #include "src/heap/local-heap-inl.h"
-#include "src/heap/local-heap.h"
 #include "src/init/bootstrapper.h"
 #include "src/interpreter/interpreter.h"
 #include "src/logging/log-inl.h"
@@ -341,13 +340,12 @@ CompilationJob::Status OptimizedCompilationJob::PrepareJob(Isolate* isolate) {
 }
 
 CompilationJob::Status OptimizedCompilationJob::ExecuteJob(
-    RuntimeCallStats* stats, LocalIsolate* local_isolate) {
+    RuntimeCallStats* stats) {
   DisallowHeapAccess no_heap_access;
   // Delegate to the underlying implementation.
   DCHECK_EQ(state(), State::kReadyToExecute);
   ScopedTimer t(&time_taken_to_execute_);
-  return UpdateState(ExecuteJobImpl(stats, local_isolate),
-                     State::kReadyToFinalize);
+  return UpdateState(ExecuteJobImpl(stats), State::kReadyToFinalize);
 }
 
 CompilationJob::Status OptimizedCompilationJob::FinalizeJob(Isolate* isolate) {
@@ -953,21 +951,10 @@ bool GetOptimizedCodeNow(OptimizedCompilationJob* job, Isolate* isolate,
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                "V8.OptimizeNonConcurrent");
 
-  if (!PrepareJobWithHandleScope(job, isolate, compilation_info)) {
-    CompilerTracer::TraceAbortedJob(isolate, compilation_info);
-    return false;
-  }
-
-  {
-    LocalIsolate local_isolate(isolate);
-    if (job->ExecuteJob(isolate->counters()->runtime_call_stats(),
-                        &local_isolate)) {
-      CompilerTracer::TraceAbortedJob(isolate, compilation_info);
-      return false;
-    }
-  }
-
-  if (job->FinalizeJob(isolate) != CompilationJob::SUCCEEDED) {
+  if (!PrepareJobWithHandleScope(job, isolate, compilation_info) ||
+      job->ExecuteJob(isolate->counters()->runtime_call_stats()) !=
+          CompilationJob::SUCCEEDED ||
+      job->FinalizeJob(isolate) != CompilationJob::SUCCEEDED) {
     CompilerTracer::TraceAbortedJob(isolate, compilation_info);
     return false;
   }
@@ -1573,7 +1560,6 @@ void BackgroundCompileTask::Run() {
     DCHECK(info_->flags().is_toplevel());
 
     LocalIsolate isolate(isolate_for_local_isolate_);
-    UnparkedScope unparked_scope(isolate.heap());
     LocalHandleScope handle_scope(&isolate);
 
     info_->ast_value_factory()->Internalize(&isolate);

src/codegen/compiler.h

@@ -332,8 +332,7 @@ class OptimizedCompilationJob : public CompilationJob {
 
   // Executes the compile job. Can be called on a background thread if
   // can_execute_on_background_thread() returns true.
-  V8_WARN_UNUSED_RESULT Status
-  ExecuteJob(RuntimeCallStats* stats, LocalIsolate* local_isolate = nullptr);
+  V8_WARN_UNUSED_RESULT Status ExecuteJob(RuntimeCallStats* stats);
 
   // Finalizes the compile job. Must be called on the main thread.
   V8_WARN_UNUSED_RESULT Status FinalizeJob(Isolate* isolate);
@@ -358,8 +357,7 @@ class OptimizedCompilationJob : public CompilationJob {
  protected:
   // Overridden by the actual implementation.
   virtual Status PrepareJobImpl(Isolate* isolate) = 0;
-  virtual Status ExecuteJobImpl(RuntimeCallStats* stats,
-                                LocalIsolate* local_heap) = 0;
+  virtual Status ExecuteJobImpl(RuntimeCallStats* stats) = 0;
   virtual Status FinalizeJobImpl(Isolate* isolate) = 0;
 
  private:

src/codegen/optimized-compilation-info.h

@@ -302,8 +302,11 @@ class V8_EXPORT_PRIVATE OptimizedCompilationInfo final {
   // 1) PersistentHandles created via PersistentHandlesScope inside of
   //    CompilationHandleScope
   // 2) Owned by OptimizedCompilationInfo
-  // 3) Owned by the broker's LocalHeap when entering the LocalHeapScope.
-  // 4) Back to OptimizedCompilationInfo when exiting the LocalHeapScope.
+  // 3) Owned by JSHeapBroker
+  // 4) Owned by the broker's LocalHeap
+  // 5) Back to the broker for a brief moment (after tearing down the
+  //   LocalHeap as part of exiting LocalHeapScope)
+  // 6) Back to OptimizedCompilationInfo when exiting the LocalHeapScope.
   //
   // In normal execution it gets destroyed when PipelineData gets destroyed.
   // There is a special case in GenerateCodeForTesting where the JSHeapBroker

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

@@ -8,8 +8,6 @@
 #include "src/codegen/compiler.h"
 #include "src/codegen/optimized-compilation-info.h"
 #include "src/execution/isolate.h"
-#include "src/execution/local-isolate.h"
-#include "src/heap/local-heap.h"
 #include "src/init/v8.h"
 #include "src/logging/counters.h"
 #include "src/logging/log.h"
@@ -58,7 +56,6 @@ class OptimizingCompileDispatcher::CompileTask : public CancelableTask {
  private:
   // v8::Task overrides.
   void RunInternal() override {
-    LocalIsolate local_isolate(isolate_);
     DisallowHeapAllocation no_allocation;
     DisallowHandleAllocation no_handles;
     DisallowHandleDereference no_deref;
@@ -79,8 +76,8 @@ class OptimizingCompileDispatcher::CompileTask : public CancelableTask {
             dispatcher_->recompilation_delay_));
       }
 
-      dispatcher_->CompileNext(dispatcher_->NextInput(&local_isolate, true),
-                               runtime_call_stats_scope.Get(), &local_isolate);
+      dispatcher_->CompileNext(dispatcher_->NextInput(true),
+                               runtime_call_stats_scope.Get());
     }
     {
       base::MutexGuard lock_guard(&dispatcher_->ref_count_mutex_);
@@ -109,7 +106,7 @@ OptimizingCompileDispatcher::~OptimizingCompileDispatcher() {
 }
 
 OptimizedCompilationJob* OptimizingCompileDispatcher::NextInput(
-    LocalIsolate* local_isolate, bool check_if_flushing) {
+    bool check_if_flushing) {
   base::MutexGuard access_input_queue_(&input_queue_mutex_);
   if (input_queue_length_ == 0) return nullptr;
   OptimizedCompilationJob* job = input_queue_[InputQueueIndex(0)];
@@ -118,7 +115,6 @@ OptimizedCompilationJob* OptimizingCompileDispatcher::NextInput(
   input_queue_length_--;
   if (check_if_flushing) {
     if (mode_ == FLUSH) {
-      UnparkedScope scope(local_isolate->heap());
       AllowHandleDereference allow_handle_dereference;
       DisposeCompilationJob(job, true);
       return nullptr;
@@ -128,12 +124,11 @@ OptimizedCompilationJob* OptimizingCompileDispatcher::NextInput(
 }
 
 void OptimizingCompileDispatcher::CompileNext(OptimizedCompilationJob* job,
-                                              RuntimeCallStats* stats,
-                                              LocalIsolate* local_isolate) {
+                                              RuntimeCallStats* stats) {
   if (!job) return;
 
   // The function may have already been optimized by OSR.  Simply continue.
-  CompilationJob::Status status = job->ExecuteJob(stats, local_isolate);
+  CompilationJob::Status status = job->ExecuteJob(stats);
   USE(status);  // Prevent an unused-variable error.
 
   {

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

@@ -18,7 +18,6 @@
 namespace v8 {
 namespace internal {
 
-class LocalHeap;
 class OptimizedCompilationJob;
 class RuntimeCallStats;
 class SharedFunctionInfo;
@@ -59,10 +58,8 @@ class V8_EXPORT_PRIVATE OptimizingCompileDispatcher {
   enum ModeFlag { COMPILE, FLUSH };
 
   void FlushOutputQueue(bool restore_function_code);
-  void CompileNext(OptimizedCompilationJob* job, RuntimeCallStats* stats,
-                   LocalIsolate* local_isolate);
-  OptimizedCompilationJob* NextInput(LocalIsolate* local_isolate,
-                                     bool check_if_flushing = false);
+  void CompileNext(OptimizedCompilationJob* job, RuntimeCallStats* stats);
+  OptimizedCompilationJob* NextInput(bool check_if_flushing = false);
 
   inline int InputQueueIndex(int i) {
     int result = (i + input_queue_shift_) % input_queue_capacity_;

src/compiler/js-heap-broker.cc

@@ -2427,6 +2427,7 @@ JSHeapBroker::JSHeapBroker(Isolate* isolate, Zone* broker_zone,
       tracing_enabled_(tracing_enabled),
       is_concurrent_inlining_(is_concurrent_inlining),
       code_kind_(code_kind),
+      local_heap_(base::nullopt),
       feedback_(zone()),
       bytecode_analyses_(zone()),
       property_access_infos_(zone()),
@@ -2471,23 +2472,20 @@ std::string JSHeapBroker::Trace() const {
   return oss.str();
 }
 
-void JSHeapBroker::InitializeLocalHeap(OptimizedCompilationInfo* info,
-                                       LocalHeap* local_heap) {
+void JSHeapBroker::InitializeLocalHeap(OptimizedCompilationInfo* info) {
+  set_persistent_handles(info->DetachPersistentHandles());
   set_canonical_handles(info->DetachCanonicalHandles());
-  DCHECK_NULL(local_heap_);
-  local_heap_ = local_heap;
-  DCHECK_NOT_NULL(local_heap_);
-  local_heap_->AttachPersistentHandles(info->DetachPersistentHandles());
+  DCHECK(!local_heap_);
+  local_heap_.emplace(isolate_->heap(), std::move(ph_));
 }
 
 void JSHeapBroker::TearDownLocalHeap(OptimizedCompilationInfo* info) {
   DCHECK_NULL(ph_);
   DCHECK(local_heap_);
-  std::unique_ptr<PersistentHandles> ph =
-      local_heap_->DetachPersistentHandles();
-  local_heap_ = nullptr;
+  ph_ = local_heap_->DetachPersistentHandles();
+  local_heap_.reset();
   info->set_canonical_handles(DetachCanonicalHandles());
-  info->set_persistent_handles(std::move(ph));
+  info->set_persistent_handles(DetachPersistentHandles());
 }
 
 void JSHeapBroker::StopSerializing() {

src/compiler/js-heap-broker.h

@@ -117,8 +117,7 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
   BrokerMode mode() const { return mode_; }
   // Initialize the local heap with the persistent and canonical handles
   // provided by {info}.
-  void InitializeLocalHeap(OptimizedCompilationInfo* info,
-                           LocalHeap* local_heap);
+  void InitializeLocalHeap(OptimizedCompilationInfo* info);
   // Tear down the local heap and pass the persistent and canonical handles
   // provided back to {info}. {info} is responsible for disposing of them.
   void TearDownLocalHeap(OptimizedCompilationInfo* info);
@@ -228,7 +227,9 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
   bool IsSerializedForCompilation(const SharedFunctionInfoRef& shared,
                                   const FeedbackVectorRef& feedback) const;
 
-  LocalHeap* local_heap() { return local_heap_; }
+  LocalHeap* local_heap() {
+    return local_heap_.has_value() ? &(*local_heap_) : nullptr;
+  }
 
   // Return the corresponding canonical persistent handle for {object}. Create
   // one if it does not exist.
@@ -360,7 +361,7 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
   bool const is_concurrent_inlining_;
   CodeKind const code_kind_;
   std::unique_ptr<PersistentHandles> ph_;
-  LocalHeap* local_heap_ = nullptr;
+  base::Optional<LocalHeap> local_heap_;
   std::unique_ptr<CanonicalHandlesMap> canonical_handles_;
   unsigned trace_indentation_ = 0;
   PerIsolateCompilerCache* compiler_cache_ = nullptr;
@@ -452,16 +453,14 @@ class OffHeapBytecodeArray final : public interpreter::AbstractBytecodeArray {
 
 // Scope that unparks the LocalHeap, if:
 //   a) We have a JSHeapBroker,
-//   b) Said JSHeapBroker has a LocalHeap,
-//   c) Said LocalHeap has been parked and
-//   d) The given condition evaluates to true.
+//   b) Said JSHeapBroker has a LocalHeap, and
+//   c) Said LocalHeap has been parked.
 // Used, for example, when printing the graph with --trace-turbo with a
 // previously parked LocalHeap.
 class UnparkedScopeIfNeeded {
  public:
-  explicit UnparkedScopeIfNeeded(JSHeapBroker* broker,
-                                 bool extra_condition = true) {
-    if (broker != nullptr && extra_condition) {
+  explicit UnparkedScopeIfNeeded(JSHeapBroker* broker) {
+    if (broker != nullptr) {
       LocalHeap* local_heap = broker->local_heap();
       if (local_heap != nullptr && local_heap->IsParked()) {
         unparked_scope.emplace(local_heap);

src/compiler/pipeline.cc

@@ -83,7 +83,6 @@
 #include "src/diagnostics/code-tracer.h"
 #include "src/diagnostics/disassembler.h"
 #include "src/execution/isolate-inl.h"
-#include "src/heap/local-heap.h"
 #include "src/init/bootstrapper.h"
 #include "src/logging/counters.h"
 #include "src/objects/shared-function-info.h"
@@ -766,14 +765,12 @@ class PipelineRunScope {
   RuntimeCallTimerScope runtime_call_timer_scope;
 };
 
-// LocalHeapScope encapsulates the phase where persistent handles are attached
-// to the LocalHeap.
+// LocalHeapScope encapsulates the liveness of the brokers's LocalHeap.
 class LocalHeapScope {
  public:
-  explicit LocalHeapScope(JSHeapBroker* broker, OptimizedCompilationInfo* info,
-                          LocalHeap* local_heap)
+  explicit LocalHeapScope(JSHeapBroker* broker, OptimizedCompilationInfo* info)
       : broker_(broker), info_(info) {
-    broker_->InitializeLocalHeap(info_, local_heap);
+    broker_->InitializeLocalHeap(info_);
     info_->tick_counter().AttachLocalHeap(broker_->local_heap());
   }
 
@@ -1033,8 +1030,7 @@ class PipelineCompilationJob final : public OptimizedCompilationJob {
 
  protected:
   Status PrepareJobImpl(Isolate* isolate) final;
-  Status ExecuteJobImpl(RuntimeCallStats* stats,
-                        LocalIsolate* local_isolate) final;
+  Status ExecuteJobImpl(RuntimeCallStats* stats) final;
   Status FinalizeJobImpl(Isolate* isolate) final;
 
   // Registers weak object to optimized code dependencies.
@@ -1181,27 +1177,29 @@ PipelineCompilationJob::Status PipelineCompilationJob::PrepareJobImpl(
 }
 
 PipelineCompilationJob::Status PipelineCompilationJob::ExecuteJobImpl(
-    RuntimeCallStats* stats, LocalIsolate* local_isolate) {
+    RuntimeCallStats* stats) {
   // Ensure that the RuntimeCallStats table is only available during execution
   // and not during finalization as that might be on a different thread.
   PipelineJobScope scope(&data_, stats);
-  LocalHeapScope local_heap_scope(data_.broker(), data_.info(),
-                                  local_isolate->heap());
-
-  if (data_.broker()->is_concurrent_inlining()) {
-    if (!pipeline_.CreateGraph()) {
-      return AbortOptimization(BailoutReason::kGraphBuildingFailed);
+  {
+    LocalHeapScope local_heap_scope(data_.broker(), data_.info());
+    if (data_.broker()->is_concurrent_inlining()) {
+      if (!pipeline_.CreateGraph()) {
+        return AbortOptimization(BailoutReason::kGraphBuildingFailed);
+      }
     }
-  }
 
-  // We selectively Unpark inside OptimizeGraph*.
-  bool success;
-  if (compilation_info_.code_kind() == CodeKind::TURBOPROP) {
-    success = pipeline_.OptimizeGraphForMidTier(linkage_);
-  } else {
-    success = pipeline_.OptimizeGraph(linkage_);
+    // We selectively Unpark inside OptimizeGraph*.
+    ParkedScope parked_scope(data_.broker()->local_heap());
+
+    bool success;
+    if (compilation_info_.code_kind() == CodeKind::TURBOPROP) {
+      success = pipeline_.OptimizeGraphForMidTier(linkage_);
+    } else {
+      success = pipeline_.OptimizeGraph(linkage_);
+    }
+    if (!success) return FAILED;
   }
-  if (!success) return FAILED;
 
   pipeline_.AssembleCode(linkage_);
 
@@ -1285,8 +1283,7 @@ class WasmHeapStubCompilationJob final : public OptimizedCompilationJob {
 
  protected:
   Status PrepareJobImpl(Isolate* isolate) final;
-  Status ExecuteJobImpl(RuntimeCallStats* stats,
-                        LocalIsolate* local_isolate) final;
+  Status ExecuteJobImpl(RuntimeCallStats* stats) final;
   Status FinalizeJobImpl(Isolate* isolate) final;
 
  private:
@@ -1321,7 +1318,7 @@ CompilationJob::Status WasmHeapStubCompilationJob::PrepareJobImpl(
 }
 
 CompilationJob::Status WasmHeapStubCompilationJob::ExecuteJobImpl(
-    RuntimeCallStats* stats, LocalIsolate* local_isolate) {
+    RuntimeCallStats* stats) {
   std::unique_ptr<PipelineStatistics> pipeline_statistics;
   if (FLAG_turbo_stats || FLAG_turbo_stats_nvp) {
     pipeline_statistics.reset(new PipelineStatistics(
@@ -2474,7 +2471,6 @@ void PipelineImpl::Serialize() {
 
 bool PipelineImpl::CreateGraph() {
   PipelineData* data = this->data_;
-  UnparkedScopeIfNeeded unparked_scope(data->broker());
 
   data->BeginPhaseKind("V8.TFGraphCreation");
 
@@ -3026,15 +3022,13 @@ MaybeHandle<Code> Pipeline::GenerateCodeForTesting(
   }
 
   {
-    LocalIsolate local_isolate(isolate);
-    LocalHeapScope local_heap_scope(data.broker(), info, local_isolate.heap());
+    LocalHeapScope local_heap_scope(data.broker(), info);
     if (data.broker()->is_concurrent_inlining()) {
       if (!pipeline.CreateGraph()) return MaybeHandle<Code>();
     }
     // We selectively Unpark inside OptimizeGraph.
+    ParkedScope parked_scope(data.broker()->local_heap());
     if (!pipeline.OptimizeGraph(&linkage)) return MaybeHandle<Code>();
-
-    pipeline.AssembleCode(&linkage);
   }
 
   const bool will_retire_broker = out_broker == nullptr;
@@ -3046,6 +3040,7 @@ MaybeHandle<Code> Pipeline::GenerateCodeForTesting(
         info->DetachPersistentHandles(), info->DetachCanonicalHandles());
   }
 
+  pipeline.AssembleCode(&linkage);
   Handle<Code> code;
   if (pipeline.FinalizeCode(will_retire_broker).ToHandle(&code) &&
       pipeline.CommitDependencies(code)) {
@@ -3483,8 +3478,6 @@ void PipelineImpl::AssembleCode(Linkage* linkage,
   data->BeginPhaseKind("V8.TFCodeGeneration");
   data->InitializeCodeGenerator(linkage, std::move(buffer));
 
-  UnparkedScopeIfNeeded unparked_scope(data->broker(), FLAG_code_comments);
-
   Run<AssembleCodePhase>();
   if (data->info()->trace_turbo_json()) {
     TurboJsonFile json_of(data->info(), std::ios_base::app);

src/compiler/wasm-compiler.cc

@@ -7657,7 +7657,7 @@ Handle<Code> CompileCWasmEntry(Isolate* isolate, const wasm::FunctionSig* sig,
           CodeKind::C_WASM_ENTRY, std::move(name_buffer),
           AssemblerOptions::Default(isolate)));
 
-  CHECK_NE(job->ExecuteJob(isolate->counters()->runtime_call_stats(), nullptr),
+  CHECK_NE(job->ExecuteJob(isolate->counters()->runtime_call_stats()),
            CompilationJob::FAILED);
   CHECK_NE(job->FinalizeJob(isolate), CompilationJob::FAILED);
 

src/heap/concurrent-allocator.cc

@@ -9,7 +9,6 @@
 #include "src/handles/persistent-handles.h"
 #include "src/heap/concurrent-allocator-inl.h"
 #include "src/heap/local-heap-inl.h"
-#include "src/heap/local-heap.h"
 #include "src/heap/marking.h"
 #include "src/heap/memory-chunk.h"
 
@@ -19,7 +18,6 @@ namespace internal {
 void StressConcurrentAllocatorTask::RunInternal() {
   Heap* heap = isolate_->heap();
   LocalHeap local_heap(heap);
-  UnparkedScope unparked_scope(&local_heap);
 
   const int kNumIterations = 2000;
   const int kSmallObjectSize = 10 * kTaggedSize;

src/heap/local-heap.cc

@@ -27,7 +27,7 @@ LocalHeap* LocalHeap::Current() { return current_local_heap; }
 LocalHeap::LocalHeap(Heap* heap,
                      std::unique_ptr<PersistentHandles> persistent_handles)
     : heap_(heap),
-      state_(ThreadState::Parked),
+      state_(ThreadState::Running),
       safepoint_requested_(false),
       allocation_failed_(false),
       prev_(nullptr),
@@ -36,36 +36,34 @@ LocalHeap::LocalHeap(Heap* heap,
       persistent_handles_(std::move(persistent_handles)),
       marking_barrier_(new MarkingBarrier(this)),
       old_space_allocator_(this, heap->old_space()) {
-  heap_->safepoint()->AddLocalHeap(this, [this] {
-    if (FLAG_local_heaps) {
-      WriteBarrier::SetForThread(marking_barrier_.get());
-      if (heap_->incremental_marking()->IsMarking()) {
-        marking_barrier_->Activate(
-            heap_->incremental_marking()->IsCompacting());
-      }
-    }
-  });
-
+  heap_->safepoint()->AddLocalHeap(this);
   if (persistent_handles_) {
     persistent_handles_->Attach(this);
   }
   DCHECK_NULL(current_local_heap);
   current_local_heap = this;
+  // TODO(ulan): Ensure that LocalHeap cannot be created without --local-heaps.
+  if (FLAG_local_heaps) {
+    WriteBarrier::SetForThread(marking_barrier_.get());
+    if (heap_->incremental_marking()->IsMarking()) {
+      marking_barrier_->Activate(heap_->incremental_marking()->IsCompacting());
+    }
+  }
 }
 
 LocalHeap::~LocalHeap() {
+  // TODO(ulan): Ensure that LocalHeap cannot be created without --local-heaps.
+  if (FLAG_local_heaps) {
+    marking_barrier_->Publish();
+    WriteBarrier::ClearForThread(marking_barrier_.get());
+  }
   // Give up LAB before parking thread
   old_space_allocator_.FreeLinearAllocationArea();
 
   // Park thread since removing the local heap could block.
   EnsureParkedBeforeDestruction();
 
-  heap_->safepoint()->RemoveLocalHeap(this, [this] {
-    if (FLAG_local_heaps) {
-      marking_barrier_->Publish();
-      WriteBarrier::ClearForThread(marking_barrier_.get());
-    }
-  });
+  heap_->safepoint()->RemoveLocalHeap(this);
 
   DCHECK_EQ(current_local_heap, this);
   current_local_heap = nullptr;
@@ -79,13 +77,6 @@ void LocalHeap::EnsurePersistentHandles() {
   }
 }
 
-void LocalHeap::AttachPersistentHandles(
-    std::unique_ptr<PersistentHandles> persistent_handles) {
-  DCHECK_NULL(persistent_handles_);
-  persistent_handles_ = std::move(persistent_handles);
-  persistent_handles_->Attach(this);
-}
-
 std::unique_ptr<PersistentHandles> LocalHeap::DetachPersistentHandles() {
   if (persistent_handles_) persistent_handles_->Detach();
   return std::move(persistent_handles_);

src/heap/local-heap.h

@@ -22,15 +22,6 @@ class Heap;
 class Safepoint;
 class LocalHandles;
 
-// LocalHeap is used by the GC to track all threads with heap access in order to
-// stop them before performing a collection. LocalHeaps can be either Parked or
-// Running and are in Parked mode when initialized.
-//   Running: Thread is allowed to access the heap but needs to give the GC the
-//            chance to run regularly by manually invoking Safepoint(). The
-//            thread can be parked using ParkedScope.
-//   Parked:  Heap access is not allowed, so the GC will not stop this thread
-//            for a collection. Useful when threads do not need heap access for
-//            some time or for blocking operations like locking a mutex.
 class V8_EXPORT_PRIVATE LocalHeap {
  public:
   explicit LocalHeap(
@@ -79,8 +70,6 @@ class V8_EXPORT_PRIVATE LocalHeap {
     return kNullMaybeHandle;
   }
 
-  void AttachPersistentHandles(
-      std::unique_ptr<PersistentHandles> persistent_handles);
   std::unique_ptr<PersistentHandles> DetachPersistentHandles();
 #ifdef DEBUG
   bool ContainsPersistentHandle(Address* location);

src/heap/safepoint.cc

@@ -114,6 +114,23 @@ SafepointScope::SafepointScope(Heap* heap) : safepoint_(heap->safepoint()) {
 
 SafepointScope::~SafepointScope() { safepoint_->LeaveSafepointScope(); }
 
+void GlobalSafepoint::AddLocalHeap(LocalHeap* local_heap) {
+  base::MutexGuard guard(&local_heaps_mutex_);
+  if (local_heaps_head_) local_heaps_head_->prev_ = local_heap;
+  local_heap->prev_ = nullptr;
+  local_heap->next_ = local_heaps_head_;
+  local_heaps_head_ = local_heap;
+}
+
+void GlobalSafepoint::RemoveLocalHeap(LocalHeap* local_heap) {
+  base::MutexGuard guard(&local_heaps_mutex_);
+  if (local_heap->next_) local_heap->next_->prev_ = local_heap->prev_;
+  if (local_heap->prev_)
+    local_heap->prev_->next_ = local_heap->next_;
+  else
+    local_heaps_head_ = local_heap->next_;
+}
+
 bool GlobalSafepoint::ContainsLocalHeap(LocalHeap* local_heap) {
   base::MutexGuard guard(&local_heaps_mutex_);
   LocalHeap* current = local_heaps_head_;

src/heap/safepoint.h

@@ -62,36 +62,8 @@ class GlobalSafepoint {
   void EnterSafepointScope();
   void LeaveSafepointScope();
 
-  template <typename Callback>
-  void AddLocalHeap(LocalHeap* local_heap, Callback callback) {
-    // Safepoint holds this lock in order to stop threads from starting or
-    // stopping.
-    base::MutexGuard guard(&local_heaps_mutex_);
-
-    // Additional code protected from safepoint
-    callback();
-
-    // Add list to doubly-linked list
-    if (local_heaps_head_) local_heaps_head_->prev_ = local_heap;
-    local_heap->prev_ = nullptr;
-    local_heap->next_ = local_heaps_head_;
-    local_heaps_head_ = local_heap;
-  }
-
-  template <typename Callback>
-  void RemoveLocalHeap(LocalHeap* local_heap, Callback callback) {
-    base::MutexGuard guard(&local_heaps_mutex_);
-
-    // Additional code protected from safepoint
-    callback();
-
-    // Remove list from doubly-linked list
-    if (local_heap->next_) local_heap->next_->prev_ = local_heap->prev_;
-    if (local_heap->prev_)
-      local_heap->prev_->next_ = local_heap->next_;
-    else
-      local_heaps_head_ = local_heap->next_;
-  }
+  void AddLocalHeap(LocalHeap* local_heap);
+  void RemoveLocalHeap(LocalHeap* local_heap);
 
   Barrier barrier_;
   Heap* heap_;

test/cctest/heap/test-concurrent-allocation.cc

@@ -52,7 +52,6 @@ class ConcurrentAllocationThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_);
-    UnparkedScope unparked_scope(&local_heap);
 
     for (int i = 0; i < kNumIterations; i++) {
       Address address = local_heap.AllocateRawOrFail(
@@ -120,7 +119,6 @@ class LargeObjectConcurrentAllocationThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_);
-    UnparkedScope unparked_scope(&local_heap);
     const size_t kLargeObjectSize = kMaxRegularHeapObjectSize * 2;
 
     for (int i = 0; i < kNumIterations; i++) {
@@ -188,7 +186,6 @@ class ConcurrentBlackAllocationThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_);
-    UnparkedScope unparked_scope(&local_heap);
 
     for (int i = 0; i < kNumIterations; i++) {
       if (i == kWhiteIterations) {
@@ -268,7 +265,6 @@ class ConcurrentWriteBarrierThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_);
-    UnparkedScope unparked_scope(&local_heap);
     fixed_array_.set(0, value_);
   }
 
@@ -330,7 +326,6 @@ class ConcurrentRecordRelocSlotThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_);
-    UnparkedScope unparked_scope(&local_heap);
     int mode_mask = RelocInfo::EmbeddedObjectModeMask();
     for (RelocIterator it(code_, mode_mask); !it.done(); it.next()) {
       DCHECK(RelocInfo::IsEmbeddedObjectMode(it.rinfo()->rmode()));

test/cctest/heap/test-heap.cc

@@ -7184,7 +7184,6 @@ TEST(GarbageCollectionWithLocalHeap) {
   Heap* heap = CcTest::i_isolate()->heap();
 
   LocalHeap local_heap(heap);
-  UnparkedScope unparked_scope(&local_heap);
   CcTest::CollectGarbage(OLD_SPACE);
   { ParkedScope parked_scope(&local_heap); }
   CcTest::CollectGarbage(OLD_SPACE);

test/cctest/test-concurrent-descriptor-array.cc

@@ -34,7 +34,6 @@ class ConcurrentSearchThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
-    UnparkedScope unparked_scope(&local_heap);
     LocalHandleScope scope(&local_heap);
 
     for (int i = 0; i < kNumHandles; i++) {

test/cctest/test-concurrent-prototype.cc

@@ -33,7 +33,6 @@ class ConcurrentSearchThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
-    UnparkedScope unparked_scope(&local_heap);
     LocalHandleScope scope(&local_heap);
 
     for (int i = 0; i < kNumHandles; i++) {

test/cctest/test-concurrent-script-context-table.cc

@@ -35,7 +35,6 @@ class ScriptContextTableAccessUsedThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
-    UnparkedScope unparked_scope(&local_heap);
     LocalHandleScope scope(&local_heap);
 
     sema_started_->Signal();
@@ -68,7 +67,6 @@ class AccessScriptContextTableThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
-    UnparkedScope unparked_scope(&local_heap);
     LocalHandleScope scope(&local_heap);
 
     sema_started_->Signal();

test/cctest/test-concurrent-transition-array.cc

@@ -35,7 +35,6 @@ class ConcurrentSearchThread : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
-    UnparkedScope scope(&local_heap);
 
     background_thread_started_->Signal();
 
@@ -71,7 +70,6 @@ class ConcurrentSearchOnOutdatedAccessorThread final
 
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
-    UnparkedScope scope(&local_heap);
 
     TransitionsAccessor accessor(CcTest::i_isolate(), map_, true);
     background_thread_started_->Signal();

test/cctest/test-local-handles.cc

@@ -36,7 +36,6 @@ class LocalHandlesThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_);
-    UnparkedScope unparked_scope(&local_heap);
     LocalHandleScope scope(&local_heap);
 
     static constexpr int kNumHandles =
@@ -104,7 +103,6 @@ TEST(CreateLocalHandlesWithoutLocalHandleScope) {
 
   {
     LocalHeap local_heap(isolate->heap());
-    UnparkedScope scope(&local_heap);
     handle(Smi::FromInt(17), &local_heap);
   }
 }
@@ -125,7 +123,6 @@ TEST(DereferenceLocalHandle) {
   }
   {
     LocalHeap local_heap(isolate->heap(), std::move(phs));
-    UnparkedScope unparked_scope(&local_heap);
     LocalHandleScope scope(&local_heap);
     Handle<HeapNumber> local_number = handle(*ph, &local_heap);
     CHECK_EQ(42, local_number->value());

test/cctest/test-persistent-handles.cc

@@ -42,7 +42,6 @@ class PersistentHandlesThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
-    UnparkedScope unparked_scope(&local_heap);
     LocalHandleScope scope(&local_heap);
 
     for (int i = 0; i < kNumHandles; i++) {
@@ -130,7 +129,6 @@ TEST(DereferencePersistentHandle) {
   }
   {
     LocalHeap local_heap(isolate->heap(), std::move(phs));
-    UnparkedScope scope(&local_heap);
     CHECK_EQ(42, ph->value());
     DisallowHandleDereference disallow_scope;
     CHECK_EQ(42, ph->value());
@@ -142,6 +140,7 @@ TEST(NewPersistentHandleFailsWhenParked) {
   Isolate* isolate = CcTest::i_isolate();
 
   LocalHeap local_heap(isolate->heap());
+  ParkedScope scope(&local_heap);
   // Fail here in debug mode: Persistent handles can't be created if local heap
   // is parked
   local_heap.NewPersistentHandle(Smi::FromInt(1));
@@ -152,6 +151,7 @@ TEST(NewPersistentHandleFailsWhenParkedExplicit) {
   Isolate* isolate = CcTest::i_isolate();
 
   LocalHeap local_heap(isolate->heap(), isolate->NewPersistentHandles());
+  ParkedScope scope(&local_heap);
   // Fail here in debug mode: Persistent handles can't be created if local heap
   // is parked
   local_heap.NewPersistentHandle(Smi::FromInt(1));

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

@@ -10,9 +10,7 @@
 #include "src/codegen/compiler.h"
 #include "src/codegen/optimized-compilation-info.h"
 #include "src/execution/isolate.h"
-#include "src/execution/local-isolate.h"
 #include "src/handles/handles.h"
-#include "src/heap/local-heap.h"
 #include "src/objects/objects-inl.h"
 #include "src/parsing/parse-info.h"
 #include "test/unittests/test-helpers.h"
@@ -44,8 +42,7 @@ class BlockingCompilationJob : public OptimizedCompilationJob {
   // OptimiziedCompilationJob implementation.
   Status PrepareJobImpl(Isolate* isolate) override { UNREACHABLE(); }
 
-  Status ExecuteJobImpl(RuntimeCallStats* stats,
-                        LocalIsolate* local_isolate) override {
+  Status ExecuteJobImpl(RuntimeCallStats* stats) override {
     blocking_.SetValue(true);
     semaphore_.Wait();
     blocking_.SetValue(false);

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

@@ -62,8 +62,7 @@ class LocalFactoryTest : public TestWithIsolateAndZone {
                 isolate(), true, construct_language_mode(FLAG_use_strict),
                 REPLMode::kNo),
             &state_),
-        local_isolate_(isolate()),
-        unparked_scope_(local_isolate_.heap()) {
+        local_isolate_(isolate()) {
     FLAG_concurrent_allocation = true;
   }
 
@@ -115,7 +114,6 @@ class LocalFactoryTest : public TestWithIsolateAndZone {
   UnoptimizedCompileState state_;
   ParseInfo parse_info_;
   LocalIsolate local_isolate_;
-  UnparkedScope unparked_scope_;
   Handle<String> source_string_;
   Handle<Script> script_;
 };

test/unittests/heap/safepoint-unittest.cc

@@ -43,6 +43,7 @@ class ParkedThread final : public v8::base::Thread {
     LocalHeap local_heap(heap_);
 
     if (mutex_) {
+      ParkedScope scope(&local_heap);
       base::MutexGuard guard(mutex_);
     }
   }
@@ -99,7 +100,6 @@ class RunningThread final : public v8::base::Thread {
 
   void Run() override {
     LocalHeap local_heap(heap_);
-    UnparkedScope unparked_scope(&local_heap);
 
     for (int i = 0; i < kRuns; i++) {
       counter_->fetch_add(1);
@@ -148,7 +148,6 @@ TEST_F(SafepointTest, SkipLocalHeapOfThisThread) {
   EnsureFlagLocalHeapsEnabled();
   Heap* heap = i_isolate()->heap();
   LocalHeap local_heap(heap);
-  UnparkedScope unparked_scope(&local_heap);
   {
     SafepointScope scope(heap);
     local_heap.Safepoint();