Revision: 9011
Author: [email protected]
Date: Thu Aug 25 03:42:12 2011
Log: Fix lazy sweeping for new marking bit clearing.
This cleans the semantics of the WAS_SWEPT_* page flags. Pages swept
precisely can be iterated, hitting only the live objects. Whereas those
swept conservatively cannot be iterated over. Both flags indicate that
marking bits have been cleared by the sweeper, otherwise marking bits
are still intact.
The actual fix is in SetPagesToSweep, which had a buggy iteration and
set the WAS_SWEPT_CONSERVATIVELY flag on too many pages. Another fix
is in EvacuateLiveObjectsFromPage which clears marking bits now.
[email protected],[email protected]
BUG=v8:1463
TEST=cctest/test-api/Threading
Review URL: http://codereview.chromium.org/7720024
http://code.google.com/p/v8/source/detail?r=9011
Modified:
/branches/experimental/gc/src/mark-compact.cc
/branches/experimental/gc/src/spaces.cc
/branches/experimental/gc/src/spaces.h
=======================================
--- /branches/experimental/gc/src/mark-compact.cc Tue Aug 23 09:03:55 2011
+++ /branches/experimental/gc/src/mark-compact.cc Thu Aug 25 03:42:12 2011
@@ -2494,12 +2494,10 @@
void MarkCompactCollector::EvacuateLiveObjectsFromPage(Page* p) {
AlwaysAllocateScope always_allocate;
-
- ASSERT(p->IsEvacuationCandidate() && !p->WasSwept());
-
PagedSpace* space = static_cast<PagedSpace*>(p->owner());
-
+ ASSERT(p->IsEvacuationCandidate() && !p->WasSwept());
MarkBit::CellType* cells = p->markbits()->cells();
+ p->MarkSweptPrecisely();
int last_cell_index =
Bitmap::IndexToCell(
@@ -2536,6 +2534,9 @@
space->identity());
ASSERT(object->map_word().IsForwardingAddress());
}
+
+ // Clear marking bits for current cell.
+ cells[cell_index] = 0;
}
}
@@ -2614,9 +2615,8 @@
// if requested.
static void SweepPrecisely(PagedSpace* space, Page* p, SweepingMode mode) {
ASSERT(!p->IsEvacuationCandidate() && !p->WasSwept());
- ASSERT(!p->IsFlagSet(MemoryChunk::WAS_SWEPT_CONSERVATIVELY));
MarkBit::CellType* cells = p->markbits()->cells();
- p->MarkSwept();
+ p->MarkSweptPrecisely();
int last_cell_index =
Bitmap::IndexToCell(
@@ -2711,6 +2711,10 @@
SlotsBuffer::SizeOfChain(p->slots_buffer()));
}
} else {
+ 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);
SweepPrecisely(space, p, SWEEP_AND_UPDATE_SLOTS);
@@ -2761,8 +2765,6 @@
p->set_scan_on_scavenge(false);
slots_buffer_allocator_.DeallocateChain(p->slots_buffer_address());
p->ClearEvacuationCandidate();
- p->MarkSwept();
- p->ClearFlag(MemoryChunk::WAS_SWEPT_CONSERVATIVELY);
}
evacuation_candidates_.Rewind(0);
compacting_ = false;
@@ -3095,7 +3097,7 @@
intptr_t MarkCompactCollector::SweepConservatively(PagedSpace* space,
Page* p) {
ASSERT(!p->IsEvacuationCandidate() && !p->WasSwept());
MarkBit::CellType* cells = p->markbits()->cells();
- p->SetFlag(MemoryChunk::WAS_SWEPT_CONSERVATIVELY);
+ p->MarkSweptConservatively();
int last_cell_index =
Bitmap::IndexToCell(
@@ -3189,10 +3191,15 @@
intptr_t freed_bytes = 0;
intptr_t newspace_size = space->heap()->new_space()->Size();
+ bool lazy_sweeping_active = false;
while (it.has_next()) {
Page* p = it.next();
+ // Clear sweeping flags indicating that marking bits are still intact.
+ p->ClearSweptPrecisely();
+ p->ClearSweptConservatively();
+
if (p->IsEvacuationCandidate()) {
ASSERT(evacuation_candidates_.length() > 0);
continue;
@@ -3202,6 +3209,20 @@
// Will be processed in EvacuateNewSpaceAndCandidates.
continue;
}
+
+ if (lazy_sweeping_active) {
+ if (FLAG_gc_verbose) {
+ PrintF("Sweeping 0x%" V8PRIxPTR " lazily postponed.\n",
+ reinterpret_cast<intptr_t>(p));
+ }
+ continue;
+ }
+
+ if (FLAG_gc_verbose) {
+ PrintF("Sweeping 0x%" V8PRIxPTR " with sweeper %d.\n",
+ reinterpret_cast<intptr_t>(p),
+ sweeper);
+ }
switch (sweeper) {
case CONSERVATIVE: {
@@ -3209,11 +3230,10 @@
break;
}
case LAZY_CONSERVATIVE: {
- Page* next_page = p->next_page();
freed_bytes += SweepConservatively(space, p);
if (freed_bytes >= newspace_size && p != space->LastPage()) {
- space->SetPagesToSweep(next_page, space->LastPage());
- return;
+ space->SetPagesToSweep(p->next_page(), space->LastPage());
+ lazy_sweeping_active = true;
}
break;
}
=======================================
--- /branches/experimental/gc/src/spaces.cc Tue Aug 23 09:03:55 2011
+++ /branches/experimental/gc/src/spaces.cc Thu Aug 25 03:42:12 2011
@@ -79,7 +79,7 @@
page->ObjectAreaEnd(),
kOnePageOnly,
size_func);
- ASSERT(!page->IsFlagSet(Page::WAS_SWEPT_CONSERVATIVELY));
+ ASSERT(page->WasSweptPrecisely());
}
@@ -118,7 +118,7 @@
if (cur_page == space_->anchor()) return false;
cur_addr_ = cur_page->ObjectAreaStart();
cur_end_ = cur_page->ObjectAreaEnd();
- ASSERT(!cur_page->IsFlagSet(Page::WAS_SWEPT_CONSERVATIVELY));
+ ASSERT(cur_page->WasSweptPrecisely());
return true;
}
@@ -450,6 +450,7 @@
chunk->slots_buffer_ = NULL;
Bitmap::Clear(chunk);
chunk->initialize_scan_on_scavenge(false);
+ chunk->SetFlag(WAS_SWEPT_PRECISELY);
ASSERT(OFFSET_OF(MemoryChunk, flags_) == kFlagsOffset);
@@ -790,7 +791,7 @@
if (page == Page::FromAllocationTop(allocation_info_.top)) {
allocation_pointer_found_in_space = true;
}
- ASSERT(!page->IsFlagSet(MemoryChunk::WAS_SWEPT_CONSERVATIVELY));
+ ASSERT(page->WasSweptPrecisely());
HeapObjectIterator it(page, NULL);
Address end_of_previous_object = page->ObjectAreaStart();
Address top = page->ObjectAreaEnd();
@@ -1930,32 +1931,26 @@
Free(top(), old_linear_size);
SetTop(NULL, NULL);
- // Stop lazy sweeping and clear marking bits for the space.
+ // Stop lazy sweeping and clear marking bits for unswept pages.
if (first_unswept_page_ != NULL) {
- int pages = 0;
Page* last = last_unswept_page_->next_page();
Page* p = first_unswept_page_;
do {
- pages++;
- Bitmap::Clear(p);
+ if (ShouldBeSweptLazily(p)) {
+ ASSERT(!p->WasSwept());
+ Bitmap::Clear(p);
+ if (FLAG_gc_verbose) {
+ PrintF("Sweeping 0x%" V8PRIxPTR " lazily abandoned.\n",
+ reinterpret_cast<intptr_t>(p));
+ }
+ }
p = p->next_page();
} while (p != last);
- if (FLAG_trace_gc) {
- PrintF("Abandoned %d unswept pages\n", pages);
- }
}
first_unswept_page_ = last_unswept_page_ = Page::FromAddress(NULL);
// Clear the free list before a full GC---it will be rebuilt afterward.
free_list_.Reset();
-
- // Clear WAS_SWEPT and WAS_SWEPT_CONSERVATIVELY flags from all pages.
- PageIterator it(this);
- while (it.has_next()) {
- Page* page = it.next();
- page->ClearSwept();
- page->ClearFlag(MemoryChunk::WAS_SWEPT_CONSERVATIVELY);
- }
}
@@ -1997,10 +1992,11 @@
Page* p = first_unswept_page_;
do {
Page* next_page = p->next_page();
- // Evacuation candidates were swept by evacuator.
- if (!p->IsEvacuationCandidate() &&
- !p->IsFlagSet(Page::RESCAN_ON_EVACUATION) &&
- !p->WasSwept()) {
+ if (ShouldBeSweptLazily(p)) {
+ if (FLAG_gc_verbose) {
+ PrintF("Sweeping 0x%" V8PRIxPTR " lazily advanced.\n",
+ reinterpret_cast<intptr_t>(p));
+ }
freed_bytes += MarkCompactCollector::SweepConservatively(this, p);
}
p = next_page;
=======================================
--- /branches/experimental/gc/src/spaces.h Tue Aug 23 06:33:22 2011
+++ /branches/experimental/gc/src/spaces.h Thu Aug 25 03:42:12 2011
@@ -365,7 +365,6 @@
enum MemoryChunkFlags {
IS_EXECUTABLE,
- WAS_SWEPT_CONSERVATIVELY,
ABOUT_TO_BE_FREED,
POINTERS_TO_HERE_ARE_INTERESTING,
POINTERS_FROM_HERE_ARE_INTERESTING,
@@ -376,7 +375,15 @@
CONTAINS_ONLY_DATA,
EVACUATION_CANDIDATE,
RESCAN_ON_EVACUATION,
- WAS_SWEPT,
+
+ // Pages swept precisely can be iterated, hitting only the live
objects.
+ // Whereas those swept conservatively cannot be iterated over. Both
flags
+ // indicate that marking bits have been cleared by the sweeper,
otherwise
+ // marking bits are still intact.
+ WAS_SWEPT_PRECISELY,
+ WAS_SWEPT_CONSERVATIVELY,
+
+ // Last flag, keep at bottom.
NUM_MEMORY_CHUNK_FLAGS
};
@@ -655,11 +662,15 @@
void InitializeAsAnchor(PagedSpace* owner);
- bool WasSwept() { return IsFlagSet(WAS_SWEPT); }
-
- void MarkSwept() { SetFlag(WAS_SWEPT); }
-
- void ClearSwept() { ClearFlag(WAS_SWEPT); }
+ bool WasSweptPrecisely() { return IsFlagSet(WAS_SWEPT_PRECISELY); }
+ bool WasSweptConservatively() { return
IsFlagSet(WAS_SWEPT_CONSERVATIVELY); }
+ bool WasSwept() { return WasSweptPrecisely() ||
WasSweptConservatively(); }
+
+ void MarkSweptPrecisely() { SetFlag(WAS_SWEPT_PRECISELY); }
+ void MarkSweptConservatively() { SetFlag(WAS_SWEPT_CONSERVATIVELY); }
+
+ void ClearSweptPrecisely() { ClearFlag(WAS_SWEPT_PRECISELY); }
+ void ClearSweptConservatively() { ClearFlag(WAS_SWEPT_CONSERVATIVELY); }
friend class MemoryAllocator;
};
@@ -1423,24 +1434,18 @@
bool was_swept_conservatively() { return was_swept_conservatively_; }
void set_was_swept_conservatively(bool b) { was_swept_conservatively_ =
b; }
+
+ // Evacuation candidates are swept by evacuator. Needs to return a valid
+ // result before _and_ after evacuation has finished.
+ static bool ShouldBeSweptLazily(Page* p) {
+ return !p->IsEvacuationCandidate() &&
+ !p->IsFlagSet(Page::RESCAN_ON_EVACUATION) &&
+ !p->WasSweptPrecisely();
+ }
void SetPagesToSweep(Page* first, Page* last) {
first_unswept_page_ = first;
last_unswept_page_ = last;
- int unswept_pages = 0;
-
- Page* p = first;
- do {
- // The WAS_SWEPT_CONSERVATIVELY flag means that we can't iterate
over the
- // page. We have to set this flag on the pages to indicate this.
- p->SetFlag(MemoryChunk::WAS_SWEPT_CONSERVATIVELY);
- p = p->next_page();
- unswept_pages++;
- } while (p != last);
-
- if (FLAG_trace_gc_verbose) {
- PrintF("Postponing sweep for %d pages\n", unswept_pages);
- }
}
bool AdvanceSweeper(intptr_t bytes_to_sweep);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
