Oilpan: support eager tracing of objects when marking.

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 563 matching lines @@
         if (objectFields[i] != 0)
             return false;
     }
     return true;
 }
 #endif
 
 template<>
 void LargeObject<FinalizedHeapObjectHeader>::mark(Visitor* visitor)
 {
-    if (heapObjectHeader()->hasVTable() && !vTableInitialized(payload())) {
-        FinalizedHeapObjectHeader* header = heapObjectHeader();
+    FinalizedHeapObjectHeader* header = heapObjectHeader();
+    if (header->hasVTable() && !vTableInitialized(payload())) {
         visitor->markNoTracing(header);
         ASSERT(isUninitializedMemory(header->payload(), header->payloadSize()));
     } else {
-        visitor->mark(heapObjectHeader(), heapObjectHeader()->traceCallback());
+        visitor->mark(header, header->traceCallback());
     }
 }
 
 template<>
 void LargeObject<HeapObjectHeader>::mark(Visitor* visitor)
 {
     ASSERT(gcInfo());
     if (gcInfo()->hasVTable() && !vTableInitialized(payload())) {
         HeapObjectHeader* header = heapObjectHeader();
         visitor->markNoTracing(header);
@@ skipping 901 matching lines @@
 {
     setAllocationPoint(0, 0);
     clearFreeLists();
 }
 
 template<typename Header>
 void ThreadHeap<Header>::markUnmarkedObjectsDead()
 {
     ASSERT(Heap::isInGC());
     ASSERT(isConsistentForSweeping());
     for (HeapPage<Header>* page = m_firstPage; page; page = page->next()) {

[kouhei (in TOK), 2014/12/02 00:41:58]

Nit: Unintended {}?

[kouhei (in TOK), 2014/12/02 06:51:02]

Sorry, this was unrelated to your CL. Please ignore.

[sof, 2014/12/02 09:52:15]

Your bracing preference is identical to mine though :)

         page->markUnmarkedObjectsDead();
     }
     for (LargeObject<Header>* largeObject = m_firstLargeObject; largeObject; largeObject = largeObject->next()) {
         largeObject->markUnmarkedObjectsDead();
     }
 }
 
 template<typename Header>
 void ThreadHeap<Header>::clearFreeLists()
 {
@@ skipping 440 matching lines @@
 }
 
 class MarkingVisitor final : public Visitor {
 public:
 #if ENABLE(GC_PROFILE_MARKING)
     typedef HashSet<uintptr_t> LiveObjectSet;
     typedef HashMap<String, LiveObjectSet> LiveObjectMap;
     typedef HashMap<uintptr_t, std::pair<uintptr_t, String> > ObjectGraph;
 #endif
 
-    MarkingVisitor(CallbackStack* markingStack) : m_markingStack(markingStack)
+    MarkingVisitor(CallbackStack* markingStack)

[haraken, 2014/12/02 06:16:04]

Add explicit.

[sof, 2014/12/02 09:52:15]

Done.

+        : m_markingStack(markingStack)
+        , m_checkIfNeedsTracing(false)
     {
     }
 
     inline void visitHeader(HeapObjectHeader* header, const void* objectPointer, TraceCallback callback)
     {
         ASSERT(header);
-#if ENABLE(ASSERT)
-        {
-            // Check that we are not marking objects that are outside
-            // the heap by calling Heap::contains. However we cannot
-            // call Heap::contains when outside a GC and we call mark
-            // when doing weakness for ephemerons. Hence we only check
-            // when called within.
-            ASSERT(!Heap::isInGC() || Heap::containedInHeapOrOrphanedPage(header));
-        }
-#endif
         ASSERT(objectPointer);
+        // Check that we are not marking objects that are outside
+        // the heap by calling Heap::contains. However we cannot
+        // call Heap::contains when outside a GC and we call mark
+        // when doing weakness for ephemerons. Hence we only check
+        // when called within.
+        ASSERT(!Heap::isInGC() || Heap::containedInHeapOrOrphanedPage(header));
+
         if (header->isMarked())
             return;
         header->mark();
+
 #if ENABLE(GC_PROFILE_MARKING)
         MutexLocker locker(objectGraphMutex());
         String className(classOf(objectPointer));
         {
             LiveObjectMap::AddResult result = currentlyLive().add(className, LiveObjectSet());
             result.storedValue->value.add(reinterpret_cast<uintptr_t>(objectPointer));
         }
         ObjectGraph::AddResult result = objectGraph().add(reinterpret_cast<uintptr_t>(objectPointer), std::make_pair(reinterpret_cast<uintptr_t>(m_hostObject), m_hostName));
         ASSERT(result.isNewEntry);
         // fprintf(stderr, "%s[%p] -> %s[%p]\n", m_hostName.ascii().data(), m_hostObject, className.ascii().data(), objectPointer);
 #endif
-        if (callback)
+#if ENABLE(ASSERT)
+        {
+            BaseHeapPage* page = pageFromObject(objectPointer);
+            // If you hit this ASSERT, it means that there is a dangling pointer
+            // from a live thread heap to a dead thread heap. We must eliminate
+            // the dangling pointer.
+            // Release builds don't have the ASSERT, but it is OK because
+            // release builds will crash at the following item->call

[haraken, 2014/12/02 06:16:04]

Update this comment. Now we don't have "the following item->call".

+            // because all the entries of the orphaned heaps are zeroed out and
+            // thus the item does not have a valid vtable.
+            ASSERT(!page->orphaned());
+        }
+#endif
+        if (callback) {
+            if (UNLIKELY(m_checkIfNeedsTracing && !needsTracing(objectPointer)))
+                return;
+
             Heap::pushTraceCallback(m_markingStack, const_cast<void*>(objectPointer), callback);
+        }
     }
 
+    // We need both HeapObjectHeader and FinalizedHeapObjectHeader versions to correctly find the payload.
     virtual void mark(HeapObjectHeader* header, TraceCallback callback) override
     {
-        // We need both the HeapObjectHeader and FinalizedHeapObjectHeader
-        // version to correctly find the payload.
         visitHeader(header, header->payload(), callback);
     }
 
     virtual void mark(FinalizedHeapObjectHeader* header, TraceCallback callback) override
     {
-        // We need both the HeapObjectHeader and FinalizedHeapObjectHeader
-        // version to correctly find the payload.
         visitHeader(header, header->payload(), callback);
     }
 
     virtual void mark(const void* objectPointer, TraceCallback callback) override
     {
         if (!objectPointer)
             return;
         FinalizedHeapObjectHeader* header = FinalizedHeapObjectHeader::fromPayload(objectPointer);
         visitHeader(header, header->payload(), callback);
     }
@@ skipping 18 matching lines @@
     {
         return Heap::weakTableRegistered(closure);
     }
 #endif
 
     virtual bool isMarked(const void* objectPointer) override
     {
         return FinalizedHeapObjectHeader::fromPayload(objectPointer)->isMarked();
     }
 
+    virtual bool ensureMarked(const void* objectPointer) override
+    {
+        if (!objectPointer)
+            return false;
+#if ENABLE(ASSERT)
+        if (isMarked(objectPointer))
+            return false;
+
+        markNoTracing(objectPointer);
+#else
+        FinalizedHeapObjectHeader* header =
+            FinalizedHeapObjectHeader::fromPayload(objectPointer);
+        if (header->isMarked())
+            return false;
+        header->mark();
+
+        if (UNLIKELY(m_checkIfNeedsTracing && !needsTracing(objectPointer)))

[haraken, 2014/12/02 06:16:04]

Hmm, it's a bit unfortunate we need to have this branch. In the previous code,
this branch was needed only for the mark() method in the ThreadLocalMarking
mode. Would it be possible to keep the templatized Mode and avoid adding this
branch to the GlobalMarking mode?

[sof, 2014/12/02 06:19:27]

As I said in the previous comment, I don't see a way to do that without having
separate MarkingVisitor for these two modes (either by parameterizing
MarkingVisitor over a type, or making visitHeader() virtual.)

[sof, 2014/12/02 09:52:15]

Had a go at the former; acceptable?

+            return false;
+#endif
+        return true;
+    }
+
+#if ENABLE(ASSERT)
+#define DEFINE_ENSURE_MARKED_METHOD(Type)                                                       \
+    virtual bool ensureMarked(const Type* objectPointer) override                               \
+    {                                                                                           \
+        if (!objectPointer)                                                                     \
+            return false;                                                                       \
+        COMPILE_ASSERT(!NeedsAdjustAndMark<Type>::value, CanOnlyUseIsMarkedOnNonAdjustedTypes); \
+        if (isMarked(objectPointer))                                                            \
+            return false;                                                                       \
+        markNoTracing(objectPointer);                                                           \
+        return true;                                                                            \
+    }
+#else
+#define DEFINE_ENSURE_MARKED_METHOD(Type)                                    \
+    virtual bool ensureMarked(const Type* objectPointer) override            \
+    {                                                                        \
+        if (!objectPointer)                                                  \
+            return false;                                                    \
+        HeapObjectHeader* header =                                           \
+            HeapObjectHeader::fromPayload(objectPointer);                    \
+        if (header->isMarked())                                              \
+            return false;                                                    \
+        header->mark();                                                      \
+        if (UNLIKELY(m_checkIfNeedsTracing && !needsTracing(objectPointer))) \
+            return false;                                                    \
+        return true;                                                         \
+    }
+#endif
+
     // This macro defines the necessary visitor methods for typed heaps
 #define DEFINE_VISITOR_METHODS(Type)                                              \
     virtual void mark(const Type* objectPointer, TraceCallback callback) override \
     {                                                                             \
         if (!objectPointer)                                                       \
             return;                                                               \
         HeapObjectHeader* header =                                                \
             HeapObjectHeader::fromPayload(objectPointer);                         \
         visitHeader(header, header->payload(), callback);                         \
     }                                                                             \
     virtual bool isMarked(const Type* objectPointer) override                     \
     {                                                                             \
         return HeapObjectHeader::fromPayload(objectPointer)->isMarked();          \
-    }
+    }                                                                             \
+    DEFINE_ENSURE_MARKED_METHOD(Type)
 
     FOR_EACH_TYPED_HEAP(DEFINE_VISITOR_METHODS)
 #undef DEFINE_VISITOR_METHODS
 
 #if ENABLE(GC_PROFILE_MARKING)
     void reportStats()
     {
         fprintf(stderr, "\n---------- AFTER MARKING -------------------\n");
         for (LiveObjectMap::iterator it = currentlyLive().begin(), end = currentlyLive().end(); it != end; ++it) {
             fprintf(stderr, "%s %u", it->key.ascii().data(), it->value.size());
@@ skipping 80 matching lines @@
         return graph;
     }
 
     static HashSet<uintptr_t>& objectsToFindPath()
     {
         DEFINE_STATIC_LOCAL(HashSet<uintptr_t>, set, ());
         return set;
     }
 #endif
 
+    static inline bool needsTracing(const void* objectPointer)
+    {
+        BaseHeapPage* page = pageFromObject(objectPointer);
+        ASSERT(!page->orphaned());
+        // When doing a thread local GC, the marker checks if
+        // the object resides in another thread's heap. The
+        // object should not be traced, if it does.
+        if (!page->terminating())
+            return false;
+
+        return true;
+    }
+
 protected:
     virtual void registerWeakCell(void** cell, WeakPointerCallback callback) override
     {
         Heap::pushWeakCellPointerCallback(cell, callback);
     }
 
 private:
+    friend class ThreadLocalMarkingScope;
+
     CallbackStack* m_markingStack;
+    bool m_checkIfNeedsTracing;
+};
+
+class ThreadLocalMarkingScope {
+    STACK_ALLOCATED();
+public:
+    ThreadLocalMarkingScope(MarkingVisitor* visitor)
+        : m_visitor(visitor)
+    {
+        ASSERT(m_visitor);
+        m_visitor->m_checkIfNeedsTracing = true;
+    }
+
+    ~ThreadLocalMarkingScope()
+    {
+        m_visitor->m_checkIfNeedsTracing = false;
+    }
+
+private:
+    MarkingVisitor* m_visitor;
 };
 
 void Heap::init()
 {
     ThreadState::init();
     s_markingStack = new CallbackStack();
     s_postMarkingCallbackStack = new CallbackStack();
     s_weakCallbackStack = new CallbackStack();
     s_ephemeronStack = new CallbackStack();
     s_heapDoesNotContainCache = new HeapDoesNotContainCache();
@@ skipping 132 matching lines @@
         builder.append("\n\t");
         builder.append(frameToName.nullableName());
         --framesToShow;
     }
     return builder.toString().replace("blink::", "");
 }
 #endif
 
 void Heap::pushTraceCallback(CallbackStack* stack, void* object, TraceCallback callback)
 {
-#if ENABLE(ASSERT)
-    {
-        ASSERT(Heap::containedInHeapOrOrphanedPage(object));
-    }
-#endif
+    ASSERT(Heap::containedInHeapOrOrphanedPage(object));
     CallbackStack::Item* slot = stack->allocateEntry();
     *slot = CallbackStack::Item(object, callback);
 }
 
-template<CallbackInvocationMode Mode>
 bool Heap::popAndInvokeTraceCallback(CallbackStack* stack, Visitor* visitor)
 {
     CallbackStack::Item* item = stack->pop();
     if (!item)
         return false;
-#if ENABLE(ASSERT)
-    if (Mode == GlobalMarking) {
-        BaseHeapPage* page = pageFromObject(item->object());
-        // If you hit this ASSERT, it means that there is a dangling pointer
-        // from a live thread heap to a dead thread heap. We must eliminate
-        // the dangling pointer.
-        // Release builds don't have the ASSERT, but it is OK because
-        // release builds will crash at the following item->call
-        // because all the entries of the orphaned heaps are zeroed out and
-        // thus the item does not have a valid vtable.
-        ASSERT(!page->orphaned());
-    }
-#endif
-    if (Mode == ThreadLocalMarking) {
-        BaseHeapPage* page = pageFromObject(item->object());
-        ASSERT(!page->orphaned());
-        // When doing a thread local GC, don't trace an object located in
-        // a heap of another thread.
-        if (!page->terminating())
-            return true;
-    }
 
 #if ENABLE(GC_PROFILE_MARKING)
     visitor->setHostInfo(item->object(), classOf(item->object()));
 #endif
     item->call(visitor);
     return true;
 }
 
 void Heap::pushPostMarkingCallback(void* object, TraceCallback callback)
 {
@@ skipping 115 matching lines @@
     enterGC();
     preGC();
 
     Heap::resetMarkedObjectSize();
     Heap::resetAllocatedObjectSize();
 
     // 1. trace persistent roots.
     ThreadState::visitPersistentRoots(s_markingVisitor);
 
     // 2. trace objects reachable from the persistent roots including ephemerons.
-    processMarkingStack<GlobalMarking>();
+    processMarkingStack();
 
     // 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.
     // Only do the processing if we found a pointer to an object on one of the
     // thread stacks.
-    if (lastGCWasConservative()) {
-        processMarkingStack<GlobalMarking>();
-    }
+    if (lastGCWasConservative())
+        processMarkingStack();
 
     postMarkingProcessing();
     globalWeakProcessing();
 
     // Now we can delete all orphaned pages because there are no dangling
     // pointers to the orphaned pages. (If we have such dangling pointers,
     // we should have crashed during marking before getting here.)
     orphanedPagePool()->decommitOrphanedPages();
 
     postGC();
@@ skipping 18 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;
 
         enterGC();
         state->preGC();
 
-        // 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
-        // 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);
+        {
+            ThreadLocalMarkingScope markingScope(static_cast<MarkingVisitor*>(s_markingVisitor));
 
-        // 2. trace objects reachable from the thread's persistent roots
-        // including ephemerons.
-        processMarkingStack<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
+            // 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();
+        }
 
         postMarkingProcessing();
         globalWeakProcessing();
 
         state->postGC();
         leaveGC();
     }
     state->performPendingSweep();
 }
 
