Oilpan: Always enter a GCScope when we run a GC

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 91 matching lines @@
 
 #if ENABLE(GC_PROFILING)
 static String classOf(const void* object)
 {
     if (const GCInfo* gcInfo = Heap::findGCInfo(reinterpret_cast<Address>(const_cast<void*>(object))))
         return gcInfo->m_className;
     return "unknown";
 }
 #endif
 
-class GCScope {
+class GCScope final {
 public:
-    explicit GCScope(ThreadState::StackState stackState)
+    GCScope(ThreadState::StackState stackState, ThreadState::GCType gcType)
         : m_state(ThreadState::current())
-        , m_safePointScope(stackState)
+        , m_gcType(gcType)
         , m_parkedAllThreads(false)
     {
         TRACE_EVENT0("blink_gc", "Heap::GCScope");
         const char* samplingState = TRACE_EVENT_GET_SAMPLING_STATE();
         if (m_state->isMainThread())
             TRACE_EVENT_SET_SAMPLING_STATE("blink_gc", "BlinkGCWaiting");
 
         m_state->checkThread();
 
-        // FIXME: in an unlikely coincidence that two threads decide
-        // to collect garbage at the same time, avoid doing two GCs in
-        // a row.
-        if (LIKELY(ThreadState::stopThreads())) {
-            m_parkedAllThreads = true;
+        // 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.
+        if (gcType != ThreadState::ThreadTerminationGC) {
+            RELEASE_ASSERT(!ThreadState::current()->isAtSafePoint());
+            m_state->enterSafePoint(stackState, this);
+
+            // FIXME: in an unlikely coincidence that two threads decide

[sof, 2015/05/29 13:14:07]

nit: s/FIXME/TODO/

[haraken, 2015/05/29 14:42:27]

Done.

+            // to collect garbage at the same time, avoid doing two GCs in
+            // a row.
+            if (LIKELY(ThreadState::stopThreads())) {
+                m_parkedAllThreads = true;
+            }
         }
+
         if (m_state->isMainThread())
             TRACE_EVENT_SET_NONCONST_SAMPLING_STATE(samplingState);
     }
 
     bool allThreadsParked() { return m_parkedAllThreads; }
 
     ~GCScope()
     {
-        // Only cleanup if we parked all threads in which case the GC happened
-        // and we need to resume the other threads.
-        if (LIKELY(m_parkedAllThreads)) {
-            ThreadState::resumeThreads();
+        if (m_gcType != ThreadState::ThreadTerminationGC) {
+            // Only cleanup if we parked all threads in which case the GC happened
+            // and we need to resume the other threads.
+            if (LIKELY(m_parkedAllThreads)) {
+                ThreadState::resumeThreads();
+            }
+
+            m_state->leaveSafePoint();
         }
     }
 
 private:
     ThreadState* m_state;
-    SafePointScope m_safePointScope;
+    ThreadState::GCType m_gcType;
     bool m_parkedAllThreads; // False if we fail to park all threads
 };
 
 #if ENABLE(ASSERT)
 NO_SANITIZE_ADDRESS
 void HeapObjectHeader::zapMagic()
 {
     checkHeader();
     m_magic = zappedMagic;
 }
@@ skipping 1720 matching lines @@
 }
 
 void Heap::collectGarbage(ThreadState::StackState stackState, ThreadState::GCType gcType, GCReason reason)
 {
     ThreadState* state = ThreadState::current();
     RELEASE_ASSERT(!state->isInGC());
     state->completeSweep();
     ThreadState::GCState originalGCState = state->gcState();
     state->setGCState(ThreadState::StoppingOtherThreads);
 
-    GCScope gcScope(stackState);
+    GCScope gcScope(stackState, gcType);
     // Check if we successfully parked the other threads.  If not we bail out of
     // the GC.
     if (!gcScope.allThreadsParked()) {
         // Restore the original GCState.
         if (LIKELY(state->gcState() == ThreadState::StoppingOtherThreads))
             state->setGCState(originalGCState);
         return;
     }
 
     if (state->isMainThread())
@@ skipping 61 matching lines @@
     Platform::current()->histogramEnumeration("BlinkGC.GCReason", reason, NumberOfGCReason);
     Heap::reportMemoryUsageHistogram();
     WTF::Partitions::reportMemoryUsageHistogram();
 
     if (state->isMainThread())
         ScriptForbiddenScope::exit();
 }
 
 void Heap::collectGarbageForTerminatingThread(ThreadState* state)
 {
-    // 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.
     {
+        GCScope gcScope(ThreadState::NoHeapPointersOnStack, ThreadState::ThreadTerminationGC);

[sof, 2015/05/29 13:14:07]

Just to make sure I understand motivation -- this GCScope will provide some
value once GCScope starts flagging GCForbidden? It doesn't do much now apart
from a trace call on enter.

[haraken, 2015/05/29 14:42:27]

Sorry for not being clear.

I want to implement GCScope::current()->gcType() in pushXXXCallbacks() to know
the type of a GC that the GCing thread is currently doing. To let
GCScope::current() return the current GCScope, we need to make sure that all GCs
enter a GCScope.

+
         MarkingVisitor<Visitor::ThreadLocalMarking> markingVisitor;
         ThreadState::NoAllocationScope noAllocationScope(state);
 
         state->preGC();
         StackFrameDepthScope stackDepthScope;
 
         // 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.
@@ skipping 248 matching lines @@
 size_t Heap::s_allocatedObjectSize = 0;
 size_t Heap::s_allocatedSpace = 0;
 size_t Heap::s_markedObjectSize = 0;
 // We don't want to use 0 KB for the initial value because it may end up
 // triggering the first GC of some thread too prematurely.
 size_t Heap::s_estimatedLiveObjectSize = 512 * 1024;
 size_t Heap::s_externalObjectSizeAtLastGC = 0;
 double Heap::s_estimatedMarkingTimePerByte = 0.0;
 
 } // namespace blink