[elements] Fix pathological slowness when deleting many elements

When most elements of an object are deleted, we want to normalize its
elements backing store to a dictionary in order to save space. Finding
the right time to do so should not incur a linear cost on each delete
operation. This patch changes the heuristic to an amortized-constant
approach based on a global counter and the current backing store
capacity.

BUG=chromium:542978

Change-Id: Ifdf29ab2211fdde1df9078f63be4118627d6a67e
Reviewed-on: https://chromium-review.googlesource.com/506191Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45330}

src/elements.cc

@@ -1886,8 +1886,6 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
     // TODO(verwaest): Move this out of elements.cc.
     // If an old space backing store is larger than a certain size and
     // has too few used values, normalize it.
-    // To avoid doing the check on every delete we require at least
-    // one adjacent hole to the value being deleted.
     const int kMinLengthForSparsenessCheck = 64;
     if (backing_store->length() < kMinLengthForSparsenessCheck) return;
     if (backing_store->GetHeap()->InNewSpace(*backing_store)) return;
@@ -1897,9 +1895,24 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
     } else {
       length = static_cast<uint32_t>(store->length());
     }
-    if ((entry > 0 && backing_store->is_the_hole(isolate, entry - 1)) ||
-        (entry + 1 < length &&
-         backing_store->is_the_hole(isolate, entry + 1))) {
+
+    // To avoid doing the check on every delete, use a counter-based heuristic.
+    const int kLengthFraction = 16;
+    // The above constant must be large enough to ensure that we check for
+    // normalization frequently enough. At a minimum, it should be large
+    // enough to reliably hit the "window" of remaining elements count where
+    // normalization would be beneficial.
+    STATIC_ASSERT(kLengthFraction >=
+                  SeededNumberDictionary::kEntrySize *
+                      SeededNumberDictionary::kPreferFastElementsSizeFactor);
+    size_t current_counter = isolate->elements_deletion_counter();
+    if (current_counter < length / kLengthFraction) {
+      isolate->set_elements_deletion_counter(current_counter + 1);
+      return;
+    }
+    // Reset the counter whenever the full check is performed.
+    isolate->set_elements_deletion_counter(0);
+
     if (!obj->IsJSArray()) {
       uint32_t i;
       for (i = entry + 1; i < length; i++) {
@@ -1914,18 +1927,17 @@ class FastElementsAccessor : public ElementsAccessorBase<Subclass, KindTraits> {
     for (int i = 0; i < backing_store->length(); ++i) {
       if (!backing_store->is_the_hole(isolate, i)) {
         ++num_used;
-          // Bail out if a number dictionary wouldn't be able to save at least
-          // 75% space.
-          if (4 * SeededNumberDictionary::ComputeCapacity(num_used) *
+        // Bail out if a number dictionary wouldn't be able to save much space.
+        if (SeededNumberDictionary::kPreferFastElementsSizeFactor *
+                SeededNumberDictionary::ComputeCapacity(num_used) *
                 SeededNumberDictionary::kEntrySize >
-              backing_store->length()) {
+            static_cast<uint32_t>(backing_store->length())) {
           return;
         }
       }
     }
     JSObject::NormalizeElements(obj);
   }
-  }
 
   static void ReconfigureImpl(Handle<JSObject> object,
                               Handle<FixedArrayBase> store, uint32_t entry,

src/isolate.h

@@ -1291,6 +1291,11 @@ class Isolate {
   // reset to nullptr.
   void UnregisterFromReleaseAtTeardown(ManagedObjectFinalizer** finalizer_ptr);
 
+  size_t elements_deletion_counter() { return elements_deletion_counter_; }
+  void set_elements_deletion_counter(size_t value) {
+    elements_deletion_counter_ = value;
+  }
+
  protected:
   explicit Isolate(bool enable_serializer);
   bool IsArrayOrObjectPrototype(Object* object);
@@ -1577,6 +1582,8 @@ class Isolate {
 
   size_t total_regexp_code_generated_;
 
+  size_t elements_deletion_counter_ = 0;
+
   friend class ExecutionAccess;
   friend class HandleScopeImplementer;
   friend class HeapTester;

src/objects.cc

@@ -15403,13 +15403,14 @@ static bool ShouldConvertToSlowElements(JSObject* object, uint32_t capacity,
        object->GetHeap()->InNewSpace(object))) {
     return false;
   }
-  // If the fast-case backing storage takes up roughly three times as
-  // much space (in machine words) as a dictionary backing storage
-  // would, the object should have slow elements.
+  // If the fast-case backing storage takes up much more memory than a
+  // dictionary backing storage would, the object should have slow elements.
   int used_elements = object->GetFastElementsUsage();
-  int dictionary_size = SeededNumberDictionary::ComputeCapacity(used_elements) *
+  uint32_t size_threshold =
+      SeededNumberDictionary::kPreferFastElementsSizeFactor *
+      SeededNumberDictionary::ComputeCapacity(used_elements) *
       SeededNumberDictionary::kEntrySize;
-  return 3 * static_cast<uint32_t>(dictionary_size) <= *new_capacity;
+  return size_threshold <= *new_capacity;
 }
 
 

src/objects/dictionary.h

@@ -334,6 +334,10 @@ class SeededNumberDictionary
   static const int kRequiresSlowElementsMask = 1;
   static const int kRequiresSlowElementsTagSize = 1;
   static const uint32_t kRequiresSlowElementsLimit = (1 << 29) - 1;
+
+  // JSObjects prefer dictionary elements if the dictionary saves this much
+  // memory compared to a fast elements backing store.
+  static const uint32_t kPreferFastElementsSizeFactor = 3;
 };
 
 class UnseededNumberDictionary