-template<CallbackInvocationMode Mode>
 void Heap::processMarkingStack()
 {
     // Ephemeron fixed point loop.
     do {
         {
             // 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.
+            // currently pushed onto the marking stack.
             TRACE_EVENT0("blink_gc", "Heap::processMarkingStackSingleThreaded");
-            while (popAndInvokeTraceCallback<Mode>(s_markingStack, s_markingVisitor)) { }
+            while (popAndInvokeTraceCallback(s_markingStack, s_markingVisitor)) { }
         }
 
         {
             // Mark any strong pointers that have now become reachable in ephemeron
             // maps.
             TRACE_EVENT0("blink_gc", "Heap::processEphemeronStack");
             s_ephemeronStack->invokeEphemeronCallbacks(s_markingVisitor);
         }
 
         // Rerun loop if ephemeron processing queued more objects for tracing.
@@ skipping 245 matching lines @@
 bool Heap::s_lastGCWasConservative = false;
 bool Heap::s_inGC = false;
 FreePagePool* Heap::s_freePagePool;
 OrphanedPagePool* Heap::s_orphanedPagePool;
 Heap::RegionTree* Heap::s_regionTree = 0;
 size_t Heap::s_allocatedObjectSize = 0;
 size_t Heap::s_allocatedSpace = 0;
 size_t Heap::s_markedObjectSize = 0;
 
 } // namespace blink