[global-handles] Update declarations

The change is just a refactoring without functional changes.

Bug: chromium:923361
Change-Id: Ie633c56122ff72658f0238dc40db698350a7b8e7
Reviewed-on: https://chromium-review.googlesource.com/c/1421363
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58963}

src/api.cc

@@ -8459,9 +8459,7 @@ void Isolate::GetStackSample(const RegisterState& state, void** frames,
 
 size_t Isolate::NumberOfPhantomHandleResetsSinceLastCall() {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
-  size_t result = isolate->global_handles()->NumberOfPhantomHandleResets();
-  isolate->global_handles()->ResetNumberOfPhantomHandleResets();
-  return result;
+  return isolate->global_handles()->GetAndResetGlobalHandleResetCount();
 }
 
 void Isolate::SetEventLogger(LogEventCallback that) {

src/global-handles.cc

@@ -198,7 +198,7 @@ NodeType* GlobalHandles::NodeSpace<NodeType>::Acquire(Object object) {
     block->ListAdd(&first_used_block_);
   }
   global_handles_->isolate()->counters()->global_handles()->Increment();
-  global_handles_->number_of_global_handles_++;
+  global_handles_->handles_count_++;
   DCHECK(node->IsInUse());
   return node;
 }
@@ -230,7 +230,7 @@ void GlobalHandles::NodeSpace<NodeType>::Free(NodeType* node) {
     block->ListRemove(&first_used_block_);
   }
   global_handles_->isolate()->counters()->global_handles()->Decrement();
-  global_handles_->number_of_global_handles_--;
+  global_handles_->handles_count_--;
 }
 
 class GlobalHandles::Node final {
@@ -598,10 +598,7 @@ GlobalHandles* GlobalHandles::Node::GetGlobalHandles() {
 
 GlobalHandles::GlobalHandles(Isolate* isolate)
     : isolate_(isolate),
-      regular_nodes_(new NodeSpace<GlobalHandles::Node>(this)),
-      number_of_global_handles_(0),
-      post_gc_processing_count_(0),
-      number_of_phantom_handle_resets_(0) {}
+      regular_nodes_(new NodeSpace<GlobalHandles::Node>(this)) {}
 
 GlobalHandles::~GlobalHandles() { regular_nodes_.reset(nullptr); }
 
@@ -823,7 +820,7 @@ void GlobalHandles::InvokeSecondPassPhantomCallbacks() {
 }
 
 int GlobalHandles::PostScavengeProcessing(
-    const int initial_post_gc_processing_count) {
+    unsigned initial_post_gc_processing_count) {
   int freed_nodes = 0;
   for (Node* node : new_space_nodes_) {
     DCHECK(node->is_in_new_space_list());
@@ -858,9 +855,8 @@ int GlobalHandles::PostScavengeProcessing(
   return freed_nodes;
 }
 
-
 int GlobalHandles::PostMarkSweepProcessing(
-    const int initial_post_gc_processing_count) {
+    unsigned initial_post_gc_processing_count) {
   int freed_nodes = 0;
   for (Node* node : *regular_nodes_) {
     if (!node->IsRetainer()) {
@@ -882,7 +878,6 @@ int GlobalHandles::PostMarkSweepProcessing(
   return freed_nodes;
 }
 
-
 void GlobalHandles::UpdateListOfNewSpaceNodes() {
   size_t last = 0;
   for (Node* node : new_space_nodes_) {
@@ -970,7 +965,7 @@ int GlobalHandles::PostGarbageCollectionProcessing(
   // GC is completely done, because the callbacks may invoke arbitrary
   // API functions.
   DCHECK(isolate_->heap()->gc_state() == Heap::NOT_IN_GC);
-  const int initial_post_gc_processing_count = ++post_gc_processing_count_;
+  const unsigned initial_post_gc_processing_count = ++post_gc_processing_count_;
   int freed_nodes = 0;
   bool synchronous_second_pass =
       isolate_->heap()->IsTearingDown() ||
@@ -1148,10 +1143,6 @@ void GlobalHandles::Print() {
 
 #endif
 
-void GlobalHandles::TearDown() {}
-
-EternalHandles::EternalHandles() : size_(0) {}
-
 EternalHandles::~EternalHandles() {
   for (Address* block : blocks_) delete[] block;
 }

src/global-handles.h

@@ -21,12 +21,6 @@ namespace internal {
 class HeapStats;
 class RootVisitor;
 
-// Structure for tracking global handles.
-// A single list keeps all the allocated global handles.
-// Destroyed handles stay in the list but is added to the free list.
-// At GC the destroyed global handles are removed from the free list
-// and deallocated.
-
 enum WeaknessType {
   // Embedder gets a handle to the dying object.
   FINALIZER_WEAK,
@@ -42,22 +36,10 @@ enum WeaknessType {
   PHANTOM_WEAK_RESET_HANDLE
 };
 
-class GlobalHandles {
+// Global handles hold handles that are independent of stack-state and can have
+// callbacks and finalizers attached to them.
+class GlobalHandles final {
  public:
-  ~GlobalHandles();
-
-  // Creates a new global handle that is alive until Destroy is called.
-  Handle<Object> Create(Object value);
-  Handle<Object> Create(Address value);
-
-  template <typename T>
-  Handle<T> Create(T value) {
-    static_assert(std::is_base_of<Object, T>::value, "static type violation");
-    // The compiler should only pick this method if T is not Object.
-    static_assert(!std::is_same<Object, T>::value, "compiler error");
-    return Handle<T>::cast(Create(Object(value)));
-  }
-
   // Copy a global handle
   static Handle<Object> CopyGlobal(Address* location);
 
@@ -81,23 +63,6 @@ class GlobalHandles {
 
   static void AnnotateStrongRetainer(Address* location, const char* label);
 
-  void RecordStats(HeapStats* stats);
-
-  // Returns the current number of handles to global objects.
-  int global_handles_count() const {
-    return number_of_global_handles_;
-  }
-
-  size_t NumberOfPhantomHandleResets() {
-    return number_of_phantom_handle_resets_;
-  }
-
-  void ResetNumberOfPhantomHandleResets() {
-    number_of_phantom_handle_resets_ = 0;
-  }
-
-  size_t NumberOfNewSpaceNodes() { return new_space_nodes_.size(); }
-
   // Clear the weakness of a global handle.
   static void* ClearWeakness(Address* location);
 
@@ -107,7 +72,25 @@ class GlobalHandles {
   // Tells whether global handle is weak.
   static bool IsWeak(Address* location);
 
+  explicit GlobalHandles(Isolate* isolate);
+  ~GlobalHandles();
+
+  // Creates a new global handle that is alive until Destroy is called.
+  Handle<Object> Create(Object value);
+  Handle<Object> Create(Address value);
+
+  template <typename T>
+  Handle<T> Create(T value) {
+    static_assert(std::is_base_of<Object, T>::value, "static type violation");
+    // The compiler should only pick this method if T is not Object.
+    static_assert(!std::is_same<Object, T>::value, "compiler error");
+    return Handle<T>::cast(Create(Object(value)));
+  }
+
+  void RecordStats(HeapStats* stats);
+
   int InvokeFirstPassWeakCallbacks();
+  void InvokeSecondPassPhantomCallbacks();
 
   // Process pending weak handles.
   // Returns the number of freed nodes.
@@ -115,9 +98,7 @@ class GlobalHandles {
       GarbageCollector collector, const v8::GCCallbackFlags gc_callback_flags);
 
   void IterateStrongRoots(RootVisitor* v);
-
   void IterateWeakRoots(RootVisitor* v);
-
   void IterateAllRoots(RootVisitor* v);
 
   void IterateAllNewSpaceRoots(RootVisitor* v);
@@ -143,10 +124,10 @@ class GlobalHandles {
   // |should_reset_handle| as pending.
   void IdentifyWeakHandles(WeakSlotCallbackWithHeap should_reset_handle);
 
-  // NOTE: Five ...NewSpace... functions below are used during
-  // scavenge collections and iterate over sets of handles that are
-  // guaranteed to contain all handles holding new space objects (but
-  // may also include old space objects).
+  //  Note: The following *NewSpace* methods are used for the Scavenger to
+  //  identify and process handles in new space. The set of new space handles is
+  //  complete but the methods may encounter handles that are already in old
+  //  space.
 
   // Iterates over strong and dependent handles. See the note above.
   void IterateNewSpaceStrongAndDependentRoots(RootVisitor* v);
@@ -170,18 +151,23 @@ class GlobalHandles {
   // unmodified
   void IdentifyWeakUnmodifiedObjects(WeakSlotCallback is_unmodified);
 
-  // Tear down the global handle structure.
-  void TearDown();
+  Isolate* isolate() const { return isolate_; }
 
-  Isolate* isolate() { return isolate_; }
+  // Number of global handles.
+  size_t handles_count() const { return handles_count_; }
+  size_t new_space_handles_count() const { return new_space_nodes_.size(); }
+
+  size_t GetAndResetGlobalHandleResetCount() {
+    size_t old = number_of_phantom_handle_resets_;
+    number_of_phantom_handle_resets_ = 0;
+    return old;
+  }
 
 #ifdef DEBUG
   void PrintStats();
   void Print();
 #endif  // DEBUG
 
-  void InvokeSecondPassPhantomCallbacks();
-
  private:
   // Internal node structures.
   class Node;
@@ -193,17 +179,15 @@ class GlobalHandles {
   class NodeSpace;
   class PendingPhantomCallback;
 
-  explicit GlobalHandles(Isolate* isolate);
-
   void InvokeSecondPassPhantomCallbacksFromTask();
-  int PostScavengeProcessing(int initial_post_gc_processing_count);
-  int PostMarkSweepProcessing(int initial_post_gc_processing_count);
+  int PostScavengeProcessing(unsigned initial_post_gc_processing_count);
+  int PostMarkSweepProcessing(unsigned initial_post_gc_processing_count);
   void InvokeOrScheduleSecondPassPhantomCallbacks(bool synchronous_second_pass);
   void UpdateListOfNewSpaceNodes();
   void ApplyPersistentHandleVisitor(v8::PersistentHandleVisitor* visitor,
                                     Node* node);
 
-  Isolate* isolate_;
+  Isolate* const isolate_;
 
   std::unique_ptr<NodeSpace<Node>> regular_nodes_;
   // Contains all nodes holding new space objects. Note: when the list
@@ -211,23 +195,20 @@ class GlobalHandles {
   std::vector<Node*> new_space_nodes_;
 
   // Field always containing the number of handles to global objects.
-  int number_of_global_handles_;
-
-  int post_gc_processing_count_;
-
-  size_t number_of_phantom_handle_resets_;
+  size_t handles_count_ = 0;
+  size_t number_of_phantom_handle_resets_ = 0;
 
   std::vector<PendingPhantomCallback> pending_phantom_callbacks_;
   std::vector<PendingPhantomCallback> second_pass_callbacks_;
   bool second_pass_callbacks_task_posted_ = false;
 
-  friend class Isolate;
+  // Counter for recursive garbage collections during callback processing.
+  unsigned post_gc_processing_count_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(GlobalHandles);
 };
 
-
-class GlobalHandles::PendingPhantomCallback {
+class GlobalHandles::PendingPhantomCallback final {
  public:
   typedef v8::WeakCallbackInfo<void> Data;
   PendingPhantomCallback(
@@ -241,8 +222,8 @@ class GlobalHandles::PendingPhantomCallback {
 
   void Invoke(Isolate* isolate);
 
-  Node* node() { return node_; }
-  Data::Callback callback() { return callback_; }
+  Node* node() const { return node_; }
+  Data::Callback callback() const { return callback_; }
 
  private:
   Node* node_;
@@ -251,14 +232,11 @@ class GlobalHandles::PendingPhantomCallback {
   void* embedder_fields_[v8::kEmbedderFieldsInWeakCallback];
 };
 
-
-class EternalHandles {
+class EternalHandles final {
  public:
-  EternalHandles();
+  EternalHandles() = default;
   ~EternalHandles();
 
-  int NumberOfHandles() { return size_; }
-
   // Create an EternalHandle, overwriting the index.
   void Create(Isolate* isolate, Object object, int* index);
 
@@ -274,6 +252,8 @@ class EternalHandles {
   // Rebuilds new space list.
   void PostGarbageCollectionProcessing();
 
+  size_t handles_count() const { return size_; }
+
  private:
   static const int kInvalidIndex = -1;
   static const int kShift = 8;
@@ -287,14 +267,13 @@ class EternalHandles {
     return &blocks_[index >> kShift][index & kMask];
   }
 
-  int size_;
+  int size_ = 0;
   std::vector<Address*> blocks_;
   std::vector<int> new_space_indices_;
 
   DISALLOW_COPY_AND_ASSIGN(EternalHandles);
 };
 
-
 }  // namespace internal
 }  // namespace v8
 

src/heap/heap.cc

@@ -4681,8 +4681,6 @@ void Heap::TearDown() {
   delete local_embedder_heap_tracer_;
   local_embedder_heap_tracer_ = nullptr;
 
-  isolate_->global_handles()->TearDown();
-
   external_string_table_.TearDown();
 
   // Tear down all ArrayBuffers before tearing down the heap since  their

src/heap/mark-compact.cc

@@ -3876,7 +3876,7 @@ template <class ParallelItem>
 void SeedGlobalHandles(GlobalHandles* global_handles, ItemParallelJob* job) {
   // Create batches of global handles.
   const size_t kGlobalHandlesBufferSize = 1000;
-  const size_t new_space_nodes = global_handles->NumberOfNewSpaceNodes();
+  const size_t new_space_nodes = global_handles->new_space_handles_count();
   for (size_t start = 0; start < new_space_nodes;
        start += kGlobalHandlesBufferSize) {
     size_t end = start + kGlobalHandlesBufferSize;

test/cctest/test-api.cc

@@ -4281,7 +4281,7 @@ THREADED_TEST(ResettingGlobalHandle) {
   }
   v8::internal::GlobalHandles* global_handles =
       reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
-  int initial_handle_count = global_handles->global_handles_count();
+  size_t initial_handle_count = global_handles->handles_count();
   {
     v8::HandleScope scope(isolate);
     CHECK_EQ(3, v8::Local<String>::New(isolate, global)->Length());
@@ -4290,13 +4290,13 @@ THREADED_TEST(ResettingGlobalHandle) {
     v8::HandleScope scope(isolate);
     global.Reset(isolate, v8_str("longer"));
   }
-  CHECK_EQ(global_handles->global_handles_count(), initial_handle_count);
+  CHECK_EQ(global_handles->handles_count(), initial_handle_count);
   {
     v8::HandleScope scope(isolate);
     CHECK_EQ(6, v8::Local<String>::New(isolate, global)->Length());
   }
   global.Reset();
-  CHECK_EQ(global_handles->global_handles_count(), initial_handle_count - 1);
+  CHECK_EQ(global_handles->handles_count(), initial_handle_count - 1);
 }
 
 
@@ -4309,7 +4309,7 @@ THREADED_TEST(ResettingGlobalHandleToEmpty) {
   }
   v8::internal::GlobalHandles* global_handles =
       reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
-  int initial_handle_count = global_handles->global_handles_count();
+  size_t initial_handle_count = global_handles->handles_count();
   {
     v8::HandleScope scope(isolate);
     CHECK_EQ(3, v8::Local<String>::New(isolate, global)->Length());
@@ -4320,7 +4320,7 @@ THREADED_TEST(ResettingGlobalHandleToEmpty) {
     global.Reset(isolate, empty);
   }
   CHECK(global.IsEmpty());
-  CHECK_EQ(global_handles->global_handles_count(), initial_handle_count - 1);
+  CHECK_EQ(global_handles->handles_count(), initial_handle_count - 1);
 }
 
 
@@ -4347,17 +4347,16 @@ THREADED_TEST(Global) {
   }
   v8::internal::GlobalHandles* global_handles =
       reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
-  int initial_handle_count = global_handles->global_handles_count();
+  size_t initial_handle_count = global_handles->handles_count();
   {
     v8::Global<String> unique(isolate, global);
-    CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count());
+    CHECK_EQ(initial_handle_count + 1, global_handles->handles_count());
     // Test assignment via Pass
     {
       v8::Global<String> copy = unique.Pass();
       CHECK(unique.IsEmpty());
       CHECK(copy == global);
-      CHECK_EQ(initial_handle_count + 1,
-               global_handles->global_handles_count());
+      CHECK_EQ(initial_handle_count + 1, global_handles->handles_count());
       unique = copy.Pass();
     }
     // Test ctor via Pass
@@ -4365,8 +4364,7 @@ THREADED_TEST(Global) {
       v8::Global<String> copy(unique.Pass());
       CHECK(unique.IsEmpty());
       CHECK(copy == global);
-      CHECK_EQ(initial_handle_count + 1,
-               global_handles->global_handles_count());
+      CHECK_EQ(initial_handle_count + 1, global_handles->handles_count());
       unique = copy.Pass();
     }
     // Test pass through function call
@@ -4374,19 +4372,18 @@ THREADED_TEST(Global) {
       v8::Global<String> copy = PassUnique(unique.Pass());
       CHECK(unique.IsEmpty());
       CHECK(copy == global);
-      CHECK_EQ(initial_handle_count + 1,
-               global_handles->global_handles_count());
+      CHECK_EQ(initial_handle_count + 1, global_handles->handles_count());
       unique = copy.Pass();
     }
-    CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count());
+    CHECK_EQ(initial_handle_count + 1, global_handles->handles_count());
   }
   // Test pass from function call
   {
     v8::Global<String> unique = ReturnUnique(isolate, global);
     CHECK(unique == global);
-    CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count());
+    CHECK_EQ(initial_handle_count + 1, global_handles->handles_count());
   }
-  CHECK_EQ(initial_handle_count, global_handles->global_handles_count());
+  CHECK_EQ(initial_handle_count, global_handles->handles_count());
   global.Reset();
 }
 
@@ -4580,7 +4577,7 @@ void TestGlobalValueMap() {
   Map map(isolate);
   v8::internal::GlobalHandles* global_handles =
       reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
-  int initial_handle_count = global_handles->global_handles_count();
+  size_t initial_handle_count = global_handles->handles_count();
   CHECK_EQ(0, static_cast<int>(map.Size()));
   {
     HandleScope scope(isolate);
@@ -4613,14 +4610,14 @@ void TestGlobalValueMap() {
       CHECK(expected2->Equals(env.local(), ref.NewLocal(isolate)).FromJust());
     }
   }
-  CHECK_EQ(initial_handle_count + 1, global_handles->global_handles_count());
+  CHECK_EQ(initial_handle_count + 1, global_handles->handles_count());
   if (map.IsWeak()) {
     CcTest::PreciseCollectAllGarbage();
   } else {
     map.Clear();
   }
   CHECK_EQ(0, static_cast<int>(map.Size()));
-  CHECK_EQ(initial_handle_count, global_handles->global_handles_count());
+  CHECK_EQ(initial_handle_count, global_handles->handles_count());
   {
     HandleScope scope(isolate);
     Local<v8::Object> value = NewObjectForIntKey(isolate, templ, 9);
@@ -4628,7 +4625,7 @@ void TestGlobalValueMap() {
     map.Clear();
   }
   CHECK_EQ(0, static_cast<int>(map.Size()));
-  CHECK_EQ(initial_handle_count, global_handles->global_handles_count());
+  CHECK_EQ(initial_handle_count, global_handles->handles_count());
 }
 
 }  // namespace
@@ -4650,7 +4647,7 @@ TEST(PersistentValueVector) {
   v8::Isolate* isolate = env->GetIsolate();
   v8::internal::GlobalHandles* global_handles =
       reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
-  int handle_count = global_handles->global_handles_count();
+  size_t handle_count = global_handles->handles_count();
   HandleScope scope(isolate);
 
   v8::PersistentValueVector<v8::Object> vector(isolate);
@@ -4679,12 +4676,12 @@ TEST(PersistentValueVector) {
   CHECK(obj1->Equals(env.local(), vector.Get(4)).FromJust());
   CHECK(obj2->Equals(env.local(), vector.Get(1)).FromJust());
 
-  CHECK_EQ(5 + handle_count, global_handles->global_handles_count());
+  CHECK_EQ(5 + handle_count, global_handles->handles_count());
 
   vector.Clear();
   CHECK(vector.IsEmpty());
   CHECK_EQ(0, static_cast<int>(vector.Size()));
-  CHECK_EQ(handle_count, global_handles->global_handles_count());
+  CHECK_EQ(handle_count, global_handles->handles_count());
 }
 
 
@@ -14311,7 +14308,7 @@ TEST(CopyablePersistent) {
   v8::Isolate* isolate = context->GetIsolate();
   i::GlobalHandles* globals =
       reinterpret_cast<i::Isolate*>(isolate)->global_handles();
-  int initial_handles = globals->global_handles_count();
+  size_t initial_handles = globals->handles_count();
   typedef v8::Persistent<v8::Object, v8::CopyablePersistentTraits<v8::Object> >
       CopyableObject;
   {
@@ -14320,17 +14317,17 @@ TEST(CopyablePersistent) {
       v8::HandleScope scope(isolate);
       handle1.Reset(isolate, v8::Object::New(isolate));
     }
-    CHECK_EQ(initial_handles + 1, globals->global_handles_count());
+    CHECK_EQ(initial_handles + 1, globals->handles_count());
     CopyableObject  handle2;
     handle2 = handle1;
     CHECK(handle1 == handle2);
-    CHECK_EQ(initial_handles + 2, globals->global_handles_count());
+    CHECK_EQ(initial_handles + 2, globals->handles_count());
     CopyableObject handle3(handle2);
     CHECK(handle1 == handle3);
-    CHECK_EQ(initial_handles + 3, globals->global_handles_count());
+    CHECK_EQ(initial_handles + 3, globals->handles_count());
   }
   // Verify autodispose
-  CHECK_EQ(initial_handles, globals->global_handles_count());
+  CHECK_EQ(initial_handles, globals->handles_count());
 }
 
 
@@ -14346,7 +14343,7 @@ TEST(WeakCallbackApi) {
   v8::Isolate* isolate = context->GetIsolate();
   i::GlobalHandles* globals =
       reinterpret_cast<i::Isolate*>(isolate)->global_handles();
-  int initial_handles = globals->global_handles_count();
+  size_t initial_handles = globals->handles_count();
   {
     v8::HandleScope scope(isolate);
     v8::Local<v8::Object> obj = v8::Object::New(isolate);
@@ -14360,7 +14357,7 @@ TEST(WeakCallbackApi) {
   }
   CcTest::PreciseCollectAllGarbage();
   // Verify disposed.
-  CHECK_EQ(initial_handles, globals->global_handles_count());
+  CHECK_EQ(initial_handles, globals->handles_count());
 }
 
 

test/cctest/test-global-handles.cc

@@ -145,7 +145,7 @@ TEST(EternalHandles) {
   int indices[kArrayLength];
   v8::Eternal<v8::Value> eternals[kArrayLength];
 
-  CHECK_EQ(0, eternal_handles->NumberOfHandles());
+  CHECK_EQ(0, eternal_handles->handles_count());
   for (int i = 0; i < kArrayLength; i++) {
     indices[i] = -1;
     HandleScope scope(isolate);
@@ -184,7 +184,7 @@ TEST(EternalHandles) {
     }
   }
 
-  CHECK_EQ(2*kArrayLength, eternal_handles->NumberOfHandles());
+  CHECK_EQ(2 * kArrayLength, eternal_handles->handles_count());
 
   // Create an eternal via the constructor
   {
@@ -195,7 +195,7 @@ TEST(EternalHandles) {
     CHECK(object == eternal.Get(v8_isolate));
   }
 
-  CHECK_EQ(2*kArrayLength + 1, eternal_handles->NumberOfHandles());
+  CHECK_EQ(2 * kArrayLength + 1, eternal_handles->handles_count());
 }