Revert "[heap] Refine ephemeron key processing"

mi-ac · V8 LUCI CQ · commit 31df0323564e · 2025-09-11T01:50:14.000-07:00
This reverts commit bfbbaaa. Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Blink%20Linux%20Debug/30190/overview Also blocks roll. Original change's description: > [heap] Refine ephemeron key processing > > Clean up the logic around ephemeron key processing, to remove duplicate > checks for forwarding address, and avoid no-op resetting of the key slot > with a non-forwarded live value. > > Change-Id: Ia54002696b0c0743e8276ebee71d3b9972019ada > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6909217 > Auto-Submit: Leszek Swirski <leszeks@chromium.org> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org> > Commit-Queue: Leszek Swirski <leszeks@chromium.org> > Commit-Queue: Michael Lippautz <mlippautz@chromium.org> > Cr-Commit-Position: refs/heads/main@{#102378} No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: I9d37fc005d4b464a1f3ea37638ca77bc61d7f02c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6939006 Auto-Submit: Michael Achenbach <machenbach@chromium.org> Owners-Override: Michael Achenbach <machenbach@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#102397}
diff --git a/src/heap/scavenger.cc b/src/heap/scavenger.cc
@@ -187,14 +187,21 @@ class IterateAndScavengePromotedObjectsVisitor final
 
 namespace {
 
-V8_INLINE bool IsUnscavengedHeapObject(Tagged<Object> object) {
+V8_INLINE bool IsUnscavengedHeapObject(Heap* heap, Tagged<Object> object) {
   return Heap::InFromPage(object) && !Cast<HeapObject>(object)
                                           ->map_word(kRelaxedLoad)
                                           .IsForwardingAddress();
 }
 
+// Same as IsUnscavengedHeapObject() above but specialized for HeapObjects.
+V8_INLINE bool IsUnscavengedHeapObject(Heap* heap,
+                                       Tagged<HeapObject> heap_object) {
+  return Heap::InFromPage(heap_object) &&
+         !heap_object->map_word(kRelaxedLoad).IsForwardingAddress();
+}
+
 bool IsUnscavengedHeapObjectSlot(Heap* heap, FullObjectSlot p) {
-  return IsUnscavengedHeapObject(*p);
+  return IsUnscavengedHeapObject(heap, *p);
 }
 
 }  // namespace
@@ -1308,24 +1315,17 @@ void ScavengerCollector::ProcessWeakReferences(
 // entry has a dead new-space key.
 void ScavengerCollector::ClearYoungEphemerons(
     EphemeronRememberedSet::TableList* ephemeron_table_list) {
-  ephemeron_table_list->Iterate([](Tagged<EphemeronHashTable> table) {
+  ephemeron_table_list->Iterate([this](Tagged<EphemeronHashTable> table) {
     for (InternalIndex i : table->IterateEntries()) {
       // Keys in EphemeronHashTables must be heap objects.
       HeapObjectSlot key_slot(
           table->RawFieldOfElementAt(EphemeronHashTable::EntryToIndex(i)));
       Tagged<HeapObject> key = key_slot.ToHeapObject();
-      // If the key is not in the from page, it's not being scavenged.
-      if (!Heap::InFromPage(key)) continue;
-      DCHECK(!IsAnyHole(key));
-      MapWord map_word = key->map_word(kRelaxedLoad);
-      if (!map_word.IsForwardingAddress()) {
-        // If the key is not forwarded, then it's dead.
-        DCHECK(IsUnscavengedHeapObject(key));
+      if (IsUnscavengedHeapObject(heap_, key)) {
         table->RemoveEntry(i);
       } else {
-        // Otherwise, we need to update the key slot to the forwarded address.
-        DCHECK(!IsUnscavengedHeapObject(key));
-        key_slot.StoreHeapObject(map_word.ToForwardingAddress(key));
+        Tagged<HeapObject> forwarded = ForwardingAddress(key);
+        key_slot.StoreHeapObject(forwarded);
       }
     }
   });
@@ -1344,28 +1344,13 @@ void ScavengerCollector::ClearOldEphemerons() {
       HeapObjectSlot key_slot(table->RawFieldOfElementAt(
           EphemeronHashTable::EntryToIndex(InternalIndex(*iti))));
       Tagged<HeapObject> key = key_slot.ToHeapObject();
-      // If the key is not young, we don't need it in the remembered set.
-      if (!HeapLayout::InYoungGeneration(key)) {
-        iti = indices.erase(iti);
-      }
-      DCHECK(!IsAnyHole(key));
-      // If the key is not in the from page, it's not being scavenged.
-      if (!Heap::InFromPage(key)) continue;
-      MapWord map_word = key->map_word(kRelaxedLoad);
-      DCHECK_IMPLIES(Heap::InToPage(key), !map_word.IsForwardingAddress());
-      if (!map_word.IsForwardingAddress()) {
-        // If the key is not forwarded, then it's dead.
-        DCHECK(IsUnscavengedHeapObject(key));
+      if (IsUnscavengedHeapObject(heap_, key)) {
         table->RemoveEntry(InternalIndex(*iti));
         iti = indices.erase(iti);
       } else {
-        // Otherwise, we need to update the key slot to the forwarded address.
-        DCHECK(!IsUnscavengedHeapObject(key));
-        Tagged<HeapObject> forwarded = map_word.ToForwardingAddress(key);
+        Tagged<HeapObject> forwarded = ForwardingAddress(key);
         key_slot.StoreHeapObject(forwarded);
         if (!HeapLayout::InYoungGeneration(forwarded)) {
-          // If the key was promoted out of new space, we don't need to keep it
-          // in the remembered set.
           iti = indices.erase(iti);
         } else {
           ++iti;
