Reviewers: sgjess_chromium.org,
Message:
Søren,
I was surprised to find necessity to invoke has_next as otherwise iterator
goes
wild.
For now I fixed it by stupid it.has_next() invocations. More principal
approach
could be to blend has_next and next into one method (e.g. making next
return 0
when space is exhausted.) If you think it's a reasonable idea, I'd
willingly
implement it, but I'd prefer to do that in another CL to ease rolling back
if
need would be.
Description:
Fix map compact implementation.
Always invoke HeapObjectIterator::has_next() before invoking
HeapObjectIterator::next().
This is necessary as ::has_next() has an important side-effect of going to
the
next
page when current page is exhausted.
And to find if pointers are encodable use more precise data---top of map
space,
not a number
of pages, as pages might stay in map space due to chunking.
Please review this at http://codereview.chromium.org/552066
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/flag-definitions.h
M src/mark-compact.cc
M src/spaces.h
M test/cctest/cctest.status
Index: test/cctest/cctest.status
===================================================================
--- test/cctest/cctest.status (revision 3658)
+++ test/cctest/cctest.status (working copy)
@@ -38,10 +38,7 @@
test-serialize/TestThatAlwaysFails: FAIL
test-serialize/DependentTestThatAlwaysFails: FAIL
-# Temporary disable the test.
-test-mark-compact/MapCompact: SKIP
-
[ $arch == arm ]
# BUG(240): Test seems flaky on ARM.
Index: src/spaces.h
===================================================================
--- src/spaces.h (revision 3658)
+++ src/spaces.h (working copy)
@@ -982,6 +982,18 @@
return Page::FromAllocationTop(alloc_info.limit);
}
+ int CountPagesToTop() {
+ Page* p = Page::FromAllocationTop(allocation_info_.top);
+ PageIterator it(this, PageIterator::ALL_PAGES);
+ int counter = 1;
+ while (it.has_next()) {
+ if (it.next() == p) return counter;
+ counter++;
+ }
+ UNREACHABLE();
+ return -1;
+ }
+
// Expands the space by allocating a fixed number of pages. Returns
false if
// it cannot allocate requested number of pages from OS. Newly allocated
// pages are append to the last_page;
@@ -1770,12 +1782,10 @@
// Are map pointers encodable into map word?
bool MapPointersEncodable() {
if (!FLAG_use_big_map_space) {
- ASSERT(CountTotalPages() <= kMaxMapPageIndex);
+ ASSERT(CountPagesToTop() <= kMaxMapPageIndex);
return true;
}
- int n_of_pages = Capacity() / Page::kObjectAreaSize;
- ASSERT(n_of_pages == CountTotalPages());
- return n_of_pages <= max_map_space_pages_;
+ return CountPagesToTop() <= max_map_space_pages_;
}
// Should be called after forced sweep to find out if map space needs
@@ -1790,9 +1800,11 @@
int pages_left = live_maps / kMapsPerPage;
PageIterator it(this, PageIterator::ALL_PAGES);
while (pages_left-- > 0) {
+ it.has_next();
ASSERT(it.has_next());
it.next()->ClearRSet();
}
+ it.has_next();
ASSERT(it.has_next());
Page* top_page = it.next();
top_page->ClearRSet();
Index: src/flag-definitions.h
===================================================================
--- src/flag-definitions.h (revision 3658)
+++ src/flag-definitions.h (working copy)
@@ -198,7 +198,7 @@
DEFINE_bool(canonicalize_object_literal_maps, true,
"Canonicalize maps for object literals.")
-DEFINE_bool(use_big_map_space, false,
+DEFINE_bool(use_big_map_space, true,
"Use big map space, but don't compact if it grew too big.")
DEFINE_int(max_map_space_pages, MapSpace::kMaxMapPageIndex - 1,
Index: src/mark-compact.cc
===================================================================
--- src/mark-compact.cc (revision 3658)
+++ src/mark-compact.cc (working copy)
@@ -1291,6 +1291,8 @@
MapIterator it;
HeapObject* o = it.next();
for (; o != first_map_to_evacuate_; o = it.next()) {
+ it.has_next();
+ ASSERT(it.has_next());
Map* map = reinterpret_cast<Map*>(o);
ASSERT(!map->IsMarked());
ASSERT(!map->IsOverflowed());
@@ -1362,6 +1364,7 @@
static Map* NextMap(MapIterator* it, HeapObject* last, bool live) {
while (true) {
+ it->has_next(); // Need to be called for side effects.
ASSERT(it->has_next());
HeapObject* next = it->next();
if (next == last)
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
