Simplify and speed up address-to-page cache for conservative stack scanning.

Source/platform/heap/ThreadState.cpp

Index: Source/platform/heap/ThreadState.cpp
diff --git a/Source/platform/heap/ThreadState.cpp b/Source/platform/heap/ThreadState.cpp
index 25b50dd9a0fe927b6899bf912eb54c4ccfae8507..09c3a11b12b7fa890779236d5e9677fd38c9b539 100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -469,14 +469,18 @@ bool ThreadState::checkAndMarkPointer(Visitor* visitor, Address address)
     if (m_isCleaningUp)
         return false;
 
+    // This checks for normal pages and for large objects which span the extent
+    // of several normal pages.
     BaseHeapPage* page = heapPageFromAddress(address);
-    if (page)
-        return page->checkAndMarkPointer(visitor, address);
-    // Not in heap pages, check large objects
-    for (int i = 0; i < NumberOfHeaps; i++) {
-        if (m_heaps[i]->checkAndMarkLargeHeapObject(visitor, address))
-            return true;
+    if (page) {
+        page->checkAndMarkPointer(visitor, address);
+        // Whether or not the pointer was within an object it was certainly
+        // within a page that is part of the heap, so we don't want to ask the
+        // other other heaps or put this address in the

[wibling-chromium, 2014/05/09 10:25:20]

other other -> other

+        // HeapDoesNotContainCache.
+        return true;
     }
+
     return false;
 }
 
@@ -487,12 +491,6 @@ const GCInfo* ThreadState::findGCInfo(Address address)
     if (page) {
         return page->findGCInfo(address);
     }
-
-    // Not in heap pages, check large objects
-    for (int i = 0; i < NumberOfHeaps; i++) {
-        if (const GCInfo* info = m_heaps[i]->findGCInfoOfLargeHeapObject(address))
-            return info;
-    }
     return 0;
 }
 #endif
@@ -654,37 +652,28 @@ void ThreadState::prepareForGC()
 
 BaseHeapPage* ThreadState::heapPageFromAddress(Address address)
 {
-    BaseHeapPage* page;
-    bool found = heapContainsCache()->lookup(address, &page);
-    if (found)
-        return page;
-
-    for (int i = 0; i < NumberOfHeaps; i++) {
-        page = m_heaps[i]->heapPageFromAddress(address);
-#ifndef NDEBUG
-        Address blinkPageAddr = roundToBlinkPageStart(address);
+    BaseHeapPage* cachedPage = heapContainsCache()->lookup(address);
+#ifdef NDEBUG

[wibling-chromium, 2014/05/09 10:25:20]

Should this be removed as well?

+    if (cachedPage)
+        return cachedPage;
 #endif
-        ASSERT(page == m_heaps[i]->heapPageFromAddress(blinkPageAddr));
-        ASSERT(page == m_heaps[i]->heapPageFromAddress(blinkPageAddr + blinkPageSize - 1));
-        if (page)
-            break;
-    }
-    heapContainsCache()->addEntry(address, page);
-    return page; // 0 if not found.
-}
 
-BaseHeapPage* ThreadState::contains(Address address)
-{
-    // Check heap contains cache first.
-    BaseHeapPage* page = heapPageFromAddress(address);
-    if (page)
-        return page;
-    // If no heap page was found check large objects.
     for (int i = 0; i < NumberOfHeaps; i++) {
-        page = m_heaps[i]->largeHeapObjectFromAddress(address);
-        if (page)
+        BaseHeapPage* page = m_heaps[i]->heapPageFromAddress(address);
+        if (page) {
+            // Asserts that make sure heapPageFromAddress takes addresses from
+            // the whole aligned blinkPageSize memory area. This is necessary
+            // for the negative cache to work.
+            ASSERT(page->isLargeObject() || page == m_heaps[i]->heapPageFromAddress(roundToBlinkPageStart(address)));
+            if (roundToBlinkPageStart(address) != roundToBlinkPageEnd(address))
+                ASSERT(page->isLargeObject() || page == m_heaps[i]->heapPageFromAddress(roundToBlinkPageEnd(address) - 1));
+            ASSERT(!cachedPage || page == cachedPage);
+            if (!cachedPage)
+                heapContainsCache()->addEntry(address, page);
             return page;
+        }
     }
+    ASSERT(!cachedPage);
     return 0;
 }