Oilpan: Tweak a GC threshold to consider memory increase in PartitionAlloc

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 986eea1f3c245bb8fe3c121900f53b3c242127bf..eb3522ec31ea45e75ed2a856b76448a9d2c3e47b 100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -515,10 +515,9 @@ bool ThreadState::shouldScheduleIdleGC()
     // Heap::markedObjectSize() may be underestimated if any thread has not
     // finished completeSweep().
     size_t currentObjectSize = allocatedObjectSize + Heap::markedObjectSize() + WTF::Partitions::totalSizeOfCommittedPages();
-    // Schedule an idle GC if Oilpan has allocated more than 1 MB since
-    // the last GC and the current memory usage is >50% larger than
-    // the estimated live memory usage.
-    return allocatedObjectSize >= 1024 * 1024 && currentObjectSize > estimatedLiveObjectSize * 3 / 2;
+    return Heap::allocatedSpace() >= 1024 * 1024 // Oilpan has allocated >1 MB.
+        && currentObjectSize >= 1024 * 1024 // Oilpan or/and PartitionAlloc have allocated >1 MB since the last GC.

[sof, 2015/05/25 21:37:51]

If allocatedObjectSize >= 1024 * 1024, doesn't that imply that currentObjectSize
is larger than that threshold also?

[haraken, 2015/05/25 23:31:53]

Yes.

- allocatedObjectSize >= 1 MB implies currentObjectSize >= 1 MB.

- But in this CL, I'm using Heap::allocatedSpace() >= 1 MB, which is different
from allocatedObjectSize >= 1 MB. Note that Heap::allocatedSpace() is the total
size of memory allocated in Oilpan.

[sof, 2015/05/26 05:10:59]

Oops, quite right - I didn't pick the suffixes apart.

+        && currentObjectSize > estimatedLiveObjectSize * 3 / 2; // The current memory usage is >50% larger than the estimated live memory usage.
 #else
     return false;
 #endif
@@ -541,10 +540,9 @@ bool ThreadState::shouldSchedulePreciseGC()
     // Heap::markedObjectSize() may be underestimated if any thread has not
     // finished completeSweep().
     size_t currentObjectSize = allocatedObjectSize + Heap::markedObjectSize() + WTF::Partitions::totalSizeOfCommittedPages();
-    // Schedule a precise GC if Oilpan has allocated more than 1 MB since
-    // the last GC and the current memory usage is >50% larger than
-    // the estimated live memory usage.
-    return allocatedObjectSize >= 1024 * 1024 && currentObjectSize > estimatedLiveObjectSize * 3 / 2;
+    return Heap::allocatedSpace() >= 1024 * 1024 // Oilpan has allocated >1 MB.
+        && currentObjectSize >= 1024 * 1024 // Oilpan or/and PartitionAlloc have allocated >1 MB since the last GC.

[sof, 2015/05/26 19:25:37]

Is this really true? totalSizeOfCommittedPages() doesn't measure a since-last-GC
amount, nor does markedObjectSize().

[haraken, 2015/05/27 00:39:00]

You're right; this comment is wrong. It should be:

  // The current memory usage in Oilpan and PartitionAlloc is >1 MB.

[sof, 2015/05/27 05:09:07]

Not just the comment is wrong; this seems like curve fitting to satisfy a
benchmark.

[haraken, 2015/05/27 05:50:03]

There would be various opinions, but IMO this is a threshold that has a
theoretical background (as explained in the old document). If Oilpan uses X MB,
PartitionAlloc uses Y MB and Oilpan has strong pointers to PartitionAlloc, it
would make sense to trigger Oilpan's GC based on X+Y.

+        && currentObjectSize > estimatedLiveObjectSize * 3 / 2; // The current memory usage is >50% larger than the estimated live memory usage.
 #endif
 }
 
@@ -568,11 +566,9 @@ bool ThreadState::shouldForceConservativeGC()
         // aggressively. This is a safe guard to avoid OOM.
         return currentObjectSize > estimatedLiveObjectSize * 3 / 2;
     }
-    // Schedule a conservative GC if Oilpan has allocated more than 32 MB since
-    // the last GC and the current memory usage is >400% larger than
-    // the estimated live memory usage.
-    // TODO(haraken): 400% is too large. Lower the heap growing factor.
-    return allocatedObjectSize >= 32 * 1024 * 1024 && currentObjectSize > 5 * estimatedLiveObjectSize;
+    return Heap::allocatedSpace() >= 1024 * 1024 // Oilpan has allocated >1 MB.
+        && currentObjectSize >= 32 * 1024 * 1024 // Oilpan or/and PartitionAlloc have allocated >32 MB since the last GC.
+        && currentObjectSize > estimatedLiveObjectSize * 5; // The current memory usage is >400% larger than the estimated live memory usage. TODO(haraken): 400% is too large. Lower the heap growing factor.
 }
 
 void ThreadState::scheduleGCIfNeeded()