Oilpan: Introduce a state transition model for Oilpan GC states

Oilpan: Introduce a state transition model for Oilpan GC states

Currently we have three boolean flags to maintain GC states (i.e., m_gcRequested, m_sweepRequested, m_inGC). Incremental sweeping will add a couple of more boolean flags. This is confusing, so this CL introduces a GCState, which maintains a state transition of GC states. See RELEASE_ASSERTs in ThreadState::setGCState() for possible state transitions.

BUG=420515
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=186029

[haraken, 2014-11-26 01:50:31]

haraken@chromium.org changed reviewers:
+ oilpan-reviews@chromium.org, sigbjornf@opera.com

[haraken, 2014-11-26 01:50:56]

PTAL

[tkent, 2014-11-26 02:27:06]

tkent@chromium.org changed reviewers:
+ tkent@chromium.org

[tkent, 2014-11-26 02:27:07]

> Also this CL moves ThreadState::s_inGC to Heap::s_inGC.

Why?  This movement makes the patch larger unnecessarily.

https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
File Source/platform/heap/ThreadState.h (left):

https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
Source/platform/heap/ThreadState.h:405: void setGCRequested();
nit: I'd like to keep setGCRequested() as an alias of setGCState(GCScheduled)
for code readability.
(and rename it to requestGC() optionally)

https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
File Source/platform/heap/ThreadState.h (right):

https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
Source/platform/heap/ThreadState.h:328: GCRunning,
What's 'GC' here? Does this contains both of marking and sweeping?

[haraken, 2014-11-26 02:43:28]

On 2014/11/26 02:27:07, tkent wrote:
> > Also this CL moves ThreadState::s_inGC to Heap::s_inGC.
> 
> Why?  This movement makes the patch larger unnecessarily.

ok, I'll make the change first.

>
https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
> File Source/platform/heap/ThreadState.h (left):
> 
>
https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
> Source/platform/heap/ThreadState.h:405: void setGCRequested();
> nit: I'd like to keep setGCRequested() as an alias of setGCState(GCScheduled)
> for code readability.
> (and rename it to requestGC() optionally)

Makes sense, will do.

>
https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
> File Source/platform/heap/ThreadState.h (right):
> 
>
https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
> Source/platform/heap/ThreadState.h:328: GCRunning,
> What's 'GC' here? Does this contains both of marking and sweeping?

In oilpan's infrastructure, "GC" means a phase where other threads are in safe
points. "GC" includes a marking phase, global weak processings and other
operations that need to be done in a stop-the-world manner, but doesn't include
thread-local weak processings and a sweeping phase.

[haraken, 2014-11-26 05:27:01]

Rebased with ToT.

tkent-san: Would you take another look?

https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
File Source/platform/heap/ThreadState.h (left):

https://codereview.chromium.org/747363005/diff/40001/Source/platform/heap/Thr...
Source/platform/heap/ThreadState.h:405: void setGCRequested();
On 2014/11/26 02:27:07, tkent wrote:
> nit: I'd like to keep setGCRequested() as an alias of setGCState(GCScheduled)
> for code readability.
> (and rename it to requestGC() optionally)
> 

Done.

[tkent, 2014-11-26 05:36:41]

lgtm

[haraken, 2014-11-26 05:39:50]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-11-26 05:40:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/747363005/80001

[commit-bot: I haz the power, 2014-11-26 07:08:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-26 07:08:38]

Failed to apply patch for Source/platform/heap/ThreadState.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/platform/heap/ThreadState.cpp
  Hunk #1 FAILED at 326.
  Hunk #3 succeeded at 765 (offset -4 lines).
  Hunk #4 succeeded at 812 (offset -4 lines).
  Hunk #5 succeeded at 863 (offset -4 lines).
  Hunk #6 succeeded at 874 (offset -4 lines).
  Hunk #7 succeeded at 1016 (offset -4 lines).
  Hunk #8 succeeded at 1074 (offset -4 lines).
  Hunk #9 succeeded at 1157 (offset -3 lines).
  1 out of 9 hunks FAILED -- saving rejects to file
