[oilpan]: Change marking to do precise roots first and conservative second.

Source/platform/heap/Heap.cpp

Index: Source/platform/heap/Heap.cpp
diff --git a/Source/platform/heap/Heap.cpp b/Source/platform/heap/Heap.cpp
index 3b77309dd2369f8eee6f802a6265fbb66bef65da..4eb9c0ac1426f5a6b10d782c0f64c2c82aab06dd 100644
--- a/Source/platform/heap/Heap.cpp
+++ b/Source/platform/heap/Heap.cpp
@@ -2079,7 +2079,20 @@ void Heap::collectGarbage(ThreadState::StackState stackState)
 
     prepareForGC();
 
-    traceRootsAndPerformGlobalWeakProcessing<GlobalMarking>();
+    // 1. trace persistent roots.
+    ThreadState::visitPersistentRoots(s_markingVisitor);
+
+    // 2. trace object reachable from the persistent roots including ephemerons.

[haraken, 2014/07/22 11:32:43]

object => objects

[wibling-chromium, 2014/07/22 11:56:56]

Done.

+    processMarkingStack<GlobalMarking>();
+
+    // 3. trace objects reachable from the stack. We do this independent of the
+    // given stackState since other threads might have a different stack state.
+    ThreadState::visitStackRoots(s_markingVisitor);
+
+    // 4. trace objects reachable from the stack "roots" including ephemerons.

[Erik Corry, 2014/07/22 12:39:27]

This will always do at least one run through the ephemerons even if there is
nothing on the marking stack. That's correct, but it's only needed if at least
one conservative pointer was found. Since we already have a flag for that, we
could use it here to not bother calling processMarkingStack a second time.

[wibling-chromium, 2014/07/22 12:46:33]

Good idea. I guarded it by lastGCWasConservative().

+    processMarkingStack<GlobalMarking>();
+
+    globalWeakProcessingAndCleanup();
 
     // After a global marking we know that any orphaned page that was not reached
     // cannot be reached in a subsequent GC. This is due to a thread either having
@@ -2116,7 +2129,23 @@ void Heap::collectGarbageForTerminatingThread(ThreadState* state)
         state->enterGC();
         state->prepareForGC();
 
-        traceRootsAndPerformGlobalWeakProcessing<ThreadLocalMarking>();
+        // 1. trace the thread local persistent roots. For thread local GCs we
+        // don't trace the stack (ie. no conservative scanning) since this is
+        // only called during thread shutdown where there should be no objects
+        // on the stack.
+        // We also assume that orphaned pages have no objects reachable from
+        // persistent handles on other threads or CrossThreadPersistents. The
+        // only cases where this could happen is if a subsequent conservative

[haraken, 2014/07/22 11:32:43]

cases => case

[wibling-chromium, 2014/07/22 11:56:56]

I think it should be "cases" since I mention two scenarios below, ie. global
conservative GC or programming error.

+        // global GC finds a "pointer" on the stack or due to a programming
+        // error where an object has a dangling cross-thread pointer to an
+        // object on this heap.
+        state->visitPersistents(s_markingVisitor);
+
+        // 2. trace objects reachable from the thread's persistent roots
+        // including ephemerons.
+        processMarkingStack<ThreadLocalMarking>();
+
+        globalWeakProcessingAndCleanup();
 
         state->leaveGC();
     }
@@ -2124,18 +2153,14 @@ void Heap::collectGarbageForTerminatingThread(ThreadState* state)
 }
 
 template<CallbackInvocationMode Mode>
-void Heap::traceRootsAndPerformGlobalWeakProcessing()
+void Heap::processMarkingStack()
 {
-    if (Mode == ThreadLocalMarking)
-        ThreadState::current()->visitLocalRoots(s_markingVisitor);
-    else
-        ThreadState::visitRoots(s_markingVisitor);
-
     // Ephemeron fixed point loop.
     do {
-        // Recursively mark all objects that are reachable from the roots for
-        // this thread. If Mode is ThreadLocalMarking don't continue tracing if
-        // the trace hits an object on another thread's heap.
+        // Iteratively mark all objects that are reachable from the objects
+        // currently pushed onto the marking stack. If Mode is ThreadLocalMarking
+        // don't continue tracing if the trace hits an object on another thread's
+        // heap.
         while (popAndInvokeTraceCallback<Mode>(s_markingVisitor)) { }
 
         // Mark any strong pointers that have now become reachable in ephemeron
@@ -2144,7 +2169,10 @@ void Heap::traceRootsAndPerformGlobalWeakProcessing()
 
         // Rerun loop if ephemeron processing queued more objects for tracing.
     } while (!s_markingStack->isEmpty());
+}
 
+void Heap::globalWeakProcessingAndCleanup()
+{
     // Call weak callbacks on objects that may now be pointing to dead
     // objects and call ephemeronIterationDone callbacks on weak tables
     // to do cleanup (specifically clear the queued bits for weak hash