Unlink evacuation candidates from list of pages before starting sweeper threads.

src/mark-compact.cc

Index: src/mark-compact.cc
diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index cbb9dfbff0cef15f00545fdd142e41dc9e18db49..f84cddb351e22542ab05dbf19309a67fe93e893e 100644
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -560,6 +560,7 @@ void MarkCompactCollector::WaitUntilSweepingCompleted() {
   StealMemoryFromSweeperThreads(heap()->paged_space(OLD_POINTER_SPACE));
   heap()->paged_space(OLD_DATA_SPACE)->ResetUnsweptFreeBytes();
   heap()->paged_space(OLD_POINTER_SPACE)->ResetUnsweptFreeBytes();
+  heap()->FreeQueuedChunks();

[Michael Starzinger, 2013/03/08 12:52:06]

There shouldn't be any queued chunks at this point, I think this call is
obsolete.

[Hannes Payer (out of office), 2013/03/08 14:02:30]

Done.

 }
 
 
@@ -585,13 +586,6 @@ bool MarkCompactCollector::IsConcurrentSweepingInProgress() {
 }
 
 
-void MarkCompactCollector::FinalizeSweeping() {
-  ASSERT(sweeping_pending_ == false);
-  ReleaseEvacuationCandidates();
-  heap()->FreeQueuedChunks();
-}
-
-
 void MarkCompactCollector::MarkInParallel() {
   for (int i = 0; i < FLAG_marking_threads; i++) {
     heap()->isolate()->marking_threads()[i]->StartMarking();
@@ -911,7 +905,6 @@ void MarkCompactCollector::Prepare(GCTracer* tracer) {
   if (IsConcurrentSweepingInProgress()) {
     // Instead of waiting we could also abort the sweeper threads here.
     WaitUntilSweepingCompleted();
-    FinalizeSweeping();
   }
 
   // Clear marking bits if incremental marking is aborted.
@@ -2849,6 +2842,7 @@ void MarkCompactCollector::EvacuatePages() {
           slots_buffer_allocator_.DeallocateChain(page->slots_buffer_address());
           page->ClearEvacuationCandidate();
           page->SetFlag(Page::RESCAN_ON_EVACUATION);
+          page->InsertAfter(static_cast<PagedSpace*>(page->owner())->anchor());
         }
         return;
       }
@@ -3326,6 +3320,18 @@ void MarkCompactCollector::ReleaseEvacuationCandidates() {
 }
 
 
+void MarkCompactCollector::UnlinkEvacuationCandidates() {

[Michael Starzinger, 2013/03/08 12:52:06]

Can we move that to above the ReleaseEvacuationCandidates because that's the
natural order in which it gets called.

[Hannes Payer (out of office), 2013/03/08 14:02:30]

Done.

+  int npages = evacuation_candidates_.length();
+  for (int i = 0; i < npages; i++) {
+    Page* p = evacuation_candidates_[i];
+    if (!p->IsEvacuationCandidate()) continue;
+    p->Unlink();
+    p->ClearSweptPrecisely();
+    p->ClearSweptConservatively();
+  }
+}
+
+
 static const int kStartTableEntriesPerLine = 5;
 static const int kStartTableLines = 171;
 static const int kStartTableInvalidLine = 127;
@@ -3899,6 +3905,8 @@ void MarkCompactCollector::SweepSpaces() {
   SweepSpace(heap()->old_pointer_space(), how_to_sweep);
   SweepSpace(heap()->old_data_space(), how_to_sweep);
 
+  UnlinkEvacuationCandidates();

[Michael Starzinger, 2013/03/08 12:52:06]

Add a short comment about why this needs to be done before the sweeper threads
start.

[Hannes Payer (out of office), 2013/03/08 14:02:30]

Done.

+
   if (how_to_sweep == PARALLEL_CONSERVATIVE ||
       how_to_sweep == CONCURRENT_CONSERVATIVE) {
     // TODO(hpayer): fix race with concurrent sweeper
@@ -3924,9 +3932,7 @@ void MarkCompactCollector::SweepSpaces() {
   // Deallocate unmarked objects and clear marked bits for marked objects.
   heap_->lo_space()->FreeUnmarkedObjects();
 
-  if (how_to_sweep != CONCURRENT_CONSERVATIVE) {
-    FinalizeSweeping();
-  }
+  ReleaseEvacuationCandidates();

[Michael Starzinger, 2013/03/08 12:52:06]

The right place to call FreeQueuedChunks would be after we release evacuation
candidates, it might even make sense to move it into
ReleaseEvacuationCandidates().

[Hannes Payer (out of office), 2013/03/08 14:02:30]

Done.

 }