Source/platform/heap/ThreadState.cpp.rej

Patch:       Source/platform/heap/ThreadState.cpp
Index: Source/platform/heap/ThreadState.cpp
diff --git a/Source/platform/heap/ThreadState.cpp
b/Source/platform/heap/ThreadState.cpp
index
08f8dd9d794253b254cbda242faf8fe65d5aafc5..f364b7dd0f976f0fdb8243002bf5c5579bc94bfe
100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -326,16 +326,14 @@ ThreadState::ThreadState()
     , m_safePointScopeMarker(0)
     , m_atSafePoint(false)
     , m_interruptors()
-    , m_gcRequested(false)
     , m_didV8GCAfterLastGC(false)
     , m_forcePreciseGCForTesting(false)
-    , m_sweepRequested(0)
     , m_sweepInProgress(false)
     , m_noAllocationCount(0)
-    , m_inGC(false)
     , m_isTerminating(false)
     , m_shouldFlushHeapDoesNotContainCache(false)
     , m_lowCollectionRate(false)
+    , m_gcState(NoGCScheduled)
     , m_traceDOMWrappers(0)
 #if defined(ADDRESS_SANITIZER)
     , m_asanFakeStack(__asan_get_current_fake_stack())
@@ -358,6 +356,7 @@ ThreadState::ThreadState()
 ThreadState::~ThreadState()
 {
     checkThread();
+    ASSERT(gcState() == NoGCScheduled);
     delete m_weakCallbackStack;
     m_weakCallbackStack = 0;
     for (int i = 0; i < NumberOfHeaps; i++)
@@ -768,43 +767,38 @@ bool ThreadState::shouldForceConservativeGC()
     return newSize >= 32 * 1024 * 1024 && newSize > 4 *
Heap::markedObjectSize();
 }
 
-bool ThreadState::sweepRequested()
+void ThreadState::setGCState(GCState gcState)
 {
-    ASSERT(Heap::isInGC() || checkThread());
-    return m_sweepRequested;
-}
-
-void ThreadState::setSweepRequested()
-{
-    // Sweep requested is set from the thread that initiates garbage
-    // collection which could be different from the thread for this
-    // thread state. Therefore the setting of m_sweepRequested needs a
-    // barrier.
-    atomicTestAndSetToOne(&m_sweepRequested);
-}
-
-void ThreadState::clearSweepRequested()
-{
-    checkThread();
-    m_sweepRequested = 0;
-}
-
-bool ThreadState::gcRequested()
-{
-    checkThread();
-    return m_gcRequested;
-}
-
-void ThreadState::setGCRequested()
-{
-    checkThread();
-    m_gcRequested = true;
+    switch (gcState) {
+    case NoGCScheduled:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == Sweeping);
+        break;
+    case GCScheduled:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == NoGCScheduled || m_gcState == GCScheduled
|| m_gcState == StoppingOtherThreads);
+        break;
+    case StoppingOtherThreads:
+        checkThread();
+        break;
+    case GCRunning:
+        break;
+    case SweepScheduled:
+        RELEASE_ASSERT(m_gcState == GCRunning);
+        break;
+    case Sweeping:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == SweepScheduled);
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
+    m_gcState = gcState;
 }
 
-void ThreadState::clearGCRequested()
+ThreadState::GCState ThreadState::gcState() const
 {
-    checkThread();
-    m_gcRequested = false;
+    return m_gcState;
 }
 
 void ThreadState::didV8GC()
@@ -820,7 +814,7 @@ void ThreadState::performPendingGC(StackState stackState)
         if (forcePreciseGCForTesting()) {
             setForcePreciseGCForTesting(false);
             Heap::collectAllGarbage();
-        } else if (gcRequested()) {
+        } else if (gcState() == GCScheduled) {
             Heap::collectGarbage(NoHeapPointersOnStack);
         }
     }
