[heap] Make PersistentHandles::NewHandle typed

PersistentHandles::NewHandle/LocalHeap::NewPersistentHandle currently
erase the type of the object. This patch templatizes them to preserve
the type and introduces versions that take Handle<T>

Bug: v8:10315
Change-Id: I899179a5b842b7b16144b340f6cd2b91e1db228f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2287501
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68779}

BUILD.gn

@@ -2457,6 +2457,7 @@ v8_source_set("v8_base_without_compiler") {
     "src/handles/local-handles.h",
     "src/handles/maybe-handles-inl.h",
     "src/handles/maybe-handles.h",
+    "src/handles/persistent-handles-inl.h",
     "src/handles/persistent-handles.cc",
     "src/handles/persistent-handles.h",
     "src/heap/allocation-stats.h",
@@ -2524,6 +2525,7 @@ v8_source_set("v8_base_without_compiler") {
     "src/heap/list.h",
     "src/heap/local-allocator-inl.h",
     "src/heap/local-allocator.h",
+    "src/heap/local-heap-inl.h",
     "src/heap/local-heap.cc",
     "src/heap/local-heap.h",
     "src/heap/mark-compact-inl.h",

src/handles/persistent-handles-inl.h

@@ -0,0 +1,29 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HANDLES_PERSISTENT_HANDLES_INL_H_
+#define V8_HANDLES_PERSISTENT_HANDLES_INL_H_
+
+#include "src/handles/persistent-handles.h"
+
+namespace v8 {
+namespace internal {
+
+template <typename T>
+Handle<T> PersistentHandles::NewHandle(T obj) {
+#ifdef DEBUG
+  CheckOwnerIsParked();
+#endif
+  return Handle<T>(GetHandle(obj.ptr()));
+}
+
+template <typename T>
+Handle<T> PersistentHandles::NewHandle(Handle<T> obj) {
+  return NewHandle(*obj);
+}
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HANDLES_PERSISTENT_HANDLES_INL_H_

src/handles/persistent-handles.cc

@@ -41,6 +41,10 @@ void PersistentHandles::Detach() {
   owner_ = nullptr;
 }
 
+void PersistentHandles::CheckOwnerIsParked() {
+  if (owner_) DCHECK(!owner_->IsParked());
+}
+
 bool PersistentHandles::Contains(Address* location) {
   auto it = ordered_blocks_.upper_bound(location);
   if (it == ordered_blocks_.begin()) return false;
@@ -69,13 +73,6 @@ void PersistentHandles::AddBlock() {
 #endif
 }
 
-Handle<Object> PersistentHandles::NewHandle(Address value) {
-#ifdef DEBUG
-  if (owner_) DCHECK(!owner_->IsParked());
-#endif
-  return Handle<Object>(GetHandle(value));
-}
-
 Address* PersistentHandles::GetHandle(Address value) {
   if (block_next_ == block_limit_) {
     AddBlock();

src/handles/persistent-handles.h

@@ -31,7 +31,11 @@ class PersistentHandles {
 
   void Iterate(RootVisitor* visitor);
 
-  V8_EXPORT_PRIVATE Handle<Object> NewHandle(Address value);
+  template <typename T>
+  inline Handle<T> NewHandle(T object);
+
+  template <typename T>
+  inline Handle<T> NewHandle(Handle<T> object);
 
 #ifdef DEBUG
   bool Contains(Address* location);
@@ -39,11 +43,12 @@ class PersistentHandles {
 
  private:
   void AddBlock();
-  Address* GetHandle(Address value);
+  V8_EXPORT_PRIVATE Address* GetHandle(Address value);
 
 #ifdef DEBUG
   void Attach(LocalHeap* local_heap);
   void Detach();
+  V8_EXPORT_PRIVATE void CheckOwnerIsParked();
 
   LocalHeap* owner_ = nullptr;
 

src/heap/local-heap-inl.h

@@ -0,0 +1,31 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HEAP_LOCAL_HEAP_INL_H_
+#define V8_HEAP_LOCAL_HEAP_INL_H_
+
+#include "src/handles/persistent-handles-inl.h"
+#include "src/handles/persistent-handles.h"
+#include "src/heap/local-heap.h"
+
+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);
+}
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HEAP_LOCAL_HEAP_INL_H_

src/heap/local-heap.cc

@@ -52,12 +52,11 @@ LocalHeap::~LocalHeap() {
   current_local_heap = nullptr;
 }
 
-Handle<Object> LocalHeap::NewPersistentHandle(Address value) {
+void LocalHeap::EnsurePersistentHandles() {
   if (!persistent_handles_) {
     persistent_handles_.reset(
         heap_->isolate()->NewPersistentHandles().release());
   }
-  return persistent_handles_->NewHandle(value);
 }
 
 std::unique_ptr<PersistentHandles> LocalHeap::DetachPersistentHandles() {

src/heap/local-heap.h

@@ -37,7 +37,10 @@ class V8_EXPORT_PRIVATE LocalHeap {
 
   LocalHandles* handles() { return handles_.get(); }
 
-  Handle<Object> NewPersistentHandle(Address value);
+  template <typename T>
+  inline Handle<T> NewPersistentHandle(T object);
+  template <typename T>
+  inline Handle<T> NewPersistentHandle(Handle<T> object);
   std::unique_ptr<PersistentHandles> DetachPersistentHandles();
 #ifdef DEBUG
   bool ContainsPersistentHandle(Address* location);
@@ -83,6 +86,8 @@ class V8_EXPORT_PRIVATE LocalHeap {
   void Unpark();
   void EnsureParkedBeforeDestruction();
 
+  void EnsurePersistentHandles();
+
   bool IsSafepointRequested();
   void ClearSafepointRequested();
 

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

@@ -6,8 +6,10 @@
 #include "src/base/platform/semaphore.h"
 #include "src/handles/handles-inl.h"
 #include "src/handles/local-handles-inl.h"
+#include "src/handles/persistent-handles-inl.h"
 #include "src/handles/persistent-handles.h"
 #include "src/heap/heap.h"
+#include "src/heap/local-heap-inl.h"
 #include "src/heap/local-heap.h"
 #include "test/cctest/cctest.h"
 #include "test/cctest/heap/heap-utils.h"
@@ -34,11 +36,9 @@ class PersistentHandlesThread final : public v8::base::Thread {
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
     LocalHandleScope scope(&local_heap);
-    Address object = handles_[0]->ptr();
 
     for (int i = 0; i < kNumHandles; i++) {
-      handles_.push_back(
-          Handle<JSObject>::cast(local_heap.NewPersistentHandle(object)));
+      handles_.push_back(local_heap.NewPersistentHandle(handles_[0]));
     }
 
     sema_started_->Signal();
@@ -92,12 +92,11 @@ TEST(LinearSearchFlatObject) {
                                                     NONE)
       .Check();
 
-  Address object = js_object->ptr();
   for (int i = 0; i < kNumHandles; i++) {
-    handles.push_back(Handle<JSObject>::cast(ph->NewHandle(object)));
+    handles.push_back(ph->NewHandle(js_object));
   }
 
-  Handle<Name> persistent_name = Handle<Name>::cast(ph->NewHandle(name->ptr()));
+  Handle<Name> persistent_name = ph->NewHandle(name);
 
   base::Semaphore sema_started(0);
 
@@ -157,12 +156,11 @@ TEST(LinearSearchFlatObject_ManyElements) {
   }
   CHECK_GT(js_object->map().NumberOfOwnDescriptors(), 8);
 
-  Address object = js_object->ptr();
   for (int i = 0; i < kNumHandles; i++) {
-    handles.push_back(Handle<JSObject>::cast(ph->NewHandle(object)));
+    handles.push_back(ph->NewHandle(js_object));
   }
 
-  Handle<Name> persistent_name = Handle<Name>::cast(ph->NewHandle(name->ptr()));
+  Handle<Name> persistent_name = ph->NewHandle(name);
 
   base::Semaphore sema_started(0);
 

test/cctest/test-concurrent-prototype.cc

@@ -6,8 +6,10 @@
 #include "src/base/platform/semaphore.h"
 #include "src/handles/handles-inl.h"
 #include "src/handles/local-handles-inl.h"
+#include "src/handles/persistent-handles-inl.h"
 #include "src/handles/persistent-handles.h"
 #include "src/heap/heap.h"
+#include "src/heap/local-heap-inl.h"
 #include "src/heap/local-heap.h"
 #include "test/cctest/cctest.h"
 #include "test/cctest/heap/heap-utils.h"
@@ -33,11 +35,9 @@ class ConcurrentSearchThread final : public v8::base::Thread {
   void Run() override {
     LocalHeap local_heap(heap_, std::move(ph_));
     LocalHandleScope scope(&local_heap);
-    Address object = handles_[0]->ptr();
 
     for (int i = 0; i < kNumHandles; i++) {
-      handles_.push_back(
-          Handle<JSObject>::cast(local_heap.NewPersistentHandle(object)));
+      handles_.push_back(local_heap.NewPersistentHandle(handles_[0]));
     }
 
     sema_started_->Signal();
@@ -90,9 +90,8 @@ TEST(ProtoWalkBackground) {
                                                     NONE)
       .Check();
 
-  Address object = js_object->ptr();
   for (int i = 0; i < kNumHandles; i++) {
-    handles.push_back(Handle<JSObject>::cast(ph->NewHandle(object)));
+    handles.push_back(ph->NewHandle(js_object));
   }
 
   base::Semaphore sema_started(0);
@@ -131,9 +130,8 @@ TEST(ProtoWalkBackground_DescriptorArrayWrite) {
                                                     NONE)
       .Check();
 
-  Address object = js_object->ptr();
   for (int i = 0; i < kNumHandles; i++) {
-    handles.push_back(Handle<JSObject>::cast(ph->NewHandle(object)));
+    handles.push_back(ph->NewHandle(js_object));
   }
 
   base::Semaphore sema_started(0);
@@ -172,9 +170,8 @@ TEST(ProtoWalkBackground_PrototypeChainWrite) {
       factory->NewFunctionForTest(factory->empty_string());
   Handle<JSObject> js_object = factory->NewJSObject(function);
 
-  Address object = js_object->ptr();
   for (int i = 0; i < kNumHandles; i++) {
-    handles.push_back(Handle<JSObject>::cast(ph->NewHandle(object)));
+    handles.push_back(ph->NewHandle(js_object));
   }
 
   base::Semaphore sema_started(0);

test/cctest/test-persistent-handles.cc

@@ -10,8 +10,10 @@
 #include "src/base/platform/semaphore.h"
 #include "src/handles/handles-inl.h"
 #include "src/handles/local-handles-inl.h"
+#include "src/handles/persistent-handles-inl.h"
 #include "src/handles/persistent-handles.h"
 #include "src/heap/heap.h"
+#include "src/heap/local-heap-inl.h"
 #include "src/heap/local-heap.h"
 #include "src/heap/safepoint.h"
 #include "src/objects/heap-number.h"
@@ -28,14 +30,14 @@ namespace {
 class PersistentHandlesThread final : public v8::base::Thread {
  public:
   PersistentHandlesThread(Heap* heap, std::vector<Handle<HeapNumber>> handles,
-                          std::unique_ptr<PersistentHandles> ph, Address object,
-                          base::Semaphore* sema_started,
+                          std::unique_ptr<PersistentHandles> ph,
+                          HeapNumber number, base::Semaphore* sema_started,
                           base::Semaphore* sema_gc_finished)
       : v8::base::Thread(base::Thread::Options("ThreadWithLocalHeap")),
         heap_(heap),
         handles_(std::move(handles)),
         ph_(std::move(ph)),
-        object_(object),
+        number_(number),
         sema_started_(sema_started),
         sema_gc_finished_(sema_gc_finished) {}
 
@@ -44,8 +46,7 @@ class PersistentHandlesThread final : public v8::base::Thread {
     LocalHandleScope scope(&local_heap);
 
     for (int i = 0; i < kNumHandles; i++) {
-      handles_.push_back(
-          Handle<HeapNumber>::cast(local_heap.NewPersistentHandle(object_)));
+      handles_.push_back(local_heap.NewPersistentHandle(number_));
     }
 
     sema_started_->Signal();
@@ -68,7 +69,7 @@ class PersistentHandlesThread final : public v8::base::Thread {
   Heap* heap_;
   std::vector<Handle<HeapNumber>> handles_;
   std::unique_ptr<PersistentHandles> ph_;
-  Address object_;
+  HeapNumber number_;
   base::Semaphore* sema_started_;
   base::Semaphore* sema_gc_finished_;
 };
@@ -78,17 +79,14 @@ TEST(CreatePersistentHandles) {
   FLAG_local_heaps = true;
   Isolate* isolate = CcTest::i_isolate();
 
-  Address object = kNullAddress;
   std::unique_ptr<PersistentHandles> ph = isolate->NewPersistentHandles();
   std::vector<Handle<HeapNumber>> handles;
 
   HandleScope handle_scope(isolate);
   Handle<HeapNumber> number = isolate->factory()->NewHeapNumber(42.0);
 
-  object = number->ptr();
-
   for (int i = 0; i < kNumHandles; i++) {
-    handles.push_back(Handle<HeapNumber>::cast(ph->NewHandle(object)));
+    handles.push_back(ph->NewHandle(number));
   }
 
   base::Semaphore sema_started(0);
@@ -96,8 +94,8 @@ TEST(CreatePersistentHandles) {
 
   // pass persistent handles to background thread
   std::unique_ptr<PersistentHandlesThread> thread(new PersistentHandlesThread(
-      isolate->heap(), std::move(handles), std::move(ph), object, &sema_started,
-      &sema_gc_finished));
+      isolate->heap(), std::move(handles), std::move(ph), *number,
+      &sema_started, &sema_gc_finished));
   CHECK(thread->Start());
 
   sema_started.Wait();
@@ -109,7 +107,7 @@ TEST(CreatePersistentHandles) {
 
   // get persistent handles back to main thread
   ph = std::move(thread->ph_);
-  ph->NewHandle(number->ptr());
+  ph->NewHandle(number);
 }
 
 TEST(DereferencePersistentHandle) {
@@ -122,7 +120,7 @@ TEST(DereferencePersistentHandle) {
   {
     HandleScope handle_scope(isolate);
     Handle<HeapNumber> number = isolate->factory()->NewHeapNumber(42.0);
-    ph = Handle<HeapNumber>::cast(phs->NewHandle(number->ptr()));
+    ph = phs->NewHandle(number);
   }
   {
     LocalHeap local_heap(isolate->heap(), std::move(phs));