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 7141de5e93ad3a585bbc646eb75728f9d7c2616a..2fc623c2a202ef4ac24382f1e5d340caa29ea38d 100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -469,14 +469,16 @@ bool ThreadState::checkAndMarkPointer(Visitor* visitor, Address address)
     if (m_isCleaningUp)
         return false;
 
+    if (Heap::notInHeap(address))
+        return false;
+
     BaseHeapPage* page = heapPageFromAddress(address);

[wibling-chromium, 2014/05/08 07:43:41]

NIT: Perhaps add a comment that this will check both "normal" and large objects
since the variable name doesn't indicate the latter.

[Erik Corry, 2014/05/08 09:26:08]

Done.

-    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);
+        return true;
     }
+
+    Heap::addressIsNotInHeap(address);
     return false;
 }
 
@@ -487,12 +489,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
@@ -526,6 +522,8 @@ static bool increasedEnoughToGC(size_t newSize, size_t oldSize)
 {
     if (newSize < 2 * blinkPagePayloadSize())
         return false;
+    if (newSize < (1 << 20))

[haraken, 2014/05/08 05:44:58]

What is this condition for?

[Mads Ager (chromium), 2014/05/08 06:52:37]

Could we do tuning of the GC heuristics separately from this change? Let's
introduce the negative cache first. Then we can tune these as a follow-up (and
update the comments when we do.)

[Erik Corry, 2014/05/08 09:26:08]

Done.

[Erik Corry, 2014/05/08 09:26:08]

Heuristic.  Removed for now.

+        return false;
     return newSize > oldSize + (oldSize >> 1);
 }
 
@@ -654,9 +652,8 @@ void ThreadState::prepareForGC()
 
 BaseHeapPage* ThreadState::heapPageFromAddress(Address address)
 {
-    BaseHeapPage* page;
-    bool found = heapContainsCache()->lookup(address, &page);
-    if (found)
+    BaseHeapPage* page = heapContainsCache()->lookup(address);
+    if (page)
         return page;
 
     for (int i = 0; i < NumberOfHeaps; i++) {
@@ -664,26 +661,12 @@ BaseHeapPage* ThreadState::heapPageFromAddress(Address address)
 #ifndef NDEBUG
         Address blinkPageAddr = roundToBlinkPageStart(address);
 #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)
+        ASSERT(!page || page->isLargeObject() || page == m_heaps[i]->heapPageFromAddress(blinkPageAddr));

[haraken, 2014/05/08 05:44:58]

heapPageFromAddress(roundToBlinkPageStart(address))

[Erik Corry, 2014/05/08 09:26:08]

Done.

+        ASSERT(!page || page->isLargeObject() || page == m_heaps[i]->heapPageFromAddress(blinkPageAddr + blinkPageSize - 1));

[haraken, 2014/05/08 05:44:58]

heapPageFromAddress(roundToBlinkPageEnd(address))

[Erik Corry, 2014/05/08 09:26:08]

Done.

+        if (page) {

[haraken, 2014/05/08 05:44:58]

You can add the two ASSERTs into this if branch. Then you won't need to check
!page.

[Erik Corry, 2014/05/08 09:26:08]

Done.

+            heapContainsCache()->addEntry(address, page);
             return page;
+        }
     }
     return 0;
 }