Author: [email protected]
Date: Thu May 7 03:19:38 2009
New Revision: 1895
Modified:
branches/bleeding_edge/src/spaces-inl.h
branches/bleeding_edge/src/spaces.cc
branches/bleeding_edge/src/spaces.h
Log:
Changed the PageIterator class so that it only returns pages existing
at construction time. If allocation during iteration causes a paged
space to expand, the iterator will not return the new pages.
This makes it more closely match the HeapObjectIterator behavior, and
it removes a possible source of bugs (if the allocation top was in the
last page in the space, the old iterator would stop only when it
reached the end of the space, potentially returning invalid pages from
a freshly expanded space).
Review URL: http://codereview.chromium.org/115074
Modified: branches/bleeding_edge/src/spaces-inl.h
==============================================================================
--- branches/bleeding_edge/src/spaces-inl.h (original)
+++ branches/bleeding_edge/src/spaces-inl.h Thu May 7 03:19:38 2009
@@ -64,15 +64,16 @@
// PageIterator
bool PageIterator::has_next() {
- return cur_page_ != stop_page_;
+ return prev_page_ != stop_page_;
}
Page* PageIterator::next() {
ASSERT(has_next());
- Page* result = cur_page_;
- cur_page_ = cur_page_->next_page();
- return result;
+ prev_page_ = (prev_page_ == NULL)
+ ? space_->first_page_
+ : prev_page_->next_page();
+ return prev_page_;
}
Modified: branches/bleeding_edge/src/spaces.cc
==============================================================================
--- branches/bleeding_edge/src/spaces.cc (original)
+++ branches/bleeding_edge/src/spaces.cc Thu May 7 03:19:38 2009
@@ -111,17 +111,17 @@
//
-----------------------------------------------------------------------------
// PageIterator
-PageIterator::PageIterator(PagedSpace* space, Mode mode) {
- cur_page_ = space->first_page_;
+PageIterator::PageIterator(PagedSpace* space, Mode mode) : space_(space) {
+ prev_page_ = NULL;
switch (mode) {
case PAGES_IN_USE:
- stop_page_ = space->AllocationTopPage()->next_page();
+ stop_page_ = space->AllocationTopPage();
break;
case PAGES_USED_BY_MC:
- stop_page_ = space->MCRelocationTopPage()->next_page();
+ stop_page_ = space->MCRelocationTopPage();
break;
case ALL_PAGES:
- stop_page_ = Page::FromAddress(NULL);
+ stop_page_ = space->last_page_;
break;
default:
UNREACHABLE();
@@ -496,8 +496,11 @@
accounting_stats_.ExpandSpace(num_pages * Page::kObjectAreaSize);
ASSERT(Capacity() <= max_capacity_);
+ // Sequentially initialize remembered sets in the newly allocated
+ // pages and cache the current last page in the space.
for (Page* p = first_page_; p->is_valid(); p = p->next_page()) {
p->ClearRSet();
+ last_page_ = p;
}
// Use first_page_ for allocation.
@@ -676,9 +679,11 @@
MemoryAllocator::SetNextPage(last_page, p);
- // Clear remembered set of new pages.
+ // Sequentially clear remembered set of new pages and and cache the
+ // new last page in the space.
while (p->is_valid()) {
p->ClearRSet();
+ last_page_ = p;
p = p->next_page();
}
@@ -723,10 +728,12 @@
Page* p = MemoryAllocator::FreePages(last_page_to_keep->next_page());
MemoryAllocator::SetNextPage(last_page_to_keep, p);
- // Since pages are only freed in whole chunks, we may have kept more than
- // pages_to_keep.
+ // Since pages are only freed in whole chunks, we may have kept more
+ // than pages_to_keep. Count the extra pages and cache the new last
+ // page in the space.
while (p->is_valid()) {
pages_to_keep++;
+ last_page_ = p;
p = p->next_page();
}
Modified: branches/bleeding_edge/src/spaces.h
==============================================================================
--- branches/bleeding_edge/src/spaces.h (original)
+++ branches/bleeding_edge/src/spaces.h Thu May 7 03:19:38 2009
@@ -511,11 +511,22 @@
//
// A HeapObjectIterator iterates objects from a given address to the
// top of a space. The given address must be below the current
-// allocation pointer (space top). If the space top changes during
-// iteration (because of allocating new objects), the iterator does
-// not iterate new objects. The caller function must create a new
-// iterator starting from the old top in order to visit these new
-// objects. Heap::Scavenage() is such an example.
+// allocation pointer (space top). There are some caveats.
+//
+// (1) If the space top changes upward during iteration (because of
+// allocating new objects), the iterator does not iterate objects
+// above the original space top. The caller must create a new
+// iterator starting from the old top in order to visit these new
+// objects. Heap::Scavenge() is such an example.
+//
+// (2) If new objects are allocated below the original allocation top
+// (e.g., free-list allocation in paged spaces), the new objects
+// may or may not be iterated depending on their position with
+// respect to the current point of iteration.
+//
+// (3) The space top should not change downward during iteration,
+// otherwise the iterator will return not-necessarily-valid
+// objects.
class HeapObjectIterator: public ObjectIterator {
public:
@@ -559,17 +570,35 @@
//
-----------------------------------------------------------------------------
-// A PageIterator iterates pages in a space.
+// A PageIterator iterates the pages in a paged space.
//
// The PageIterator class provides three modes for iterating pages in a
space:
-// PAGES_IN_USE iterates pages that are in use by the allocator;
-// PAGES_USED_BY_GC iterates pages that hold relocated objects during a
-// mark-compact collection;
+// PAGES_IN_USE iterates pages containing allocated objects.
+// PAGES_USED_BY_MC iterates pages that hold relocated objects during a
+// mark-compact collection.
// ALL_PAGES iterates all pages in the space.
+//
+// There are some caveats.
+//
+// (1) If the space expands during iteration, new pages will not be
+// returned by the iterator in any mode.
+//
+// (2) If new objects are allocated during iteration, they will appear
+// in pages returned by the iterator. Allocation may cause the
+// allocation pointer or MC allocation pointer in the last page to
+// change between constructing the iterator and iterating the last
+// page.
+//
+// (3) The space should not shrink during iteration, otherwise the
+// iterator will return deallocated pages.
class PageIterator BASE_EMBEDDED {
public:
- enum Mode {PAGES_IN_USE, PAGES_USED_BY_MC, ALL_PAGES};
+ enum Mode {
+ PAGES_IN_USE,
+ PAGES_USED_BY_MC,
+ ALL_PAGES
+ };
PageIterator(PagedSpace* space, Mode mode);
@@ -577,8 +606,9 @@
inline Page* next();
private:
- Page* cur_page_; // next page to return
- Page* stop_page_; // page where to stop
+ PagedSpace* space_;
+ Page* prev_page_; // Previous page returned.
+ Page* stop_page_; // Page to stop at (last page returned by the
iterator).
};
@@ -808,6 +838,10 @@
// The first page in this space.
Page* first_page_;
+
+ // The last page in this space. Initially set in Setup, updated in
+ // Expand and Shrink.
+ Page* last_page_;
// Normal allocation information.
AllocationInfo allocation_info_;
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
