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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3672 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/flag-definitions.h

@@ -198,7 +198,7 @@ DEFINE_bool(cleanup_caches_in_maps_at_gc, true,
 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,

src/mark-compact.cc

@@ -1291,6 +1291,8 @@ class MapCompact {
     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 @@ class MapCompact {
 
   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)

src/spaces.h

@@ -982,6 +982,18 @@ class PagedSpace : public Space {
     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 @@ class MapSpace : public FixedSpace {
   // 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 @@ class MapSpace : public FixedSpace {
     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();

test/cctest/cctest.status

@@ -38,9 +38,6 @@ test-api/ApplyInterruption: PASS || TIMEOUT
 test-serialize/TestThatAlwaysFails: FAIL
 test-serialize/DependentTestThatAlwaysFails: FAIL
 
-# Temporary disable the test.
-test-mark-compact/MapCompact: SKIP
-
 
 [ $arch == arm ]