@@ -871,7 +865,7 @@ void ThreadState::flushHeapDoesNotContainCacheIfNeeded()
     }
 }
 
-void ThreadState::prepareForGC()
+void ThreadState::preGC()
 {
     for (int i = 0; i < NumberOfHeaps; i++) {
         BaseHeap* heap = m_heaps[i];
@@ -882,14 +876,19 @@ void ThreadState::prepareForGC()
         // not trace already dead objects. If we trace a dead object we could
end up tracing
         // into garbage or the middle of another object via the newly
conservatively found
         // object.
-        if (sweepRequested())
+        if (gcState() == ThreadState::SweepScheduled)
             heap->markUnmarkedObjectsDead();
     }
     prepareRegionTree();
-    setSweepRequested();
+    setGCState(ThreadState::GCRunning);
     flushHeapDoesNotContainCacheIfNeeded();
 }
 
+void ThreadState::postGC()
+{
+    setGCState(ThreadState::SweepScheduled);
+}
+
 void ThreadState::setupHeapsForTermination()
 {
     checkThread();
@@ -1019,8 +1018,9 @@ void ThreadState::copyStackUntilSafePointScope()
 void ThreadState::performPendingSweep()
 {
     checkThread();
-    if (!sweepRequested())
+    if (gcState() != SweepScheduled)
         return;
+    setGCState(Sweeping);
 
 #if ENABLE(GC_PROFILE_HEAP)
     // We snapshot the heap prior to sweeping to get numbers for both resources
@@ -1076,9 +1076,8 @@ void ThreadState::performPendingSweep()
             m_heaps[i]->postSweepProcessing();
     }
 
-    clearGCRequested();
     m_didV8GCAfterLastGC = false;
-    clearSweepRequested();
+    setGCState(ThreadState::NoGCScheduled);
 
     // If we collected less than 50% of objects, record that the collection
rate
     // is low which we use to determine when to perform the next GC.
@@ -1159,7 +1158,7 @@ void ThreadState::invokePreFinalizers(Visitor& visitor)
 #if ENABLE(GC_PROFILE_MARKING)
 const GCInfo* ThreadState::findGCInfoFromAllThreads(Address address)
 {
-    bool needLockForIteration = !isInGC();
+    bool needLockForIteration = !Heap::isInGC();
     if (needLockForIteration)
         threadAttachMutex().lock();

[commit-bot: I haz the power, 2014-11-26 07:08:46]

The CQ bit was unchecked by commit-bot@chromium.org

[haraken, 2014-11-26 07:17:39]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-11-26 07:18:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/747363005/100001

[commit-bot: I haz the power, 2014-11-26 07:18:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-26 07:19:00]

Failed to apply patch for Source/platform/heap/ThreadState.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/platform/heap/ThreadState.cpp
  Hunk #1 FAILED at 326.
  Hunk #3 succeeded at 765 (offset -1 lines).
  Hunk #4 succeeded at 812 (offset -1 lines).
  Hunk #5 succeeded at 863 (offset -1 lines).
  Hunk #6 succeeded at 874 (offset -1 lines).
  Hunk #7 succeeded at 1016 (offset -1 lines).
  Hunk #8 succeeded at 1074 (offset -1 lines).
  1 out of 9 hunks FAILED -- saving rejects to file
Source/platform/heap/ThreadState.cpp.rej

Patch:       Source/platform/heap/ThreadState.cpp
Index: Source/platform/heap/ThreadState.cpp
diff --git a/Source/platform/heap/ThreadState.cpp
b/Source/platform/heap/ThreadState.cpp
index
5da13c31557c80ea764570e02c0f18286bc7b74f..2fa4512b58c92e74ee57f92161432d61635244eb
100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -326,16 +326,14 @@ ThreadState::ThreadState()
     , m_safePointScopeMarker(0)
     , m_atSafePoint(false)
     , m_interruptors()
-    , m_gcRequested(false)
     , m_didV8GCAfterLastGC(false)
     , m_forcePreciseGCForTesting(false)
-    , m_sweepRequested(0)
     , m_sweepInProgress(false)
     , m_noAllocationCount(0)
-    , m_inGC(false)
     , m_isTerminating(false)
     , m_shouldFlushHeapDoesNotContainCache(false)
     , m_lowCollectionRate(false)
+    , m_gcState(NoGCScheduled)
     , m_traceDOMWrappers(0)
 #if defined(ADDRESS_SANITIZER)
     , m_asanFakeStack(__asan_get_current_fake_stack())
@@ -358,6 +356,7 @@ ThreadState::ThreadState()
 ThreadState::~ThreadState()
 {
     checkThread();
+    ASSERT(gcState() == NoGCScheduled);
     delete m_weakCallbackStack;
     m_weakCallbackStack = 0;
     for (int i = 0; i < NumberOfHeaps; i++)
@@ -765,43 +764,38 @@ bool ThreadState::shouldForceConservativeGC()
     return newSize >= 32 * 1024 * 1024 && newSize > 4 *
Heap::markedObjectSize();
 }
 
-bool ThreadState::sweepRequested()
+void ThreadState::setGCState(GCState gcState)
 {
-    ASSERT(Heap::isInGC() || checkThread());
-    return m_sweepRequested;
-}
-
-void ThreadState::setSweepRequested()
-{
-    // Sweep requested is set from the thread that initiates garbage
-    // collection which could be different from the thread for this
-    // thread state. Therefore the setting of m_sweepRequested needs a
-    // barrier.
-    atomicTestAndSetToOne(&m_sweepRequested);
-}
-
-void ThreadState::clearSweepRequested()
-{
-    checkThread();
-    m_sweepRequested = 0;
-}
-
-bool ThreadState::gcRequested()
-{
-    checkThread();
-    return m_gcRequested;
-}
-
-void ThreadState::setGCRequested()
-{
-    checkThread();
-    m_gcRequested = true;
+    switch (gcState) {
+    case NoGCScheduled:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == Sweeping);
+        break;
+    case GCScheduled:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == NoGCScheduled || m_gcState == GCScheduled
|| m_gcState == StoppingOtherThreads);
+        break;
+    case StoppingOtherThreads:
+        checkThread();
+        break;
+    case GCRunning:
+        break;
+    case SweepScheduled:
+        RELEASE_ASSERT(m_gcState == GCRunning);
+        break;
+    case Sweeping:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == SweepScheduled);
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
+    m_gcState = gcState;
 }
 
