From ff8caf93b51622517544962fdb7b6d0ac6405b39 Mon Sep 17 00:00:00 2001
From: Ryan Macnak <rmacnak@google.com>
Date: Wed, 23 Nov 2022 20:24:29 +0000
Subject: [PATCH] [vm] Remove quadratic time spent zapping scoped handles.

TEST=ci
Change-Id: I7faaf61ff33ae54795db4da1b253053abea0d5a9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271581
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
---
 runtime/vm/handles.cc     | 11 +++++++++--
 runtime/vm/handles.h      |  8 --------
 runtime/vm/handles_impl.h | 26 --------------------------
 runtime/vm/zone_test.cc   | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 41 insertions(+), 36 deletions(-)

diff --git a/runtime/vm/handles.cc b/runtime/vm/handles.cc
index 888baaac1dc..1a5984ad1e5 100644
--- a/runtime/vm/handles.cc
+++ b/runtime/vm/handles.cc
@@ -90,11 +90,18 @@ HandleScope::~HandleScope() {
   ASSERT(thread()->zone() != NULL);
   VMHandles* handles = thread()->zone()->handles();
   ASSERT(handles != NULL);
+#if defined(DEBUG)
+  VMHandles::HandlesBlock* last = handles->scoped_blocks_;
+#endif
   handles->scoped_blocks_ = saved_handle_block_;
   handles->scoped_blocks_->set_next_handle_slot(saved_handle_slot_);
 #if defined(DEBUG)
-  handles->VerifyScopedHandleState();
-  handles->ZapFreeScopedHandles();
+  VMHandles::HandlesBlock* block = handles->scoped_blocks_;
+  for (;;) {
+    block->ZapFreeHandles();
+    if (block == last) break;
+    block = block->next_block();
+  }
   ASSERT(thread()->top_handle_scope() == this);
   thread()->set_top_handle_scope(link_);
 #endif
diff --git a/runtime/vm/handles.h b/runtime/vm/handles.h
index de48076932d..9cb28f20e2c 100644
--- a/runtime/vm/handles.h
+++ b/runtime/vm/handles.h
@@ -218,14 +218,6 @@ class Handles {
   // Allocates a new handle block and links it up.
   void SetupNextZoneBlock();
 
-#if defined(DEBUG)
-  // Verifies consistency of handle blocks after a scope is destroyed.
-  void VerifyScopedHandleState();
-
-  // Zaps the free scoped handles to an uninitialized value.
-  void ZapFreeScopedHandles();
-#endif
-
   HandlesBlock* zone_blocks_;        // List of zone handles.
   HandlesBlock first_scoped_block_;  // First block of scoped handles.
   HandlesBlock* scoped_blocks_;      // List of scoped handles.
diff --git a/runtime/vm/handles_impl.h b/runtime/vm/handles_impl.h
index 92607b74f35..d43e0e8a4cb 100644
--- a/runtime/vm/handles_impl.h
+++ b/runtime/vm/handles_impl.h
@@ -207,32 +207,6 @@ void Handles<kHandleSizeInWords, kHandlesPerChunk, kOffsetOfRawPtr>::
   zone_blocks_ = new HandlesBlock(zone_blocks_);
 }
 
-#if defined(DEBUG)
-template <int kHandleSizeInWords, int kHandlesPerChunk, int kOffsetOfRawPtr>
-void Handles<kHandleSizeInWords, kHandlesPerChunk, kOffsetOfRawPtr>::
-    VerifyScopedHandleState() {
-  HandlesBlock* block = &first_scoped_block_;
-  const intptr_t end_index = (kHandleSizeInWords * kHandlesPerChunk);
-  do {
-    if (scoped_blocks_ == block && block->next_handle_slot() <= end_index) {
-      return;
-    }
-    block = block->next_block();
-  } while (block != NULL);
-  ASSERT(false);
-}
-
-template <int kHandleSizeInWords, int kHandlesPerChunk, int kOffsetOfRawPtr>
-void Handles<kHandleSizeInWords, kHandlesPerChunk, kOffsetOfRawPtr>::
-    ZapFreeScopedHandles() {
-  HandlesBlock* block = scoped_blocks_;
-  while (block != NULL) {
-    block->ZapFreeHandles();
-    block = block->next_block();
-  }
-}
-#endif
-
 template <int kHandleSizeInWords, int kHandlesPerChunk, int kOffsetOfRawPtr>
 int Handles<kHandleSizeInWords, kHandlesPerChunk, kOffsetOfRawPtr>::
     CountScopedHandles() const {
diff --git a/runtime/vm/zone_test.cc b/runtime/vm/zone_test.cc
index 13ca394a9ad..1b1daf2aeaf 100644
--- a/runtime/vm/zone_test.cc
+++ b/runtime/vm/zone_test.cc
@@ -263,4 +263,36 @@ ISOLATE_UNIT_TEST_CASE(ZonesNotLimitedByCompressedHeap) {
 }
 #endif  // defined(DART_COMPRESSED_POINTERS)
 
+ISOLATE_UNIT_TEST_CASE(ZoneVerificationScaling) {
+  // This ought to complete in O(n), not O(n^2).
+  const intptr_t n = 1000000;
+
+  StackZone stack_zone(thread);
+  Zone* zone = stack_zone.GetZone();
+
+  {
+    HANDLESCOPE(thread);
+    for (intptr_t i = 0; i < n; i++) {
+      const Object& a = Object::Handle(zone);
+      DEBUG_ASSERT(!a.IsNotTemporaryScopedHandle());
+      USE(a);
+      const Object& b = Object::ZoneHandle(zone);
+      DEBUG_ASSERT(b.IsNotTemporaryScopedHandle());
+      USE(b);
+    }
+    // Leaves lots of HandleBlocks for recycling.
+  }
+
+  for (intptr_t i = 0; i < n; i++) {
+    HANDLESCOPE(thread);
+    const Object& a = Object::Handle(zone);
+    DEBUG_ASSERT(!a.IsNotTemporaryScopedHandle());
+    USE(a);
+    const Object& b = Object::ZoneHandle(zone);
+    DEBUG_ASSERT(b.IsNotTemporaryScopedHandle());
+    USE(b);
+    // Should not visit those recyclable blocks over and over again.
+  }
+}
+
 }  // namespace dart