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;
 
+    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;
 }
 
 #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 13 matching lines @@
     AtomicallyInitializedStatic(Mutex&, mutex = *new Mutex);
     return mutex;
 }
 
 // Trigger garbage collection on a 50% increase in size, but not for
 // less than 2 pages.
 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);
 }
 
 // FIXME: The heuristics are local for a thread at this
 // point. Consider using heuristics that take memory for all threads
 // into account.
 bool ThreadState::shouldGC()
 {
     // Do not GC during sweeping. We allow allocation during
     // finalization, but those allocations are not allowed
@@ skipping 108 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)
+    BaseHeapPage* page = heapContainsCache()->lookup(address);
+    if (page)
         return page;
 
     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;
-    }
-    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;
 }
 
 void ThreadState::getStats(HeapStats& stats)
 {
     stats = m_stats;
 #ifndef NDEBUG
     if (isConsistentForGC()) {
         HeapStats scannedStats;
@@ skipping 153 matching lines @@
     state->safePoint(HeapPointersOnStack);
 }
 
 ThreadState::AttachedThreadStateSet& ThreadState::attachedThreads()
 {
     DEFINE_STATIC_LOCAL(AttachedThreadStateSet, threads, ());
     return threads;
 }
 
 }