Revision: 3672
Author: [email protected]
Date: Thu Jan 21 06:22:28 2010
Log: 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.
Review URL: http://codereview.chromium.org/552066
http://code.google.com/p/v8/source/detail?r=3672
Modified:
/branches/bleeding_edge/src/flag-definitions.h
/branches/bleeding_edge/src/mark-compact.cc
/branches/bleeding_edge/src/spaces.h
/branches/bleeding_edge/test/cctest/cctest.status
=======================================
--- /branches/bleeding_edge/src/flag-definitions.h Thu Jan 21 03:28:11 2010
+++ /branches/bleeding_edge/src/flag-definitions.h Thu Jan 21 06:22:28 2010
@@ -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,
=======================================
--- /branches/bleeding_edge/src/mark-compact.cc Wed Jan 13 11:16:07 2010
+++ /branches/bleeding_edge/src/mark-compact.cc Thu Jan 21 06:22:28 2010
@@ -1291,6 +1291,8 @@
MapIterator it;
HeapObject* o = it.next();
for (; o != first_map_to_evacuate_; o = it.next()) {
+ it.has_next(); // Must be called for side-effects, see bug 586.
+ 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(); // Must be called for side-effects, see bug 586.
ASSERT(it->has_next());
HeapObject* next = it->next();
if (next == last)
=======================================
--- /branches/bleeding_edge/src/spaces.h Tue Jan 19 08:34:37 2010
+++ /branches/bleeding_edge/src/spaces.h Thu Jan 21 06:22:28 2010
@@ -981,6 +981,18 @@
static Page* TopPageOf(AllocationInfo alloc_info) {
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
@@ -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(); // Must be called for side-effects, see bug 586.
ASSERT(it.has_next());
it.next()->ClearRSet();
}
+ it.has_next(); // Must be called for side-effects, see bug 586.
ASSERT(it.has_next());
Page* top_page = it.next();
top_page->ClearRSet();
=======================================
--- /branches/bleeding_edge/test/cctest/cctest.status Tue Jan 19 15:03:37
2010
+++ /branches/bleeding_edge/test/cctest/cctest.status Thu Jan 21 06:22:28
2010
@@ -38,9 +38,6 @@
test-serialize/TestThatAlwaysFails: FAIL
test-serialize/DependentTestThatAlwaysFails: FAIL
-# Temporary disable the test.
-test-mark-compact/MapCompact: SKIP
-
[ $arch == arm ]
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