-void ThreadState::clearGCRequested()
+ThreadState::GCState ThreadState::gcState() const
 {
-    checkThread();
-    m_gcRequested = false;
+    return m_gcState;
 }
 
 void ThreadState::didV8GC()
@@ -817,7 +811,7 @@ void ThreadState::performPendingGC(StackState stackState)
         if (forcePreciseGCForTesting()) {
             setForcePreciseGCForTesting(false);
             Heap::collectAllGarbage();
-        } else if (gcRequested()) {
+        } else if (gcState() == GCScheduled) {
             Heap::collectGarbage(NoHeapPointersOnStack);
         }
     }
@@ -868,7 +862,7 @@ void ThreadState::flushHeapDoesNotContainCacheIfNeeded()
     }
 }
 
-void ThreadState::prepareForGC()
+void ThreadState::preGC()
 {
     for (int i = 0; i < NumberOfHeaps; i++) {
         BaseHeap* heap = m_heaps[i];
@@ -879,14 +873,19 @@ void ThreadState::prepareForGC()
         // not trace already dead objects. If we trace a dead object we could
end up tracing
         // into garbage or the middle of another object via the newly
conservatively found
         // object.
-        if (sweepRequested())
+        if (gcState() == ThreadState::SweepScheduled)
             heap->markUnmarkedObjectsDead();
     }
     prepareRegionTree();
-    setSweepRequested();
+    setGCState(ThreadState::GCRunning);
     flushHeapDoesNotContainCacheIfNeeded();
 }
 
