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

Source/platform/heap/Heap.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 2061 matching lines @@
     static_cast<MarkingVisitor*>(s_markingVisitor)->objectGraph().clear();
 #endif
 
     // Disallow allocation during garbage collection (but not
     // during the finalization that happens when the gcScope is
     // torn down).
     NoAllocationScope<AnyThread> noAllocationScope;
 
     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
     // swept its heap or having done a "poor mans sweep" in prepareForGC which marks
     // objects that are dead, but not swept in the previous GC as dead. In this GC's
     // marking we check that any object marked as dead is not traced. E.g. via a
     // conservatively found pointer or a programming error with an object containing
     // a dangling pointer.
     orphanedPagePool()->decommitOrphanedPages();
 
@@ skipping 16 matching lines @@
     // We explicitly do not enter a safepoint while doing thread specific
     // garbage collection since we don't want to allow a global GC at the
     // same time as a thread local GC.
 
     {
         NoAllocationScope<AnyThread> noAllocationScope;
 
         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();
     }
     state->performPendingSweep();
 }
 
 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
         // maps.
         CallbackStack::invokeCallbacks(&s_ephemeronStack, s_markingVisitor);
 
         // 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
     // tables).
     while (popAndInvokeWeakPointerCallback(s_markingVisitor)) { }
 
     CallbackStack::clear(&s_ephemeronStack);
 
     // It is not permitted to trace pointers of live objects in the weak
     // callback phase, so the marking stack should still be empty here.
@@ skipping 84 matching lines @@
 Visitor* Heap::s_markingVisitor;
 CallbackStack* Heap::s_markingStack;
 CallbackStack* Heap::s_weakCallbackStack;
 CallbackStack* Heap::s_ephemeronStack;
 HeapDoesNotContainCache* Heap::s_heapDoesNotContainCache;
 bool Heap::s_shutdownCalled = false;
 bool Heap::s_lastGCWasConservative = false;
 FreePagePool* Heap::s_freePagePool;
 OrphanedPagePool* Heap::s_orphanedPagePool;
 }