- Revision
- 185387
- Author
- [email protected]
- Date
- 2015-06-09 15:15:36 -0700 (Tue, 09 Jun 2015)
Log Message
Allow one sync GC per gcTimer interval on critical memory pressure warning
https://bugs.webkit.org/show_bug.cgi?id=145773
Reviewed by Geoffrey Garen.
Source/_javascript_Core:
On critical memory pressure warning, we were calling GCController::garbageCollectSoon(),
which does not offer any guarantee on when the garbage collection will actually take
place.
On critical memory pressure, we need to free up memory as soon as possible to avoid
getting killed so this is an issue. Also, the fact that we clear the PageCache on
critical memory pressure means a GC would likely be useful, even if the last
collection did not free much memory.
This patch adds a new GCController::garbageCollectNowIfNotDoneRecently() API that allows
one synchronous GC per gcTimer interval on critical memory pressure warning. This makes
us more responsive to critical memory pressure and avoids doing synchronous GCs too
often.
* heap/FullGCActivityCallback.cpp:
(JSC::FullGCActivityCallback::doCollection):
* heap/FullGCActivityCallback.h:
(JSC::GCActivityCallback::createFullTimer):
* heap/GCActivityCallback.h:
* heap/Heap.cpp:
(JSC::Heap::collectAllGarbageIfNotDoneRecently):
* heap/Heap.h:
* heap/IncrementalSweeper.cpp:
(JSC::IncrementalSweeper::doWork): Deleted.
* heap/IncrementalSweeper.h:
Drop fullSweep() API as it no longer seems useful. garbageCollectNow()
already does a sweep after the full collection.
Source/WebCore:
* bindings/js/GCController.cpp:
(WebCore::GCController::garbageCollectNowIfNotDoneRecently):
Add new GCController::garbageCollectNowIfNotDoneRecently() API that
allows one synchronous GC per full GC timer interval. If called more
than once per interval, it becomes equivalent to garbageCollectSoon()
and merely accelerates the next collection.
* bindings/js/GCController.h:
* platform/MemoryPressureHandler.cpp:
(WebCore::MemoryPressureHandler::releaseCriticalMemory):
Call the new GCController::garbageCollectNowIfNotDoneRecently() on
critical memory pressure instead of garbageCollectionSoon() to try
as do a synchronous GC if one wasn't already done recently.
Also drop call to fullSweep() as GCController::garbageCollectNow*()
already do a sweep after the collection.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (185386 => 185387)
--- trunk/Source/_javascript_Core/ChangeLog 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-06-09 22:15:36 UTC (rev 185387)
@@ -1,3 +1,40 @@
+2015-06-09 Chris Dumez <[email protected]>
+
+ Allow one sync GC per gcTimer interval on critical memory pressure warning
+ https://bugs.webkit.org/show_bug.cgi?id=145773
+
+ Reviewed by Geoffrey Garen.
+
+ On critical memory pressure warning, we were calling GCController::garbageCollectSoon(),
+ which does not offer any guarantee on when the garbage collection will actually take
+ place.
+
+ On critical memory pressure, we need to free up memory as soon as possible to avoid
+ getting killed so this is an issue. Also, the fact that we clear the PageCache on
+ critical memory pressure means a GC would likely be useful, even if the last
+ collection did not free much memory.
+
+ This patch adds a new GCController::garbageCollectNowIfNotDoneRecently() API that allows
+ one synchronous GC per gcTimer interval on critical memory pressure warning. This makes
+ us more responsive to critical memory pressure and avoids doing synchronous GCs too
+ often.
+
+ * heap/FullGCActivityCallback.cpp:
+ (JSC::FullGCActivityCallback::doCollection):
+ * heap/FullGCActivityCallback.h:
+ (JSC::GCActivityCallback::createFullTimer):
+ * heap/GCActivityCallback.h:
+ * heap/Heap.cpp:
+ (JSC::Heap::collectAllGarbageIfNotDoneRecently):
+ * heap/Heap.h:
+
+ * heap/IncrementalSweeper.cpp:
+ (JSC::IncrementalSweeper::doWork): Deleted.
+ * heap/IncrementalSweeper.h:
+
+ Drop fullSweep() API as it no longer seems useful. garbageCollectNow()
+ already does a sweep after the full collection.
+
2015-06-09 Andreas Kling <[email protected]>
[JSC] CodeBlock::m_constantRegisters should be sized-to-fit.
Modified: trunk/Source/_javascript_Core/heap/FullGCActivityCallback.cpp (185386 => 185387)
--- trunk/Source/_javascript_Core/heap/FullGCActivityCallback.cpp 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/_javascript_Core/heap/FullGCActivityCallback.cpp 2015-06-09 22:15:36 UTC (rev 185387)
@@ -43,18 +43,19 @@
void FullGCActivityCallback::doCollection()
{
- Heap* heap = &m_vm->heap;
+ Heap& heap = m_vm->heap;
+ m_didSyncGCRecently = false;
#if !PLATFORM(IOS)
double startTime = WTF::monotonicallyIncreasingTime();
- if (heap->isPagedOut(startTime + pagingTimeOut)) {
+ if (heap.isPagedOut(startTime + pagingTimeOut)) {
cancel();
- heap->increaseLastFullGCLength(pagingTimeOut);
+ heap.increaseLastFullGCLength(pagingTimeOut);
return;
}
#endif
- heap->collect(FullCollection);
+ heap.collect(FullCollection);
}
double FullGCActivityCallback::lastGCLength()
Modified: trunk/Source/_javascript_Core/heap/FullGCActivityCallback.h (185386 => 185387)
--- trunk/Source/_javascript_Core/heap/FullGCActivityCallback.h 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/_javascript_Core/heap/FullGCActivityCallback.h 2015-06-09 22:15:36 UTC (rev 185387)
@@ -36,6 +36,9 @@
virtual void doCollection() override;
+ bool didSyncGCRecently() const { return m_didSyncGCRecently; }
+ void setDidSyncGCRecently() { m_didSyncGCRecently = true; }
+
protected:
#if USE(CF)
FullGCActivityCallback(Heap* heap, CFRunLoopRef runLoop)
@@ -47,9 +50,11 @@
virtual double lastGCLength() override;
virtual double gcTimeSlice(size_t bytes) override;
virtual double deathRate() override;
+
+ bool m_didSyncGCRecently { false };
};
-inline RefPtr<GCActivityCallback> GCActivityCallback::createFullTimer(Heap* heap)
+inline RefPtr<FullGCActivityCallback> GCActivityCallback::createFullTimer(Heap* heap)
{
return s_shouldCreateGCTimer ? adoptRef(new FullGCActivityCallback(heap)) : nullptr;
}
Modified: trunk/Source/_javascript_Core/heap/GCActivityCallback.h (185386 => 185387)
--- trunk/Source/_javascript_Core/heap/GCActivityCallback.h 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/_javascript_Core/heap/GCActivityCallback.h 2015-06-09 22:15:36 UTC (rev 185387)
@@ -38,12 +38,13 @@
namespace JSC {
+class FullGCActivityCallback;
class Heap;
class JS_EXPORT_PRIVATE GCActivityCallback : public HeapTimer, public ThreadSafeRefCounted<GCActivityCallback> {
WTF_MAKE_FAST_ALLOCATED;
public:
- static RefPtr<GCActivityCallback> createFullTimer(Heap*);
+ static RefPtr<FullGCActivityCallback> createFullTimer(Heap*);
static RefPtr<GCActivityCallback> createEdenTimer(Heap*);
GCActivityCallback(Heap*);
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (185386 => 185387)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2015-06-09 22:15:36 UTC (rev 185387)
@@ -1408,6 +1408,23 @@
m_compiledCode.append(executable);
}
+void Heap::collectAllGarbageIfNotDoneRecently()
+{
+ if (!m_fullActivityCallback) {
+ collectAllGarbage();
+ return;
+ }
+
+ if (m_fullActivityCallback->didSyncGCRecently()) {
+ // A synchronous GC was already requested recently so we merely accelerate next collection.
+ reportAbandonedObjectGraph();
+ return;
+ }
+
+ m_fullActivityCallback->setDidSyncGCRecently();
+ collectAllGarbage();
+}
+
class Zombify : public MarkedBlock::VoidFunctor {
public:
inline void visit(JSCell* cell)
Modified: trunk/Source/_javascript_Core/heap/Heap.h (185386 => 185387)
--- trunk/Source/_javascript_Core/heap/Heap.h 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2015-06-09 22:15:36 UTC (rev 185387)
@@ -154,6 +154,7 @@
void notifyIsSafeToCollect() { m_isSafeToCollect = true; }
bool isSafeToCollect() const { return m_isSafeToCollect; }
+ JS_EXPORT_PRIVATE void collectAllGarbageIfNotDoneRecently();
void collectAllGarbage() { collectAndSweep(FullCollection); }
JS_EXPORT_PRIVATE void collectAndSweep(HeapOperation collectionType = AnyCollection);
bool shouldCollect();
@@ -394,7 +395,7 @@
Vector<WeakBlock*> m_logicallyEmptyWeakBlocks;
size_t m_indexOfNextLogicallyEmptyWeakBlockToSweep { WTF::notFound };
- RefPtr<GCActivityCallback> m_fullActivityCallback;
+ RefPtr<FullGCActivityCallback> m_fullActivityCallback;
RefPtr<GCActivityCallback> m_edenActivityCallback;
std::unique_ptr<IncrementalSweeper> m_sweeper;
Vector<MarkedBlock*> m_blockSnapshot;
Modified: trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp (185386 => 185387)
--- trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp 2015-06-09 22:15:36 UTC (rev 185387)
@@ -59,11 +59,6 @@
CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + s_decade);
}
-void IncrementalSweeper::fullSweep()
-{
- while (sweepNextBlock()) { }
-}
-
void IncrementalSweeper::doWork()
{
doSweep(WTF::monotonicallyIncreasingTime());
@@ -132,10 +127,6 @@
{
}
-void IncrementalSweeper::fullSweep()
-{
-}
-
bool IncrementalSweeper::sweepNextBlock()
{
return false;
Modified: trunk/Source/_javascript_Core/heap/IncrementalSweeper.h (185386 => 185387)
--- trunk/Source/_javascript_Core/heap/IncrementalSweeper.h 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/_javascript_Core/heap/IncrementalSweeper.h 2015-06-09 22:15:36 UTC (rev 185387)
@@ -43,7 +43,6 @@
explicit IncrementalSweeper(VM*);
#endif
- JS_EXPORT_PRIVATE void fullSweep();
void startSweeping();
JS_EXPORT_PRIVATE virtual void doWork() override;
Modified: trunk/Source/WebCore/ChangeLog (185386 => 185387)
--- trunk/Source/WebCore/ChangeLog 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/WebCore/ChangeLog 2015-06-09 22:15:36 UTC (rev 185387)
@@ -1,3 +1,28 @@
+2015-06-09 Chris Dumez <[email protected]>
+
+ Allow one sync GC per gcTimer interval on critical memory pressure warning
+ https://bugs.webkit.org/show_bug.cgi?id=145773
+
+ Reviewed by Geoffrey Garen.
+
+ * bindings/js/GCController.cpp:
+ (WebCore::GCController::garbageCollectNowIfNotDoneRecently):
+
+ Add new GCController::garbageCollectNowIfNotDoneRecently() API that
+ allows one synchronous GC per full GC timer interval. If called more
+ than once per interval, it becomes equivalent to garbageCollectSoon()
+ and merely accelerates the next collection.
+
+ * bindings/js/GCController.h:
+ * platform/MemoryPressureHandler.cpp:
+ (WebCore::MemoryPressureHandler::releaseCriticalMemory):
+
+ Call the new GCController::garbageCollectNowIfNotDoneRecently() on
+ critical memory pressure instead of garbageCollectionSoon() to try
+ as do a synchronous GC if one wasn't already done recently.
+ Also drop call to fullSweep() as GCController::garbageCollectNow*()
+ already do a sweep after the collection.
+
2015-06-09 Darin Adler <[email protected]>
Follow-up fix for:
Modified: trunk/Source/WebCore/bindings/js/GCController.cpp (185386 => 185387)
--- trunk/Source/WebCore/bindings/js/GCController.cpp 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/WebCore/bindings/js/GCController.cpp 2015-06-09 22:15:36 UTC (rev 185387)
@@ -90,6 +90,17 @@
}
}
+void GCController::garbageCollectNowIfNotDoneRecently()
+{
+#if USE(CF)
+ JSLockHolder lock(JSDOMWindow::commonVM());
+ if (!JSDOMWindow::commonVM().heap.isBusy())
+ JSDOMWindow::commonVM().heap.collectAllGarbageIfNotDoneRecently();
+#else
+ garbageCollectSoon();
+#endif
+}
+
void GCController::garbageCollectOnAlternateThreadForDebugging(bool waitUntilDone)
{
ThreadIdentifier threadID = createThread(collect, 0, "WebCore: GCController");
Modified: trunk/Source/WebCore/bindings/js/GCController.h (185386 => 185387)
--- trunk/Source/WebCore/bindings/js/GCController.h 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/WebCore/bindings/js/GCController.h 2015-06-09 22:15:36 UTC (rev 185387)
@@ -40,6 +40,7 @@
WEBCORE_EXPORT void garbageCollectSoon();
WEBCORE_EXPORT void garbageCollectNow(); // It's better to call garbageCollectSoon, unless you have a specific reason not to.
+ WEBCORE_EXPORT void garbageCollectNowIfNotDoneRecently();
void garbageCollectOnNextRunLoop();
WEBCORE_EXPORT void garbageCollectOnAlternateThreadForDebugging(bool waitUntilDone); // Used for stress testing.
Modified: trunk/Source/WebCore/platform/MemoryPressureHandler.cpp (185386 => 185387)
--- trunk/Source/WebCore/platform/MemoryPressureHandler.cpp 2015-06-09 22:12:41 UTC (rev 185386)
+++ trunk/Source/WebCore/platform/MemoryPressureHandler.cpp 2015-06-09 22:15:36 UTC (rev 185387)
@@ -139,19 +139,8 @@
if (synchronous == Synchronous::Yes) {
ReliefLogger log("Collecting _javascript_ garbage");
GCController::singleton().garbageCollectNow();
- } else {
- // FIXME: We should do a garbage sweep and prune dead resources from the MemoryCache
- // after the garbage collection has completed to free up more memory.
- GCController::singleton().garbageCollectSoon();
-
- // Do a full sweep of collected objects. garbageCollectNow() already does this so we only
- // need to do this if it isn't called.
- {
- ReliefLogger log("Full _javascript_ garbage sweep");
- JSC::JSLockHolder lock(JSDOMWindow::commonVM());
- JSDOMWindow::commonVM().heap.sweeper()->fullSweep();
- }
- }
+ } else
+ GCController::singleton().garbageCollectNowIfNotDoneRecently();
}
void MemoryPressureHandler::releaseMemory(Critical critical, Synchronous synchronous)
