From cb27d09534f7ee859580f36b1df69a8e90d67608 Mon Sep 17 00:00:00 2001 From: "vegorov@chromium.org" Date: Thu, 22 Apr 2010 16:43:38 +0000 Subject: [PATCH] Fix bugs introduced by r4475: - RelinkPageListInChunkOrder might relink unused pages into the middle of a sequence of used pages. Filler objects should be placed at the beginning of such unused pages otherwise generic iterators (e.g. HeapObjectIterator) would not handle them correctly. - ObjectAreaEnd() should not be used as an allocation limit for pages from FixedSpace. Pages in such spaces do not use top page_extra_ bytes of object area. TBR=ager@chromium.org Review URL: http://codereview.chromium.org/1700005 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4476 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/mark-compact.cc | 30 ++++++++++++------------------ src/spaces.cc | 32 ++++++++++++++++++++++---------- src/spaces.h | 26 ++++++++++++++++---------- test/cctest/test-heap.cc | 2 +- 4 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/mark-compact.cc b/src/mark-compact.cc index d704e21..f144bdb 100644 --- a/src/mark-compact.cc +++ b/src/mark-compact.cc @@ -1056,21 +1056,15 @@ void MarkCompactCollector::EncodeForwardingAddressesInPagedSpace( while (it.has_next()) { Page* p = it.next(); - if (p->WasInUseBeforeMC()) { - // The offset of each live object in the page from the first live object - // in the page. - int offset = 0; - EncodeForwardingAddressesInRange( - p->ObjectAreaStart(), - p->AllocationTop(), - &offset); - } else { - // Mark whole unused page as a free region. - EncodeFreeRegion(p->ObjectAreaStart(), - p->AllocationTop() - p->ObjectAreaStart()); - } + // The offset of each live object in the page from the first live object + // in the page. + int offset = 0; + EncodeForwardingAddressesInRange( + p->ObjectAreaStart(), + p->AllocationTop(), + &offset); } } @@ -1397,17 +1391,17 @@ static void SweepSpace(PagedSpace* space, DeallocateFunction dealloc) { } if (new_allocation_top != NULL) { +#ifdef DEBUG Page* new_allocation_top_page = Page::FromAllocationTop(new_allocation_top); - ASSERT(((first_empty_page == NULL) && (new_allocation_top_page == space->AllocationTopPage())) || ((first_empty_page != NULL) && (last_free_size > 0) && (new_allocation_top_page == prec_first_empty_page)) || ((first_empty_page != NULL) && (last_free_size == 0) && (new_allocation_top_page == first_empty_page))); +#endif - space->SetTop(new_allocation_top, - new_allocation_top_page->ObjectAreaEnd()); + space->SetTop(new_allocation_top); } } diff --git a/src/spaces.cc b/src/spaces.cc index 4d838a2..c432c59 100644 --- a/src/spaces.cc +++ b/src/spaces.cc @@ -1039,7 +1039,7 @@ void PagedSpace::Verify(ObjectVisitor* visitor) { // The next page will be above the allocation top. above_allocation_top = true; } else { - ASSERT(top == current_page->ObjectAreaEnd() - page_extra_); + ASSERT(top == PageAllocationLimit(current_page)); } // It should be packed with objects from the bottom to the top. @@ -1977,12 +1977,12 @@ void PagedSpace::PrepareForMarkCompact(bool will_compact) { if (will_compact) { // MarkCompact collector relies on WAS_IN_USE_BEFORE_MC page flag // to skip unused pages. Update flag value for all pages in space. - PageIterator it(this, PageIterator::ALL_PAGES); + PageIterator all_pages_iterator(this, PageIterator::ALL_PAGES); Page* last_in_use = AllocationTopPage(); bool in_use = true; - while (it.has_next()) { - Page* p = it.next(); + while (all_pages_iterator.has_next()) { + Page* p = all_pages_iterator.next(); p->SetWasInUseBeforeMC(in_use); if (p == last_in_use) { // We passed a page containing allocation top. All consequent @@ -2005,7 +2005,7 @@ void PagedSpace::PrepareForMarkCompact(bool will_compact) { // used page so various object iterators will continue to work properly. int size_in_bytes = - last_in_use->ObjectAreaEnd() - last_in_use->AllocationTop(); + PageAllocationLimit(last_in_use) - last_in_use->AllocationTop(); if (size_in_bytes > 0) { // There is still some space left on this page. Create a fake @@ -2015,16 +2015,28 @@ void PagedSpace::PrepareForMarkCompact(bool will_compact) { FreeListNode* node = FreeListNode::FromAddress(last_in_use->AllocationTop()); - node->set_size(last_in_use->ObjectAreaEnd() - - last_in_use->AllocationTop()); + node->set_size(size_in_bytes); } // New last in use page was in the middle of the list before // sorting so it full. - SetTop(new_last_in_use->AllocationTop(), - new_last_in_use->AllocationTop()); + SetTop(new_last_in_use->AllocationTop()); ASSERT(AllocationTopPage() == new_last_in_use); + ASSERT(AllocationTopPage()->WasInUseBeforeMC()); + } + + PageIterator pages_in_use_iterator(this, PageIterator::PAGES_IN_USE); + while (pages_in_use_iterator.has_next()) { + Page* p = pages_in_use_iterator.next(); + if (!p->WasInUseBeforeMC()) { + // Empty page is in the middle of a sequence of used pages. + // Create a fake object which will occupy all free space on this page. + // Otherwise iterators would not be able to scan this page correctly. + FreeListNode* node = + FreeListNode::FromAddress(p->ObjectAreaStart()); + node->set_size(PageAllocationLimit(p) - p->ObjectAreaStart()); + } } page_list_is_chunk_ordered_ = true; @@ -2544,7 +2556,7 @@ HeapObject* FixedSpace::SlowAllocateRaw(int size_in_bytes) { HeapObject* FixedSpace::AllocateInNextPage(Page* current_page, int size_in_bytes) { ASSERT(current_page->next_page()->is_valid()); - ASSERT(current_page->ObjectAreaEnd() - allocation_info_.top == page_extra_); + ASSERT(allocation_info_.top == PageAllocationLimit(current_page)); ASSERT_EQ(object_size_in_bytes_, size_in_bytes); accounting_stats_.WasteBytes(page_extra_); SetAllocationInfo(&allocation_info_, current_page->next_page()); diff --git a/src/spaces.h b/src/spaces.h index cc02e6c..1720f09 100644 --- a/src/spaces.h +++ b/src/spaces.h @@ -927,7 +927,14 @@ class PagedSpace : public Space { // Prepares for a mark-compact GC. virtual void PrepareForMarkCompact(bool will_compact); - virtual Address PageAllocationTop(Page* page) = 0; + // The top of allocation in a page in this space. Undefined if page is unused. + Address PageAllocationTop(Page* page) { + return page == TopPageOf(allocation_info_) ? top() + : PageAllocationLimit(page); + } + + // The limit of allocation for a page in this space. + virtual Address PageAllocationLimit(Page* page) = 0; // Current capacity without growing (Size() + Available() + Waste()). int Capacity() { return accounting_stats_.Capacity(); } @@ -970,9 +977,9 @@ class PagedSpace : public Space { void FreePages(Page* prev, Page* last); // Set space allocation info. - void SetTop(Address top, Address limit) { + void SetTop(Address top) { allocation_info_.top = top; - allocation_info_.limit = limit; + allocation_info_.limit = PageAllocationLimit(Page::FromAllocationTop(top)); } // --------------------------------------------------------------------------- @@ -1724,9 +1731,9 @@ class OldSpace : public PagedSpace { // pointer). int AvailableFree() { return free_list_.available(); } - // The top of allocation in a page in this space. Undefined if page is unused. - virtual Address PageAllocationTop(Page* page) { - return page == TopPageOf(allocation_info_) ? top() : page->ObjectAreaEnd(); + // The limit of allocation for a page in this space. + virtual Address PageAllocationLimit(Page* page) { + return page->ObjectAreaEnd(); } // Give a block of memory to the space's free list. It might be added to @@ -1792,10 +1799,9 @@ class FixedSpace : public PagedSpace { page_extra_ = Page::kObjectAreaSize % object_size_in_bytes; } - // The top of allocation in a page in this space. Undefined if page is unused. - virtual Address PageAllocationTop(Page* page) { - return page == TopPageOf(allocation_info_) ? top() - : page->ObjectAreaEnd() - page_extra_; + // The limit of allocation for a page in this space. + virtual Address PageAllocationLimit(Page* page) { + return page->ObjectAreaEnd() - page_extra_; } int object_size_in_bytes() { return object_size_in_bytes_; } diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index d0a9b0f..31060bf 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -830,7 +830,7 @@ TEST(LargeObjectSpaceContains) { } CHECK(bytes_to_page > FixedArray::kHeaderSize); - intptr_t* flags_ptr = &Page::FromAddress(next_page)->flags; + int* flags_ptr = &Page::FromAddress(next_page)->flags; Address flags_addr = reinterpret_cast
(flags_ptr); int bytes_to_allocate = -- 2.7.4