+void ThreadState::postGC()
+{
+    setGCState(ThreadState::SweepScheduled);
+}
+
 void ThreadState::setupHeapsForTermination()
 {
     checkThread();
@@ -1016,8 +1015,9 @@ void ThreadState::copyStackUntilSafePointScope()
 void ThreadState::performPendingSweep()
 {
     checkThread();
-    if (!sweepRequested())
+    if (gcState() != SweepScheduled)
         return;
+    setGCState(Sweeping);
 
 #if ENABLE(GC_PROFILE_HEAP)
     // We snapshot the heap prior to sweeping to get numbers for both resources
@@ -1073,9 +1073,8 @@ void ThreadState::performPendingSweep()
             m_heaps[i]->postSweepProcessing();
     }
 
-    clearGCRequested();
     m_didV8GCAfterLastGC = false;
-    clearSweepRequested();
+    setGCState(ThreadState::NoGCScheduled);
 
     // If we collected less than 50% of objects, record that the collection
rate
     // is low which we use to determine when to perform the next GC.
@@ -1156,7 +1155,7 @@ void ThreadState::invokePreFinalizers(Visitor& visitor)
 #if ENABLE(GC_PROFILE_MARKING)
 const GCInfo* ThreadState::findGCInfoFromAllThreads(Address address)
 {
-    bool needLockForIteration = !isInGC();
+    bool needLockForIteration = !Heap::isInGC();
     if (needLockForIteration)
         threadAttachMutex().lock();

[haraken, 2014-11-26 07:32:07]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-11-26 07:32:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/747363005/100001

[commit-bot: I haz the power, 2014-11-26 07:33:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-26 07:33:08]

Failed to apply patch for Source/platform/heap/ThreadState.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/platform/heap/ThreadState.cpp
  Hunk #1 FAILED at 326.
  Hunk #3 succeeded at 765 (offset -1 lines).
  Hunk #4 succeeded at 812 (offset -1 lines).
  Hunk #5 succeeded at 863 (offset -1 lines).
  Hunk #6 succeeded at 874 (offset -1 lines).
  Hunk #7 succeeded at 1016 (offset -1 lines).
  Hunk #8 succeeded at 1074 (offset -1 lines).
  1 out of 9 hunks FAILED -- saving rejects to file
Source/platform/heap/ThreadState.cpp.rej

Patch:       Source/platform/heap/ThreadState.cpp
Index: Source/platform/heap/ThreadState.cpp
diff --git a/Source/platform/heap/ThreadState.cpp
b/Source/platform/heap/ThreadState.cpp
index
5da13c31557c80ea764570e02c0f18286bc7b74f..2fa4512b58c92e74ee57f92161432d61635244eb
100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -326,16 +326,14 @@ ThreadState::ThreadState()
     , m_safePointScopeMarker(0)
     , m_atSafePoint(false)
     , m_interruptors()
-    , m_gcRequested(false)
     , m_didV8GCAfterLastGC(false)
     , m_forcePreciseGCForTesting(false)
-    , m_sweepRequested(0)
     , m_sweepInProgress(false)
     , m_noAllocationCount(0)
-    , m_inGC(false)
     , m_isTerminating(false)
     , m_shouldFlushHeapDoesNotContainCache(false)
     , m_lowCollectionRate(false)
+    , m_gcState(NoGCScheduled)
     , m_traceDOMWrappers(0)
 #if defined(ADDRESS_SANITIZER)
     , m_asanFakeStack(__asan_get_current_fake_stack())
@@ -358,6 +356,7 @@ ThreadState::ThreadState()
 ThreadState::~ThreadState()
 {
     checkThread();
+    ASSERT(gcState() == NoGCScheduled);
     delete m_weakCallbackStack;
     m_weakCallbackStack = 0;
     for (int i = 0; i < NumberOfHeaps; i++)
@@ -765,43 +764,38 @@ bool ThreadState::shouldForceConservativeGC()
     return newSize >= 32 * 1024 * 1024 && newSize > 4 *
Heap::markedObjectSize();
 }
 
-bool ThreadState::sweepRequested()
+void ThreadState::setGCState(GCState gcState)
 {
-    ASSERT(Heap::isInGC() || checkThread());
-    return m_sweepRequested;
-}
-
-void ThreadState::setSweepRequested()
-{
-    // Sweep requested is set from the thread that initiates garbage
-    // collection which could be different from the thread for this
-    // thread state. Therefore the setting of m_sweepRequested needs a
-    // barrier.
-    atomicTestAndSetToOne(&m_sweepRequested);
-}
-
-void ThreadState::clearSweepRequested()
-{
-    checkThread();
-    m_sweepRequested = 0;
-}
-
-bool ThreadState::gcRequested()
-{
-    checkThread();
-    return m_gcRequested;
-}
-
-void ThreadState::setGCRequested()
-{
-    checkThread();
-    m_gcRequested = true;
+    switch (gcState) {
+    case NoGCScheduled:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == Sweeping);
+        break;
+    case GCScheduled:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == NoGCScheduled || m_gcState == GCScheduled
|| m_gcState == StoppingOtherThreads);
+        break;
+    case StoppingOtherThreads:
+        checkThread();
+        break;
+    case GCRunning:
+        break;
+    case SweepScheduled:
+        RELEASE_ASSERT(m_gcState == GCRunning);
+        break;
+    case Sweeping:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == SweepScheduled);
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
+    m_gcState = gcState;
 }
 
