[compiler] Have one unique PersistentHandles container

The (now unique)PersistentHandles container follows this path:
  1) PersistentHandles created via PersistentHandlesScope inside of
     CompilationHandleScope
  2) Owned by OptimizedCompilationInfo
  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.

There is a special case in GenerateCodeForTesting where the JSHeapBroker
will not be retired in that same method. In this case, we need to
re-attach the PersistentHandles container to the JSHeapBroker.

The identity map of the persistent & canonical handles also gets passed
around like the persistent handles. The only difference is that is
created in the CanonicalHandleScope (i.e step 1) is different).

Bug: v8:7790
Change-Id: I2da77a7e08f3fd360a46b606c5fbda08c0af27df
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2332811
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69360}

src/codegen/compiler.cc

@@ -152,23 +152,6 @@ class CompilerTracer : public AllStatic {
 
 }  // namespace
 
-// A wrapper around a OptimizedCompilationInfo that detaches the Handles from
-// the underlying PersistentHandlesScope and stores them in info_ on
-// destruction.
-class CompilationHandleScope final {
- public:
-  explicit CompilationHandleScope(Isolate* isolate,
-                                  OptimizedCompilationInfo* info)
-      : persistent_(isolate), info_(info) {}
-  ~CompilationHandleScope() {
-    info_->set_persistent_handles(persistent_.Detach());
-  }
-
- private:
-  PersistentHandlesScope persistent_;
-  OptimizedCompilationInfo* info_;
-};
-
 // Helper that times a scoped region and records the elapsed time.
 struct ScopedTimer {
   explicit ScopedTimer(base::TimeDelta* location) : location_(location) {
@@ -927,6 +910,17 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Code> GetCodeFromCompilationCache(
   return shared->TryGetCachedCode(isolate);
 }
 
+// Runs PrepareJob in the proper compilation & canonical scopes. Handles will be
+// allocated in a persistent handle scope that is detached and handed off to the
+// {compilation_info} after PrepareJob.
+bool PrepareJobWithHandleScope(OptimizedCompilationJob* job, Isolate* isolate,
+                               OptimizedCompilationInfo* compilation_info) {
+  CompilationHandleScope compilation(isolate, compilation_info);
+  CanonicalHandleScope canonical(isolate, compilation_info);
+  compilation_info->ReopenHandlesInNewHandleScope(isolate);
+  return job->PrepareJob(isolate) == CompilationJob::SUCCEEDED;
+}
+
 bool GetOptimizedCodeNow(OptimizedCompilationJob* job, Isolate* isolate,
                          OptimizedCompilationInfo* compilation_info) {
   TimerEventScope<TimerEventRecompileSynchronous> timer(isolate);
@@ -934,10 +928,8 @@ bool GetOptimizedCodeNow(OptimizedCompilationJob* job, Isolate* isolate,
       isolate, RuntimeCallCounterId::kOptimizeNonConcurrent);
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                "V8.OptimizeNonConcurrent");
-  CanonicalHandleScope canonical(isolate);
-  compilation_info->ReopenHandlesInNewHandleScope(isolate);
 
-  if (job->PrepareJob(isolate) != CompilationJob::SUCCEEDED ||
+  if (!PrepareJobWithHandleScope(job, isolate, compilation_info) ||
       job->ExecuteJob(isolate->counters()->runtime_call_stats()) !=
           CompilationJob::SUCCEEDED ||
       job->FinalizeJob(isolate) != CompilationJob::SUCCEEDED) {
@@ -981,15 +973,8 @@ bool GetOptimizedCodeLater(std::unique_ptr<OptimizedCompilationJob> job,
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
                "V8.OptimizeConcurrentPrepare");
 
-  {
-    // All handles in this scope will be allocated in a persistent handle
-    // scope that is detached and handed off to the background thread before
-    // queuing it.
-    CompilationHandleScope compilation(isolate, compilation_info);
-    CanonicalHandleScope canonical(isolate);
-    compilation_info->ReopenHandlesInNewHandleScope(isolate);
-    if (job->PrepareJob(isolate) != CompilationJob::SUCCEEDED) return false;
-  }
+  if (!PrepareJobWithHandleScope(job.get(), isolate, compilation_info))
+    return false;
 
   // The background recompile will own this job.
   isolate->optimizing_compile_dispatcher()->QueueForOptimization(job.get());
@@ -1364,6 +1349,10 @@ MaybeHandle<SharedFunctionInfo> CompileToplevel(
 
 }  // namespace
 
+CompilationHandleScope::~CompilationHandleScope() {
+  info_->set_persistent_handles(persistent_.Detach());
+}
+
 BackgroundCompileTask::BackgroundCompileTask(ScriptStreamingData* streamed_data,
                                              Isolate* isolate)
     : flags_(UnoptimizedCompileFlags::ForToplevelCompile(

src/codegen/compiler.h

@@ -13,6 +13,7 @@
 #include "src/common/globals.h"
 #include "src/execution/isolate.h"
 #include "src/execution/off-thread-isolate.h"
+#include "src/handles/persistent-handles.h"
 #include "src/logging/code-events.h"
 #include "src/objects/contexts.h"
 #include "src/parsing/parse-info.h"
@@ -444,6 +445,21 @@ class DeferredFinalizationJobData {
   std::unique_ptr<UnoptimizedCompilationJob> job_;
 };
 
+// A wrapper around a OptimizedCompilationInfo that detaches the Handles from
+// the underlying PersistentHandlesScope and stores them in info_ on
+// destruction.
+class CompilationHandleScope final {
+ public:
+  explicit CompilationHandleScope(Isolate* isolate,
+                                  OptimizedCompilationInfo* info)
+      : persistent_(isolate), info_(info) {}
+  ~CompilationHandleScope();
+
+ private:
+  PersistentHandlesScope persistent_;
+  OptimizedCompilationInfo* info_;
+};
+
 using DeferredFinalizationJobDataList =
     std::vector<DeferredFinalizationJobData>;
 

src/codegen/optimized-compilation-info.cc

@@ -28,7 +28,6 @@ OptimizedCompilationInfo::OptimizedCompilationInfo(
   bytecode_array_ = handle(shared->GetBytecodeArray(), isolate);
   shared_info_ = shared;
   closure_ = closure;
-  ph_ = isolate->NewPersistentHandles();
 
   // Collect source positions for optimized code when profiling or if debugger
   // is active, to be able to get more precise source positions at the price of
@@ -126,12 +125,6 @@ OptimizedCompilationInfo::~OptimizedCompilationInfo() {
   }
 }
 
-void OptimizedCompilationInfo::set_persistent_handles(
-    std::unique_ptr<PersistentHandles> persistent_handles) {
-  DCHECK_NULL(persistent_handles_);
-  persistent_handles_ = std::move(persistent_handles);
-}
-
 void OptimizedCompilationInfo::ReopenHandlesInNewHandleScope(Isolate* isolate) {
   if (!shared_info_.is_null()) {
     shared_info_ = Handle<SharedFunctionInfo>(*shared_info_, isolate);

src/codegen/optimized-compilation-info.h

@@ -16,6 +16,7 @@
 #include "src/handles/handles.h"
 #include "src/handles/persistent-handles.h"
 #include "src/objects/objects.h"
+#include "src/utils/identity-map.h"
 #include "src/utils/utils.h"
 #include "src/utils/vector.h"
 
@@ -160,7 +161,18 @@ class V8_EXPORT_PRIVATE OptimizedCompilationInfo final {
   }
 
   void set_persistent_handles(
-      std::unique_ptr<PersistentHandles> persistent_handles);
+      std::unique_ptr<PersistentHandles> persistent_handles) {
+    DCHECK_NULL(ph_);
+    ph_ = std::move(persistent_handles);
+    DCHECK_NOT_NULL(ph_);
+  }
+
+  void set_canonical_handles(
+      std::unique_ptr<CanonicalHandlesMap> canonical_handles) {
+    DCHECK_NULL(canonical_handles_);
+    canonical_handles_ = std::move(canonical_handles);
+    DCHECK_NOT_NULL(canonical_handles_);
+  }
 
   void ReopenHandlesInNewHandleScope(Isolate* isolate);
 
@@ -223,9 +235,15 @@ class V8_EXPORT_PRIVATE OptimizedCompilationInfo final {
   }
 
   std::unique_ptr<PersistentHandles> DetachPersistentHandles() {
+    DCHECK_NOT_NULL(ph_);
     return std::move(ph_);
   }
 
+  std::unique_ptr<CanonicalHandlesMap> DetachCanonicalHandles() {
+    DCHECK_NOT_NULL(canonical_handles_);
+    return std::move(canonical_handles_);
+  }
+
  private:
   void ConfigureFlags();
 
@@ -264,8 +282,6 @@ class V8_EXPORT_PRIVATE OptimizedCompilationInfo final {
   // OptimizedCompilationInfo allocates.
   Zone* const zone_;
 
-  std::unique_ptr<PersistentHandles> persistent_handles_;
-
   BailoutReason bailout_reason_ = BailoutReason::kNoReason;
 
   InlinedFunctionList inlined_functions_;
@@ -282,15 +298,26 @@ class V8_EXPORT_PRIVATE OptimizedCompilationInfo final {
 
   TickCounter tick_counter_;
 
-  // This PersistentHandles container is owned first by
-  // OptimizedCompilationInfo, then by JSHeapBroker, then by LocalHeap (when we
-  // go to the background thread), then again by JSHeapBroker (right before
-  // returning to the main thread), which gets destroyed when PipelineData gets
-  // destroyed when e.g. PipelineCompilationJob gets destroyed. Since it is a
-  // member of OptimizedCompilationInfo, we make sure that we have one and only
-  // one per compilation job.
+  // 1) PersistentHandles created via PersistentHandlesScope inside of
+  //    CompilationHandleScope
+  // 2) Owned by OptimizedCompilationInfo
+  // 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
+  // will not be retired in that same method. In this case, we need to re-attach
+  // the PersistentHandles container to the JSHeapBroker.
   std::unique_ptr<PersistentHandles> ph_;
 
+  // Canonical handles follow the same path as described by the persistent
+  // handles above. The only difference is that is created in the
+  // CanonicalHandleScope(i.e step 1) is different).
+  std::unique_ptr<CanonicalHandlesMap> canonical_handles_;
+
   DISALLOW_COPY_AND_ASSIGN(OptimizedCompilationInfo);
 };
 

src/compiler/js-heap-broker.cc

@@ -14,6 +14,7 @@
 #include "src/api/api-inl.h"
 #include "src/ast/modules.h"
 #include "src/codegen/code-factory.h"
+#include "src/codegen/optimized-compilation-info.h"
 #include "src/compiler/access-info.h"
 #include "src/compiler/bytecode-analysis.h"
 #include "src/compiler/graph-reducer.h"
@@ -1613,7 +1614,7 @@ class BytecodeArrayData : public FixedArrayBaseData {
       constant_pool_.push_back(broker->GetOrCreateData(constant_pool->get(i)));
     }
 
-    source_positions_ = broker->NewPersistentHandle(
+    source_positions_ = broker->CanonicalPersistentHandle(
         bytecode_array->SourcePositionTableIfCollected());
 
     Handle<ByteArray> handlers(bytecode_array->handler_table(),
@@ -2435,6 +2436,27 @@ JSHeapBroker::JSHeapBroker(
 
 JSHeapBroker::~JSHeapBroker() { DCHECK(!local_heap_); }
 
+void JSHeapBroker::SetPersistentAndCopyCanonicalHandlesForTesting(
+    std::unique_ptr<PersistentHandles> persistent_handles,
+    std::unique_ptr<CanonicalHandlesMap> canonical_handles) {
+  set_persistent_handles(std::move(persistent_handles));
+  CopyCanonicalHandlesForTesting(std::move(canonical_handles));
+}
+
+void JSHeapBroker::CopyCanonicalHandlesForTesting(
+    std::unique_ptr<CanonicalHandlesMap> canonical_handles) {
+  DCHECK_NULL(canonical_handles_);
+  canonical_handles_ = std::make_unique<CanonicalHandlesMap>(
+      isolate_->heap(), ZoneAllocationPolicy(zone()));
+
+  CanonicalHandlesMap::IteratableScope it_scope(canonical_handles.get());
+  for (auto it = it_scope.begin(); it != it_scope.end(); ++it) {
+    Address* entry = *it.entry();
+    Object key = it.key();
+    canonical_handles_->Set(key, entry);
+  }
+}
+
 std::string JSHeapBroker::Trace() const {
   std::ostringstream oss;
   oss << "[" << this << "] ";
@@ -2442,17 +2464,20 @@ std::string JSHeapBroker::Trace() const {
   return oss.str();
 }
 
-void JSHeapBroker::InitializeLocalHeap() {
-  DCHECK(ph_);
+void JSHeapBroker::InitializeLocalHeap(OptimizedCompilationInfo* info) {
+  set_persistent_handles(info->DetachPersistentHandles());
+  set_canonical_handles(info->DetachCanonicalHandles());
   DCHECK(!local_heap_);
   local_heap_.emplace(isolate_->heap(), std::move(ph_));
 }
 
-void JSHeapBroker::TearDownLocalHeap() {
-  DCHECK(!ph_);
+void JSHeapBroker::TearDownLocalHeap(OptimizedCompilationInfo* info) {
+  DCHECK_NULL(ph_);
   DCHECK(local_heap_);
   ph_ = local_heap_->DetachPersistentHandles();
   local_heap_.reset();
+  info->set_canonical_handles(DetachCanonicalHandles());
+  info->set_persistent_handles(DetachPersistentHandles());
 }
 
 void JSHeapBroker::StopSerializing() {

src/compiler/js-heap-broker.h

@@ -21,6 +21,7 @@
 #include "src/objects/function-kind.h"
 #include "src/objects/objects.h"
 #include "src/utils/address-map.h"
+#include "src/utils/identity-map.h"
 #include "src/utils/ostreams.h"
 #include "src/zone/zone-containers.h"
 
@@ -107,8 +108,12 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
 
   enum BrokerMode { kDisabled, kSerializing, kSerialized, kRetired };
   BrokerMode mode() const { return mode_; }
-  void InitializeLocalHeap();
-  void TearDownLocalHeap();
+  // Initialize the local heap with the persistent and canonical handles
+  // provided by {info}.
+  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);
   void StopSerializing();
   void Retire();
   bool SerializingAllowed() const;
@@ -215,20 +220,59 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
   bool IsSerializedForCompilation(const SharedFunctionInfoRef& shared,
                                   const FeedbackVectorRef& feedback) const;
 
+  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.
+  // If we have the canonical map, we can create the canonical & persistent
+  // handle through it. This commonly happens during the Execute phase.
+  // If we don't, that means we are calling this method from serialization. If
+  // that happens, we should be inside a canonical and a persistent handle
+  // scope. Then, we would just use the regular handle creation.
   template <typename T>
-  Handle<T> NewPersistentHandle(T obj) {
-    return ph_->NewHandle(obj);
+  Handle<T> CanonicalPersistentHandle(T object) {
+    if (canonical_handles_) {
+      Address address = object.ptr();
+      if (Internals::HasHeapObjectTag(address)) {
+        RootIndex root_index;
+        if (root_index_map_.Lookup(address, &root_index)) {
+          return Handle<T>(isolate_->root_handle(root_index).location());
+        }
+      }
+
+      Object obj(address);
+      Address** entry = canonical_handles_->Get(obj);
+      if (*entry == nullptr) {
+        // Allocate new PersistentHandle if one wasn't created before.
+        DCHECK(local_heap_);
+        *entry = local_heap_->NewPersistentHandle(obj).location();
+      }
+      return Handle<T>(*entry);
+    } else {
+      return Handle<T>(object, isolate());
+    }
   }
 
   template <typename T>
-  Handle<T> NewPersistentHandle(Handle<T> obj) {
-    return ph_->NewHandle(*obj);
+  Handle<T> CanonicalPersistentHandle(Handle<T> object) {
+    return CanonicalPersistentHandle<T>(*object);
   }
 
-  LocalHeap* local_heap() {
-    return local_heap_.has_value() ? &(*local_heap_) : nullptr;
+  // Find the corresponding handle in the CanonicalHandlesMap. The entry must be
+  // found.
+  template <typename T>
+  Handle<T> FindCanonicalPersistentHandleForTesting(Object object) {
+    Address** entry = canonical_handles_->Find(object);
+    return Handle<T>(*entry);
   }
 
+  // Set the persistent handles and copy the canonical handles over to the
+  // JSHeapBroker.
+  void SetPersistentAndCopyCanonicalHandlesForTesting(
+      std::unique_ptr<PersistentHandles> persistent_handles,
+      std::unique_ptr<CanonicalHandlesMap> canonical_handles);
   std::string Trace() const;
   void IncrementTracingIndentation();
   void DecrementTracingIndentation();
@@ -270,6 +314,33 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
 
   PerIsolateCompilerCache* compiler_cache() const { return compiler_cache_; }
 
+  void set_persistent_handles(
+      std::unique_ptr<PersistentHandles> persistent_handles) {
+    DCHECK_NULL(ph_);
+    ph_ = std::move(persistent_handles);
+    DCHECK_NOT_NULL(ph_);
+  }
+  std::unique_ptr<PersistentHandles> DetachPersistentHandles() {
+    DCHECK_NOT_NULL(ph_);
+    return std::move(ph_);
+  }
+
+  void set_canonical_handles(
+      std::unique_ptr<CanonicalHandlesMap> canonical_handles) {
+    DCHECK_NULL(canonical_handles_);
+    canonical_handles_ = std::move(canonical_handles);
+    DCHECK_NOT_NULL(canonical_handles_);
+  }
+
+  std::unique_ptr<CanonicalHandlesMap> DetachCanonicalHandles() {
+    DCHECK_NOT_NULL(canonical_handles_);
+    return std::move(canonical_handles_);
+  }
+
+  // Copy the canonical handles over to the JSHeapBroker.
+  void CopyCanonicalHandlesForTesting(
+      std::unique_ptr<CanonicalHandlesMap> canonical_handles);
+
   Isolate* const isolate_;
   Zone* const zone_ = nullptr;
   base::Optional<NativeContextRef> target_native_context_;
@@ -284,6 +355,7 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
   bool const is_native_context_independent_;
   std::unique_ptr<PersistentHandles> ph_;
   base::Optional<LocalHeap> local_heap_;
+  std::unique_ptr<CanonicalHandlesMap> canonical_handles_;
   unsigned trace_indentation_ = 0;
   PerIsolateCompilerCache* compiler_cache_ = nullptr;
   ZoneUnorderedMap<FeedbackSource, ProcessedFeedback const*,

src/compiler/pipeline.cc

@@ -152,10 +152,10 @@ class PipelineData {
         instruction_zone_(instruction_zone_scope_.zone()),
         codegen_zone_scope_(zone_stats_, kCodegenZoneName),
         codegen_zone_(codegen_zone_scope_.zone()),
-        broker_(new JSHeapBroker(
-            isolate_, info_->zone(), info_->trace_heap_broker(),
-            is_concurrent_inlining, info->IsNativeContextIndependent(),
-            info->DetachPersistentHandles())),
+        broker_(new JSHeapBroker(isolate_, info_->zone(),
+                                 info_->trace_heap_broker(),
+                                 is_concurrent_inlining,
+                                 info->IsNativeContextIndependent(), nullptr)),
         register_allocation_zone_scope_(zone_stats_,
                                         kRegisterAllocationZoneName),
         register_allocation_zone_(register_allocation_zone_scope_.zone()),
@@ -770,19 +770,19 @@ class PipelineRunScope {
 class LocalHeapScope {
  public:
   explicit LocalHeapScope(JSHeapBroker* broker, OptimizedCompilationInfo* info)
-      : broker_(broker), tick_counter_(&info->tick_counter()) {
-    broker_->InitializeLocalHeap();
-    tick_counter_->AttachLocalHeap(broker_->local_heap());
+      : broker_(broker), info_(info) {
+    broker_->InitializeLocalHeap(info_);
+    info_->tick_counter().AttachLocalHeap(broker_->local_heap());
   }
 
   ~LocalHeapScope() {
-    tick_counter_->DetachLocalHeap();
-    broker_->TearDownLocalHeap();
+    info_->tick_counter().DetachLocalHeap();
+    broker_->TearDownLocalHeap(info_);
   }
 
  private:
   JSHeapBroker* broker_;
-  TickCounter* tick_counter_;
+  OptimizedCompilationInfo* info_;
 };
 
 // Scope that unparks the LocalHeap, if:
@@ -3041,19 +3041,43 @@ MaybeHandle<Code> Pipeline::GenerateCodeForTesting(
   Linkage linkage(Linkage::ComputeIncoming(data.instruction_zone(), info));
   Deoptimizer::EnsureCodeForDeoptimizationEntries(isolate);
 
-  pipeline.Serialize();
   {
-    LocalHeapScope local_heap_scope(data.broker(), data.info());
-    if (!pipeline.CreateGraph()) return MaybeHandle<Code>();
+    CompilationHandleScope compilation_scope(isolate, info);
+    CanonicalHandleScope canonical(isolate, info);
+    info->ReopenHandlesInNewHandleScope(isolate);
+    pipeline.Serialize();
+    // Emulating the proper pipeline, we call CreateGraph on different places
+    // (i.e before or after creating a LocalHeapScope) depending on
+    // is_concurrent_inlining.
+    if (!data.broker()->is_concurrent_inlining()) {
+      if (!pipeline.CreateGraph()) return MaybeHandle<Code>();
+    }
+  }
+
+  {
+    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>();
   }
+
+  const bool will_retire_broker = out_broker == nullptr;
+  if (!will_retire_broker) {
+    // If the broker is going to be kept alive, pass the persistent and the
+    // canonical handles containers back to the JSHeapBroker since it will
+    // outlive the OptimizedCompilationInfo.
+    data.broker()->SetPersistentAndCopyCanonicalHandlesForTesting(
+        info->DetachPersistentHandles(), info->DetachCanonicalHandles());
+  }
+
   pipeline.AssembleCode(&linkage);
   Handle<Code> code;
-  if (pipeline.FinalizeCode(out_broker == nullptr).ToHandle(&code) &&
+  if (pipeline.FinalizeCode(will_retire_broker).ToHandle(&code) &&
       pipeline.CommitDependencies(code)) {
-    if (out_broker != nullptr) *out_broker = data.ReleaseBroker();
+    if (!will_retire_broker) *out_broker = data.ReleaseBroker();
     return code;
   }
   return MaybeHandle<Code>();

src/handles/handles.cc

@@ -6,6 +6,7 @@
 
 #include "src/api/api.h"
 #include "src/base/logging.h"
+#include "src/codegen/optimized-compilation-info.h"
 #include "src/handles/maybe-handles.h"
 #include "src/objects/objects-inl.h"
 #include "src/roots/roots-inl.h"
@@ -131,20 +132,34 @@ Address HandleScope::current_limit_address(Isolate* isolate) {
   return reinterpret_cast<Address>(&isolate->handle_scope_data()->limit);
 }
 
-CanonicalHandleScope::CanonicalHandleScope(Isolate* isolate)
-    : isolate_(isolate), zone_(isolate->allocator(), ZONE_NAME) {
+CanonicalHandleScope::CanonicalHandleScope(Isolate* isolate,
+                                           OptimizedCompilationInfo* info)
+    : isolate_(isolate),
+      info_(info),
+      zone_(info ? info->zone() : new Zone(isolate->allocator(), ZONE_NAME)) {
   HandleScopeData* handle_scope_data = isolate_->handle_scope_data();
   prev_canonical_scope_ = handle_scope_data->canonical_scope;
   handle_scope_data->canonical_scope = this;
   root_index_map_ = new RootIndexMap(isolate);
-  identity_map_ = new IdentityMap<Address*, ZoneAllocationPolicy>(
-      isolate->heap(), ZoneAllocationPolicy(&zone_));
+  identity_map_ = std::make_unique<CanonicalHandlesMap>(
+      isolate->heap(), ZoneAllocationPolicy(zone_));
   canonical_level_ = handle_scope_data->level;
 }
 
 CanonicalHandleScope::~CanonicalHandleScope() {
   delete root_index_map_;
-  delete identity_map_;
+  if (info_) {
+    // If we passed a compilation info as parameter, we created the identity map
+    // on its zone(). Then, we pass it to the compilation info which is
+    // responsible for the disposal.
+    info_->set_canonical_handles(DetachCanonicalHandles());
+  } else {
+    // If we don't have a compilation info, we created the zone manually. To
+    // properly dispose of said zone, we need to first free the identity_map_.
+    // Then we do so manually even though identity_map_ is a unique_ptr.
+    identity_map_.reset();
+    delete zone_;
+  }
   isolate_->handle_scope_data()->canonical_scope = prev_canonical_scope_;
 }
 
@@ -169,5 +184,10 @@ Address* CanonicalHandleScope::Lookup(Address object) {
   return *entry;
 }
 
+std::unique_ptr<CanonicalHandlesMap>
+CanonicalHandleScope::DetachCanonicalHandles() {
+  return std::move(identity_map_);
+}
+
 }  // namespace internal
 }  // namespace v8

src/handles/handles.h

@@ -271,6 +271,9 @@ class HandleScope {
 template <typename V, class AllocationPolicy>
 class IdentityMap;
 class RootIndexMap;
+class OptimizedCompilationInfo;
+
+using CanonicalHandlesMap = IdentityMap<Address*, ZoneAllocationPolicy>;
 
 // A CanonicalHandleScope does not open a new HandleScope. It changes the
 // existing HandleScope so that Handles created within are canonicalized.
@@ -279,16 +282,27 @@ class RootIndexMap;
 // the same CanonicalHandleScope, but not across nested ones.
 class V8_EXPORT_PRIVATE CanonicalHandleScope final {
  public:
-  explicit CanonicalHandleScope(Isolate* isolate);
+  // If we passed a compilation info as parameter, we created the
+  // CanonicalHandlesMap on said compilation info's zone(). If so, in the
+  // CanonicalHandleScope destructor we hand off the canonical handle map to the
+  // compilation info. The compilation info is responsible for the disposal. If
+  // we don't have a compilation info, we create a zone in this constructor. To
+  // properly dispose of said zone, we need to first free the identity_map_
+  // which is done manually even though identity_map_ is a unique_ptr.
+  explicit CanonicalHandleScope(Isolate* isolate,
+                                OptimizedCompilationInfo* info = nullptr);
   ~CanonicalHandleScope();
 
  private:
   Address* Lookup(Address object);
 
+  std::unique_ptr<CanonicalHandlesMap> DetachCanonicalHandles();
+
   Isolate* isolate_;
-  Zone zone_;
+  OptimizedCompilationInfo* info_;
+  Zone* zone_;
   RootIndexMap* root_index_map_;
-  IdentityMap<Address*, ZoneAllocationPolicy>* identity_map_;
+  std::unique_ptr<CanonicalHandlesMap> identity_map_;
   // Ordinary nested handle scopes within the current one are not canonical.
   int canonical_level_;
   // We may have nested canonical scopes. Handles are canonical within each one.

src/heap/local-heap-inl.h

@@ -12,19 +12,6 @@
 namespace v8 {
 namespace internal {
 
-template <typename T>
-Handle<T> LocalHeap::NewPersistentHandle(T object) {
-  if (!persistent_handles_) {
-    EnsurePersistentHandles();
-  }
-  return persistent_handles_->NewHandle(object);
-}
-
-template <typename T>
-Handle<T> LocalHeap::NewPersistentHandle(Handle<T> object) {
-  return NewPersistentHandle(*object);
-}
-
 AllocationResult LocalHeap::AllocateRaw(int size_in_bytes, AllocationType type,
                                         AllocationOrigin origin,
                                         AllocationAlignment alignment) {

src/heap/local-heap.h

@@ -44,9 +44,18 @@ class V8_EXPORT_PRIVATE LocalHeap {
   LocalHandles* handles() { return handles_.get(); }
 
   template <typename T>
-  inline Handle<T> NewPersistentHandle(T object);
+  Handle<T> NewPersistentHandle(T object) {
+    if (!persistent_handles_) {
+      EnsurePersistentHandles();
+    }
+    return persistent_handles_->NewHandle(object);
+  }
+
   template <typename T>
-  inline Handle<T> NewPersistentHandle(Handle<T> object);
+  Handle<T> NewPersistentHandle(Handle<T> object) {
+    return NewPersistentHandle(*object);
+  }
+
   std::unique_ptr<PersistentHandles> DetachPersistentHandles();
 #ifdef DEBUG
   bool ContainsPersistentHandle(Address* location);

test/cctest/compiler/serializer-tester.cc

@@ -45,6 +45,9 @@ SerializerTester::SerializerTester(const char* source)
                    i::OptimizedCompilationInfo::kSplitting |
                    i::OptimizedCompilationInfo::kAnalyzeEnvironmentLiveness;
   Optimize(function, main_zone(), main_isolate(), flags, &broker_);
+  // Update handle to the corresponding serialized Handle in the broker.
+  function =
+      broker_->FindCanonicalPersistentHandleForTesting<JSFunction>(*function);
   function_ = JSFunctionRef(broker(), function);
 }
 
@@ -77,10 +80,16 @@ void CheckForSerializedInlinee(const char* source, int argc = 0,
       "The return value of the outer function must be a function too");
   Handle<JSFunction> g_func = Handle<JSFunction>::cast(g);
 
-  SharedFunctionInfoRef g_sfi(tester.broker(),
-                              handle(g_func->shared(), tester.isolate()));
-  FeedbackVectorRef g_fv(tester.broker(),
-                         handle(g_func->feedback_vector(), tester.isolate()));
+  // Look up corresponding serialized Handles in the broker.
+  Handle<SharedFunctionInfo> sfi(
+      tester.broker()
+          ->FindCanonicalPersistentHandleForTesting<SharedFunctionInfo>(
+              g_func->shared()));
+  SharedFunctionInfoRef g_sfi(tester.broker(), sfi);
+  Handle<FeedbackVector> fv(
+      tester.broker()->FindCanonicalPersistentHandleForTesting<FeedbackVector>(
+          g_func->feedback_vector()));
+  FeedbackVectorRef g_fv(tester.broker(), fv);
   CHECK(tester.broker()->IsSerializedForCompilation(g_sfi, g_fv));
 }