Revert of Rehash and clear deleted entries in weak collections during GC (patchset #8 id:140001 of https://codereview.chromium.org/1877233005/ )

mi-ac · Commit bot · commit 5f5a3282d4b2 · 2016-04-15T12:17:20.000Z
Reason for revert: [Sheriff] Speculative revert. Suspect for gc stress crashes, like: https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/builds/5119 Original issue's description: > Rehash and clear deleted entries in weak collections during GC > > Otherwise, they'll just keep growing until we run out of memory or hit the FixedArray's maximum capacity. > > BUG=v8:4909 > R=hpayer@chromium.org > LOG=n > > Committed: https://crrev.com/e093a047796d4c0575fe63d36529e7fe89b8865d > Cr-Commit-Position: refs/heads/master@{#35514} TBR=hpayer@chromium.org,jochen@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:4909 Review URL: https://codereview.chromium.org/1891863003 Cr-Commit-Position: refs/heads/master@{#35519}
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
@@ -2572,13 +2572,6 @@ void MarkCompactCollector::ClearWeakCollections() {
           table->RemoveEntry(i);
         }
       }
-      // Rehash if more than 25% of the entries are deleted entries.
-      // TODO(jochen): Consider to shrink the fixed array in place.
-      if ((table->NumberOfDeletedElements() << kJSWeakCollectionLoadFactorExp) >
-          table->NumberOfElements()) {
-        HandleScope scope(heap()->isolate());
-        table->Rehash(heap()->isolate()->factory()->undefined_value());
-      }
     }
     weak_collection_obj = weak_collection->next();
     weak_collection->set_next(heap()->undefined_value());
diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h
@@ -512,10 +512,6 @@ class MarkCompactCollector {
   static const uint32_t kSingleFreeEncoding = 0;
   static const uint32_t kMultiFreeEncoding = 1;
 
-  // If the number of deleted slots in a JSWeakCollection exceeds the number
-  // of entries / 2^(factor), we rehash the table.
-  static const int kJSWeakCollectionLoadFactorExp = 1;
-
   static inline bool IsMarked(Object* obj);
   static bool IsUnmarkedHeapObjectWithHeap(Heap* heap, Object** p);
 
diff --git a/src/objects.cc b/src/objects.cc
@@ -16867,16 +16867,6 @@ void HashTable<Derived, Shape, Key>::Rehash(Key key) {
       }
     }
   }
-  // Wipe deleted entries.
-  Heap* heap = GetHeap();
-  Object* the_hole = heap->the_hole_value();
-  Object* undefined = heap->undefined_value();
-  for (uint32_t current = 0; current < capacity; current++) {
-    if (get(EntryToIndex(current)) == the_hole) {
-      set(EntryToIndex(current), undefined);
-    }
-  }
-  SetNumberOfDeletedElements(0);
 }
 
 
diff --git a/test/cctest/test-weakmaps.cc b/test/cctest/test-weakmaps.cc
@@ -124,7 +124,7 @@ TEST(Weakness) {
   heap->CollectAllGarbage(false);
   CHECK_EQ(1, NumberOfWeakCalls);
   CHECK_EQ(0, ObjectHashTable::cast(weakmap->table())->NumberOfElements());
-  CHECK_EQ(0,
+  CHECK_EQ(2,
            ObjectHashTable::cast(weakmap->table())->NumberOfDeletedElements());
 }
 
diff --git a/test/cctest/test-weaksets.cc b/test/cctest/test-weaksets.cc
@@ -123,8 +123,8 @@ TEST(WeakSet_Weakness) {
   heap->CollectAllGarbage(false);
   CHECK_EQ(1, NumberOfWeakCalls);
   CHECK_EQ(0, ObjectHashTable::cast(weakset->table())->NumberOfElements());
-  CHECK_EQ(0,
-           ObjectHashTable::cast(weakset->table())->NumberOfDeletedElements());
+  CHECK_EQ(
+      1, ObjectHashTable::cast(weakset->table())->NumberOfDeletedElements());
 }
 
 

Original file line number	Diff line number	Diff line change
`@@ -2572,13 +2572,6 @@ void MarkCompactCollector::ClearWeakCollections() {`
`2572`	`2572`	`table->RemoveEntry(i);`
`2573`	`2573`	`}`
`2574`	`2574`	`}`
`2575`		`- // Rehash if more than 25% of the entries are deleted entries.`
`2576`		`- // TODO(jochen): Consider to shrink the fixed array in place.`
`2577`		`- if ((table->NumberOfDeletedElements() << kJSWeakCollectionLoadFactorExp) >`
`2578`		`- table->NumberOfElements()) {`
`2579`		`- HandleScope scope(heap()->isolate());`
`2580`		`- table->Rehash(heap()->isolate()->factory()->undefined_value());`
`2581`		`- }`
`2582`	`2575`	`}`
`2583`	`2576`	`weak_collection_obj = weak_collection->next();`
`2584`	`2577`	`weak_collection->set_next(heap()->undefined_value());`
Original file line number	Diff line number	Diff line change
`@@ -16867,16 +16867,6 @@ void HashTable<Derived, Shape, Key>::Rehash(Key key) {`
`16867`	`16867`	`}`
`16868`	`16868`	`}`
`16869`	`16869`	`}`
`16870`		`- // Wipe deleted entries.`
`16871`		`- Heap* heap = GetHeap();`
`16872`		`- Object* the_hole = heap->the_hole_value();`
`16873`		`- Object* undefined = heap->undefined_value();`
`16874`		`- for (uint32_t current = 0; current < capacity; current++) {`
`16875`		`- if (get(EntryToIndex(current)) == the_hole) {`
`16876`		`- set(EntryToIndex(current), undefined);`
`16877`		`- }`
`16878`		`- }`
`16879`		`- SetNumberOfDeletedElements(0);`
`16880`	`16870`	`}`
`16881`	`16871`
`16882`	`16872`
Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@ TEST(Weakness) {`
`124`	`124`	`heap->CollectAllGarbage(false);`
`125`	`125`	`CHECK_EQ(1, NumberOfWeakCalls);`
`126`	`126`	`CHECK_EQ(0, ObjectHashTable::cast(weakmap->table())->NumberOfElements());`
`127`		`- CHECK_EQ(0,`
	`127`	`+ CHECK_EQ(2,`
`128`	`128`	`ObjectHashTable::cast(weakmap->table())->NumberOfDeletedElements());`
`129`	`129`	`}`
`130`	`130`
Original file line number	Diff line number	Diff line change
`@@ -123,8 +123,8 @@ TEST(WeakSet_Weakness) {`
`123`	`123`	`heap->CollectAllGarbage(false);`
`124`	`124`	`CHECK_EQ(1, NumberOfWeakCalls);`
`125`	`125`	`CHECK_EQ(0, ObjectHashTable::cast(weakset->table())->NumberOfElements());`
`126`		`- CHECK_EQ(0,`
`127`		`- ObjectHashTable::cast(weakset->table())->NumberOfDeletedElements());`
	`126`	`+ CHECK_EQ(`
	`127`	`+ 1, ObjectHashTable::cast(weakset->table())->NumberOfDeletedElements());`
`128`	`128`	`}`
`129`	`129`
`130`	`130`