-void ThreadState::clearGCRequested()
+ThreadState::GCState ThreadState::gcState() const
 {
-    checkThread();
-    m_gcRequested = false;
+    return m_gcState;
 }
 
 void ThreadState::didV8GC()
@@ -817,7 +811,7 @@ void ThreadState::performPendingGC(StackState stackState)
         if (forcePreciseGCForTesting()) {
             setForcePreciseGCForTesting(false);
             Heap::collectAllGarbage();
-        } else if (gcRequested()) {
+        } else if (gcState() == GCScheduled) {
             Heap::collectGarbage(NoHeapPointersOnStack);
         }
     }
@@ -868,7 +862,7 @@ void ThreadState::flushHeapDoesNotContainCacheIfNeeded()
     }
 }
 
-void ThreadState::prepareForGC()
+void ThreadState::preGC()
 {
     for (int i = 0; i < NumberOfHeaps; i++) {
         BaseHeap* heap = m_heaps[i];
@@ -879,14 +873,19 @@ void ThreadState::prepareForGC()
         // not trace already dead objects. If we trace a dead object we could
end up tracing
         // into garbage or the middle of another object via the newly
conservatively found
         // object.
-        if (sweepRequested())
+        if (gcState() == ThreadState::SweepScheduled)
             heap->markUnmarkedObjectsDead();
     }
     prepareRegionTree();
-    setSweepRequested();
+    setGCState(ThreadState::GCRunning);
     flushHeapDoesNotContainCacheIfNeeded();
 }
 
