Reland of "[heap] Clean up stale store buffer entries for aborted pages."

src/heap/mark-compact.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/heap/mark-compact.h"
 
 #include "src/base/atomicops.h"
 #include "src/base/bits.h"
 #include "src/base/sys-info.h"
 #include "src/code-stubs.h"
@@ skipping 665 matching lines @@
   int area_size = space->AreaSize();
 
   // Pairs of (live_bytes_in_page, page).
   typedef std::pair<int, Page*> LiveBytesPagePair;
   std::vector<LiveBytesPagePair> pages;
   pages.reserve(number_of_pages);
 
   PageIterator it(space);
   while (it.has_next()) {
     Page* p = it.next();
+    // Invariant: No page should be marked as aborted after a GC.
+    DCHECK(!p->IsFlagSet(Page::COMPACTION_WAS_ABORTED));
     if (p->NeverEvacuate()) continue;
     if (p->IsFlagSet(Page::POPULAR_PAGE)) {
       // This page had slots buffer overflow on previous GC, skip it.
       p->ClearFlag(Page::POPULAR_PAGE);
       continue;
     }
     // Invariant: Evacuation candidates are just created when marking is
     // started. At the end of a GC all evacuation candidates are cleared and
     // their slot buffers are released.
     CHECK(!p->IsEvacuationCandidate());
@@ skipping 2575 matching lines @@
         // moved, others are still in place.
         // We need to:
         // - Leave the evacuation candidate flag for later processing of
         //   slots buffer entries.
         // - Leave the slots buffer there for processing of entries added by
         //   the write barrier.
         // - Rescan the page as slot recording in the migration buffer only
         //   happens upon moving (which we potentially didn't do).
         // - Leave the page in the list of pages of a space since we could not
         //   fully evacuate it.
+        // - Mark them for rescanning for store buffer entries as we otherwise
+        //   might have stale store buffer entries that become "valid" again
+        //   after reusing the memory. Note that all existing store buffer
+        //   entries of such pages are filtered before rescanning.
         DCHECK(p->IsEvacuationCandidate());
         p->SetFlag(Page::COMPACTION_WAS_ABORTED);
+        p->set_scan_on_scavenge(true);
         abandoned_pages++;
         break;
       case MemoryChunk::kCompactingFinalize:
         DCHECK(p->IsEvacuationCandidate());
         p->SetWasSwept();
         p->Unlink();
         break;
       case MemoryChunk::kCompactingDone:
         DCHECK(p->IsFlagSet(Page::POPULAR_PAGE));
         DCHECK(p->IsFlagSet(Page::RESCAN_ON_EVACUATION));
@@ skipping 358 matching lines @@
     }
   }
 
   {
     GCTracer::Scope gc_scope(heap()->tracer(),
                              GCTracer::Scope::MC_UPDATE_ROOT_TO_NEW_POINTERS);
     // Update roots.
     heap_->IterateRoots(&updating_visitor, VISIT_ALL_IN_SWEEP_NEWSPACE);
   }
 
-  {
-    GCTracer::Scope gc_scope(heap()->tracer(),
-                             GCTracer::Scope::MC_UPDATE_OLD_TO_NEW_POINTERS);
-    StoreBufferRebuildScope scope(heap_, heap_->store_buffer(),
-                                  &Heap::ScavengeStoreBufferCallback);
-    heap_->store_buffer()->IteratePointersToNewSpace(&UpdatePointer);
-  }
-
   int npages = evacuation_candidates_.length();
   {
     GCTracer::Scope gc_scope(
         heap()->tracer(),
         GCTracer::Scope::MC_UPDATE_POINTERS_BETWEEN_EVACUATED);
     for (int i = 0; i < npages; i++) {
       Page* p = evacuation_candidates_[i];
       DCHECK(p->IsEvacuationCandidate() ||
              p->IsFlagSet(Page::RESCAN_ON_EVACUATION));
 
@@ skipping 48 matching lines @@
             break;
           default:
             UNREACHABLE();
             break;
         }
       }
     }
   }
 
   {
+    // Note that this phase needs to happen after making aborted pages iterable
+    // in the previous (sweeping) phase.
+    GCTracer::Scope gc_scope(heap()->tracer(),
+                             GCTracer::Scope::MC_UPDATE_OLD_TO_NEW_POINTERS);
+    StoreBufferRebuildScope scope(heap_, heap_->store_buffer(),
+                                  &Heap::ScavengeStoreBufferCallback);
+    heap_->store_buffer()->IteratePointersToNewSpace(&UpdatePointer);
+  }
+
+  {
     GCTracer::Scope gc_scope(heap()->tracer(),
                              GCTracer::Scope::MC_UPDATE_MISC_POINTERS);
     heap_->string_table()->Iterate(&updating_visitor);
 
     // Update pointers from external string table.
     heap_->UpdateReferencesInExternalStringTable(
         &UpdateReferenceInExternalStringTableEntry);
 
     EvacuationWeakObjectRetainer evacuation_object_retainer;
     heap()->ProcessAllWeakReferences(&evacuation_object_retainer);
   }
 
   {
     GCTracer::Scope gc_scope(heap()->tracer(),
                              GCTracer::Scope::MC_SWEEP_ABORTED);
     // After updating all pointers, we can finally sweep the aborted pages,
     // effectively overriding any forward pointers.
     SweepAbortedPages();
   }
 

[Hannes Payer (out of office), 2015/12/02 12:45:18]

The store buffer processing should go here, after sweeping aborted pages. The
dependencies are really complicated here. I have to think a little about the
reordering of this phase.

[Michael Lippautz, 2015/12/02 13:04:19]

Done.

   heap_->isolate()->inner_pointer_to_code_cache()->Flush();
 
   // The hashing of weak_object_to_code_table is no longer valid.
   heap()->weak_object_to_code_table()->Rehash(
       heap()->isolate()->factory()->undefined_value());
 }
 
 
 void MarkCompactCollector::MoveEvacuationCandidatesToEndOfPagesList() {
   int npages = evacuation_candidates_.length();
@@ skipping 358 matching lines @@
     MarkBit mark_bit = Marking::MarkBitFrom(host);
     if (Marking::IsBlack(mark_bit)) {
       RelocInfo rinfo(isolate(), pc, RelocInfo::CODE_TARGET, 0, host);
       RecordRelocSlot(&rinfo, target);
     }
   }
 }
 
 }  // namespace internal
 }  // namespace v8