Allow Oilpan heap objects account for their external allocations.

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 0ae8284286368653772f4242475dd2bbe5d75e9d..8617c2d7a38aab8beeba5a521086a480c0c291da 100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -669,6 +669,9 @@ bool ThreadState::shouldSchedulePreciseGC()
 // These heuristics affect performance significantly.
 bool ThreadState::shouldForceConservativeGC()
 {
+    if (Heap::isUrgentGCRequested())
+        return true;
+
     size_t newSize = Heap::allocatedObjectSize();
     if (newSize >= 300 * 1024 * 1024) {
         // If we consume too much memory, trigger a conservative GC
@@ -694,13 +697,22 @@ void ThreadState::scheduleGCIfNeeded()
     checkThread();
     // Allocation is allowed during sweeping, but those allocations should not
     // trigger nested GCs
-    if (isSweepingInProgress())
-        return;
+    if (isSweepingInProgress()) {
+        if (!Heap::isUrgentGCRequested() || !isSweepingScheduled())

[haraken, 2015/02/23 08:42:27]

Do we want to have the 'isSweepingScheduled()' check?

[sof, 2015/02/23 12:26:13]

I think we do as otherwise we would allow nested GCs?

[haraken, 2015/02/23 14:59:07]

How can it lead to nested GCs? Given that we clear the urgent GC flag at the
beginning of a GC, Heap::isUrgentGCRequested() will return false if we reach
here during the GC.

[sof, 2015/02/23 15:03:40]

If you're in the Sweeping state and you don't perform this check & the urgent-gc
has been set by some external agent, you'll fall into doing a nested
conservative GC below. 

[haraken, 2015/02/23 15:06:22]

Thanks, makes sense.

It seems more straightforward (and robust) to add some logic to
Heap::collectGarbage to prevent nested GCs. (e.g., Add a check to prevent
GCScope from getting nested.)

[sof, 2015/02/23 15:22:40]

Having that "is-allowed" check in an already conditional scheduling operation
(scheduleIfNeeded()), seems reasonable to me. i.e., like before/right now.

+            return;
+        // Urgent GC requested with only a GC scheduled; fall through
+        // and have it be serviced by a conservative GC.
+    }
     ASSERT(!sweepForbidden());
 
-    if (shouldForceConservativeGC())
-        Heap::collectGarbage(ThreadState::HeapPointersOnStack, ThreadState::GCWithoutSweep);
-    else if (shouldSchedulePreciseGC())
+    if (shouldForceConservativeGC()) {
+        // If GC is deemed urgent, eagerly sweep and finalize any external allocations right away.
+        GCType gcType = Heap::isUrgentGCRequested() ? GCWithSweep : GCWithoutSweep;
+        Heap::clearUrgentGC();

[haraken, 2015/02/23 08:42:27]

Would it be better to move clearUrgentGC() into Heap::collectGarbage()?
Otherwise, the following scenario can happen:

1) An urgent GC is scheduled
2) Heap::collectGarbage() runs for some reason.
3) shouldGCIfNeeded() is called.
4) An urgent GC is invoked.

[sof, 2015/02/23 12:26:13]

That makes good sense to do for the cases where urgent GCs are externally
requested. And it is actually already done via resetHeapCounters(), so the
clearing here shouldn't be done (and will be redundant, typically.)

+        Heap::collectGarbage(HeapPointersOnStack, gcType);
+        return;
+    }
+    if (shouldSchedulePreciseGC())
         schedulePreciseGC();
     else if (shouldScheduleIdleGC())
         scheduleIdleGC();