+void ThreadState::postGC()
+{
+    setGCState(ThreadState::SweepScheduled);
+}
+
 void ThreadState::setupHeapsForTermination()
 {
     checkThread();
@@ -1016,8 +1015,9 @@ void ThreadState::copyStackUntilSafePointScope()
 void ThreadState::performPendingSweep()
 {
     checkThread();
-    if (!sweepRequested())
+    if (gcState() != SweepScheduled)
         return;
+    setGCState(Sweeping);
 
 #if ENABLE(GC_PROFILE_HEAP)
     // We snapshot the heap prior to sweeping to get numbers for both resources
@@ -1073,9 +1073,8 @@ void ThreadState::performPendingSweep()
             m_heaps[i]->postSweepProcessing();
     }
 
-    clearGCRequested();
     m_didV8GCAfterLastGC = false;
-    clearSweepRequested();
+    setGCState(ThreadState::NoGCScheduled);
 
     // If we collected less than 50% of objects, record that the collection
rate
     // is low which we use to determine when to perform the next GC.
@@ -1156,7 +1155,7 @@ void ThreadState::invokePreFinalizers(Visitor& visitor)
 #if ENABLE(GC_PROFILE_MARKING)
 const GCInfo* ThreadState::findGCInfoFromAllThreads(Address address)
 {
-    bool needLockForIteration = !isInGC();
+    bool needLockForIteration = !Heap::isInGC();
     if (needLockForIteration)
         threadAttachMutex().lock();

[haraken, 2014-11-26 08:44:11]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-11-26 08:45:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/747363005/120001

[commit-bot: I haz the power, 2014-11-26 08:45:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-26 08:45:37]

Failed to apply patch for Source/platform/heap/ThreadState.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/platform/heap/ThreadState.cpp
  Hunk #1 FAILED at 326.
  Hunk #3 succeeded at 765 (offset -1 lines).
  Hunk #4 succeeded at 812 (offset -1 lines).
  Hunk #5 succeeded at 863 (offset -1 lines).
  Hunk #6 succeeded at 874 (offset -1 lines).
  Hunk #7 succeeded at 1016 (offset -1 lines).
  Hunk #8 succeeded at 1074 (offset -1 lines).
  1 out of 9 hunks FAILED -- saving rejects to file
Source/platform/heap/ThreadState.cpp.rej

Patch:       Source/platform/heap/ThreadState.cpp
Index: Source/platform/heap/ThreadState.cpp
diff --git a/Source/platform/heap/ThreadState.cpp
b/Source/platform/heap/ThreadState.cpp
index
5da13c31557c80ea764570e02c0f18286bc7b74f..2fa4512b58c92e74ee57f92161432d61635244eb
100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -326,16 +326,14 @@ ThreadState::ThreadState()
     , m_safePointScopeMarker(0)
     , m_atSafePoint(false)
     , m_interruptors()
-    , m_gcRequested(false)
     , m_didV8GCAfterLastGC(false)
     , m_forcePreciseGCForTesting(false)
-    , m_sweepRequested(0)
     , m_sweepInProgress(false)
     , m_noAllocationCount(0)
-    , m_inGC(false)
     , m_isTerminating(false)
     , m_shouldFlushHeapDoesNotContainCache(false)
     , m_lowCollectionRate(false)
+    , m_gcState(NoGCScheduled)
     , m_traceDOMWrappers(0)
 #if defined(ADDRESS_SANITIZER)
     , m_asanFakeStack(__asan_get_current_fake_stack())
@@ -358,6 +356,7 @@ ThreadState::ThreadState()
 ThreadState::~ThreadState()
 {
     checkThread();
+    ASSERT(gcState() == NoGCScheduled);
     delete m_weakCallbackStack;
     m_weakCallbackStack = 0;
     for (int i = 0; i < NumberOfHeaps; i++)
@@ -765,43 +764,38 @@ bool ThreadState::shouldForceConservativeGC()
     return newSize >= 32 * 1024 * 1024 && newSize > 4 *
Heap::markedObjectSize();
 }
 
-bool ThreadState::sweepRequested()
+void ThreadState::setGCState(GCState gcState)
 {
-    ASSERT(Heap::isInGC() || checkThread());
-    return m_sweepRequested;
-}
-
-void ThreadState::setSweepRequested()
-{
-    // Sweep requested is set from the thread that initiates garbage
-    // collection which could be different from the thread for this
-    // thread state. Therefore the setting of m_sweepRequested needs a
-    // barrier.
-    atomicTestAndSetToOne(&m_sweepRequested);
-}
-
-void ThreadState::clearSweepRequested()
-{
-    checkThread();
-    m_sweepRequested = 0;
-}
-
-bool ThreadState::gcRequested()
-{
-    checkThread();
-    return m_gcRequested;
-}
-
-void ThreadState::setGCRequested()
-{
-    checkThread();
-    m_gcRequested = true;
+    switch (gcState) {
+    case NoGCScheduled:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == Sweeping);
+        break;
+    case GCScheduled:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == NoGCScheduled || m_gcState == GCScheduled
|| m_gcState == StoppingOtherThreads);
+        break;
+    case StoppingOtherThreads:
+        checkThread();
+        break;
+    case GCRunning:
+        break;
+    case SweepScheduled:
+        RELEASE_ASSERT(m_gcState == GCRunning);
+        break;
+    case Sweeping:
+        checkThread();
+        RELEASE_ASSERT(m_gcState == SweepScheduled);
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
+    m_gcState = gcState;
 }
 
-void ThreadState::clearGCRequested()
+ThreadState::GCState ThreadState::gcState() const
 {
-    checkThread();
-    m_gcRequested = false;
+    return m_gcState;
 }
 
 void ThreadState::didV8GC()
@@ -817,7 +811,7 @@ void ThreadState::performPendingGC(StackState stackState)
         if (forcePreciseGCForTesting()) {
             setForcePreciseGCForTesting(false);
             Heap::collectAllGarbage();
-        } else if (gcRequested()) {
+        } else if (gcState() == GCScheduled) {
             Heap::collectGarbage(NoHeapPointersOnStack);
         }
     }
