[heap] Fix rescanning live objects when aborting compaction.

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 2917 matching lines @@
     const uintptr_t kBoundary = V8_UINT64_C(1) << 48;
     STATIC_ASSERT(kBoundary > 0);
     if (reinterpret_cast<uintptr_t>(heap_obj->address()) >= kBoundary) {
       CheckLayoutDescriptorAndDie(heap, slot);
     }
 #endif
 #endif
     MapWord map_word = heap_obj->map_word();
     if (map_word.IsForwardingAddress()) {
       DCHECK(heap->InFromSpace(heap_obj) ||
-             MarkCompactCollector::IsOnEvacuationCandidate(heap_obj));
+             MarkCompactCollector::IsOnEvacuationCandidate(heap_obj) ||
+             Page::FromAddress(heap_obj->address())
+                 ->IsFlagSet(Page::COMPACTION_WAS_ABORTED));
       HeapObject* target = map_word.ToForwardingAddress();
       base::NoBarrier_CompareAndSwap(
           reinterpret_cast<base::AtomicWord*>(slot),
           reinterpret_cast<base::AtomicWord>(obj),
           reinterpret_cast<base::AtomicWord>(target));
       DCHECK(!heap->InFromSpace(target) &&
              !MarkCompactCollector::IsOnEvacuationCandidate(target));
     }
   }
 
@@ skipping 507 matching lines @@
         // 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.
         DCHECK(p->IsEvacuationCandidate());
-        p->SetFlag(Page::RESCAN_ON_EVACUATION);
+        p->SetFlag(Page::COMPACTION_WAS_ABORTED);
         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 288 matching lines @@
 
   {
     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();
   {

[Hannes Payer (out of office), 2015/10/23 13:09:18]

This function is getting super long (and complicated). Can we at least move code
into descriptive functions to have a better overview what is going on here.

[Michael Lippautz, 2015/10/23 13:27:05]

Factored out a few pieces.

     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));
 
       if (p->IsEvacuationCandidate()) {
         UpdateSlotsRecordedIn(p->slots_buffer());
         if (FLAG_trace_fragmentation_verbose) {
           PrintF("  page %p slots buffer: %d\n", reinterpret_cast<void*>(p),
                  SlotsBuffer::SizeOfChain(p->slots_buffer()));
         }
         slots_buffer_allocator_->DeallocateChain(p->slots_buffer_address());
 
         // Important: skip list should be cleared only after roots were updated
         // because root iteration traverses the stack and might have to find
         // code objects from non-updated pc pointing into evacuation candidate.
         SkipList* list = p->skip_list();
         if (list != NULL) list->Clear();
-      }
 
-      if (p->IsEvacuationCandidate() &&
-          p->IsFlagSet(Page::RESCAN_ON_EVACUATION)) {
-        // Case where we've aborted compacting a page. Clear the flag here to
-        // avoid release the page later on.
-        p->ClearEvacuationCandidate();
+        if (p->IsFlagSet(Page::COMPACTION_WAS_ABORTED)) {
+          p->ClearEvacuationCandidate();
+          // First pass on aborted pages.
+          int offsets[16];
+          for (MarkBitCellIterator it(p); !it.Done(); it.Advance()) {
+            Address cell_base = it.CurrentCellBase();
+            MarkBit::CellType* cell = it.CurrentCell();
+            if (*cell == 0) continue;
+            int live_objects = MarkWordToObjectStarts(*cell, offsets);
+            for (int i = 0; i < live_objects; i++) {
+              Address object_addr = cell_base + offsets[i] * kPointerSize;
+              HeapObject* live_object = HeapObject::FromAddress(object_addr);
+              DCHECK(Marking::IsBlack(Marking::MarkBitFrom(live_object)));
+              Map* map = live_object->synchronized_map();
+              int size = live_object->SizeFromMap(map);
+              live_object->IterateBody(map->instance_type(), size,
+                                       &updating_visitor);
+            }
+          }
+        }
       }
 
       if (p->IsFlagSet(Page::RESCAN_ON_EVACUATION)) {
         if (FLAG_gc_verbose) {
           PrintF("Sweeping 0x%" V8PRIxPTR " during evacuation.\n",
                  reinterpret_cast<intptr_t>(p));
         }
         PagedSpace* space = static_cast<PagedSpace*>(p->owner());
         p->ClearFlag(MemoryChunk::RESCAN_ON_EVACUATION);
 
@@ skipping 15 matching lines @@
             }
             break;
           default:
             UNREACHABLE();
             break;
         }
       }
     }
   }
 
   GCTracer::Scope gc_scope(heap()->tracer(),

[Hannes Payer (out of office), 2015/10/23 13:09:18]

This timer scope now also covers the sweeping.

[Michael Lippautz, 2015/10/23 13:27:05]

Fixed to only track updating the pointers.

                            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);
 
+  // Second pass on aborted pages.

[Hannes Payer (out of office), 2015/10/23 13:09:18]

Please add a separate timer scope here.

[Michael Lippautz, 2015/10/23 13:27:05]

Done.

+  for (int i = 0; i < npages; i++) {
+    Page* p = evacuation_candidates_[i];
+    if (p->IsFlagSet(Page::COMPACTION_WAS_ABORTED)) {
+      p->ClearFlag(MemoryChunk::COMPACTION_WAS_ABORTED);
+      PagedSpace* space = static_cast<PagedSpace*>(p->owner());
+      switch (space->identity()) {
+        case OLD_SPACE:
+          Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, IGNORE_SKIP_LIST,
+                IGNORE_FREE_SPACE>(space, nullptr, p, nullptr);
+          break;
+        case CODE_SPACE:
+          if (FLAG_zap_code_space) {
+            Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
+                  ZAP_FREE_SPACE>(space, NULL, p, nullptr);
+          } else {
+            Sweep<SWEEP_ONLY, SWEEP_ON_MAIN_THREAD, REBUILD_SKIP_LIST,
+                  IGNORE_FREE_SPACE>(space, NULL, p, nullptr);
+          }
+          break;
+        default:
+          UNREACHABLE();
+          break;
+      }
+    }
+  }
+
   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 751 matching lines @@
     MarkBit mark_bit = Marking::MarkBitFrom(host);
     if (Marking::IsBlack(mark_bit)) {
       RelocInfo rinfo(pc, RelocInfo::CODE_TARGET, 0, host);
       RecordRelocSlot(&rinfo, target);
     }
   }
 }
 
 }  // namespace internal
 }  // namespace v8