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

Source/platform/heap/ThreadState.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 451 matching lines @@
         visitStack(visitor);
     visitPersistents(visitor);
 }
 
 bool ThreadState::checkAndMarkPointer(Visitor* visitor, Address address)
 {
     // If thread is cleaning up ignore conservative pointers.
     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;
 }
 
 #if ENABLE(GC_TRACING)
 const GCInfo* ThreadState::findGCInfo(Address address)
 {
     BaseHeapPage* page = heapPageFromAddress(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
 
 void ThreadState::pushWeakObjectPointerCallback(void* object, WeakPointerCallback callback)
 {
     CallbackStack::Item* slot = m_weakCallbackStack->allocateEntry(&m_weakCallbackStack);
     *slot = CallbackStack::Item(object, callback);
 }
 
@@ skipping 141 matching lines @@
         // This happens if a thread did not have time to wake up and sweep,
         // before the next GC arrived.
         if (sweepRequested())
             heap->clearMarks();
     }
     setSweepRequested();
 }
 
 BaseHeapPage* ThreadState::heapPageFromAddress(Address address)
 {
-    BaseHeapPage* page;
-    bool found = heapContainsCache()->lookup(address, &page);
-    if (found)
-        return page;
+    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
 
     for (int i = 0; i < NumberOfHeaps; i++) {
-        page = m_heaps[i]->heapPageFromAddress(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;
+        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;
+        }
     }
-    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)
-            return page;
-    }
+    ASSERT(!cachedPage);
     return 0;
 }
 
 void ThreadState::getStats(HeapStats& stats)
 {
     stats = m_stats;
 #ifndef NDEBUG
     if (isConsistentForGC()) {
         HeapStats scannedStats;
         scannedStats.clear();
@@ skipping 152 matching lines @@
     state->safePoint(HeapPointersOnStack);
 }
 
 ThreadState::AttachedThreadStateSet& ThreadState::attachedThreads()
 {
     DEFINE_STATIC_LOCAL(AttachedThreadStateSet, threads, ());
     return threads;
 }
 
 }