@@ -868,7 +862,7 @@ void ThreadState::flushHeapDoesNotContainCacheIfNeeded()
     }
 }
 
-void ThreadState::prepareForGC()
+void ThreadState::preGC()
 {
     for (int i = 0; i < NumberOfHeaps; i++) {
         BaseHeap* heap = m_heaps[i];
@@ -879,14 +873,19 @@ void ThreadState::prepareForGC()
         // not trace already dead objects. If we trace a dead object we could
end up tracing
         // into garbage or the middle of another object via the newly
conservatively found
         // object.
-        if (sweepRequested())
+        if (gcState() == ThreadState::SweepScheduled)
             heap->markUnmarkedObjectsDead();
     }
     prepareRegionTree();
-    setSweepRequested();
+    setGCState(ThreadState::GCRunning);
     flushHeapDoesNotContainCacheIfNeeded();
 }
 
+void ThreadState::postGC()
+{
+    setGCState(ThreadState::SweepScheduled);
+}
+
 void ThreadState::setupHeapsForTermination()
 {
     checkThread();
@@ -1016,8 +1015,9 @@ void ThreadState::copyStackUntilSafePointScope()
 void ThreadState::performPendingSweep()
 {
     checkThread();
-    if (!sweepRequested())
+    if (gcState() != SweepScheduled)
         return;
+    setGCState(Sweeping);
 
 #if ENABLE(GC_PROFILE_HEAP)
     // We snapshot the heap prior to sweeping to get numbers for both resources
@@ -1073,9 +1073,8 @@ void ThreadState::performPendingSweep()
             m_heaps[i]->postSweepProcessing();
     }
 
-    clearGCRequested();
     m_didV8GCAfterLastGC = false;
-    clearSweepRequested();
+    setGCState(ThreadState::NoGCScheduled);
 
     // If we collected less than 50% of objects, record that the collection
rate
     // is low which we use to determine when to perform the next GC.
@@ -1156,7 +1155,7 @@ void ThreadState::invokePreFinalizers(Visitor& visitor)
 #if ENABLE(GC_PROFILE_MARKING)
 const GCInfo* ThreadState::findGCInfoFromAllThreads(Address address)
 {
-    bool needLockForIteration = !isInGC();
+    bool needLockForIteration = !Heap::isInGC();
     if (needLockForIteration)
         threadAttachMutex().lock();

[haraken, 2014-11-26 10:47:43]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-11-26 10:47:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/747363005/140001

[commit-bot: I haz the power, 2014-11-26 12:04:04]

Committed patchset #8 (id:140001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=186029