[webkit-changes] [114511] trunk/Source/JavaScriptCore

Wed, 18 Apr 2012 09:18:42 -0700

Title: [114511] trunk/Source/_javascript_Core
Revision
114511
Author
[email protected]
Date
2012-04-18 09:18:32 -0700 (Wed, 18 Apr 2012)

Log Message

GC activity timer should be tied to allocation, not collection
https://bugs.webkit.org/show_bug.cgi?id=83919

Reviewed by Geoffrey Garen.

* API/JSContextRef.cpp: Used the new didAbandonObjectGraph callback to indicate that now that we've
released a global object, we're abandoning a potentially large number of objects that JSC might want
to collect.
* heap/CopiedSpace.cpp:
(JSC::CopiedSpace::tryAllocateSlowCase): Added the call to timer's willAllocate function to indicate
that we've hit a slow path and are allocating now, so schedule the timer.
* heap/Heap.cpp:
(JSC::Heap::Heap):
(JSC::Heap::collectAllGarbage): Removed the call to discardAllCompiledCode because it was causing us to
throw away too much code during our benchmarks (especially vp8, which is very large and thus has large
amounts of compiled code).
(JSC::Heap::collect): Added the new call to didCollect at the conclusion of a collection so that we
can cancel the timer if we no longer need to run a collection. Also added a check at the beginning of a
collection to see if we should throw away our compiled code. Currently this is set to happen about once
every minute.
* heap/Heap.h: Added field to keep track of the last time we threw away our compiled code.
* heap/MarkedAllocator.cpp:
(JSC::MarkedAllocator::allocateSlowCase): Added call to willAllocate on the allocation slow path, just like
in CopiedSpace.
* runtime/GCActivityCallback.cpp: Added default stubs for non-CF platforms.
(JSC::DefaultGCActivityCallback::willAllocate):
(JSC):
(JSC::DefaultGCActivityCallback::didCollect):
(JSC::DefaultGCActivityCallback::didAbandonObjectGraph):
* runtime/GCActivityCallback.h: Added new functions to make JSC's GC timer less arcane. This includes replacing
the operator () with willAllocate() and adding an explicit didCollect() to cancel the timer after a collection
occurs rather than relying on the way the timer is invoked to cancel itself. Also added a callback for
when somebody else (e.g. WebCore or the JSC API) to notify JSC that they have just abandoned an entire graph of
objects and that JSC might want to clean them up.
(JSC::GCActivityCallback::~GCActivityCallback):
(JSC::GCActivityCallback::willAllocate):
(JSC::GCActivityCallback::didCollect):
(JSC::GCActivityCallback::didAbandonObjectGraph):
(JSC::GCActivityCallback::synchronize):
(DefaultGCActivityCallback):
* runtime/GCActivityCallbackCF.cpp: Re-wired all the run loop stuff to implement the aforementioned functions.
We added a flag to check whether the timer was active because the call to CFRunLoopTimerSetNextFireDate actually
turned out to be quite expensive (although Instruments couldn't tell us this).
(DefaultGCActivityCallbackPlatformData):
(JSC):
(JSC::DefaultGCActivityCallbackPlatformData::timerDidFire):
(JSC::DefaultGCActivityCallback::commonConstructor):
(JSC::scheduleTimer):
(JSC::cancelTimer):
(JSC::DefaultGCActivityCallback::willAllocate):
(JSC::DefaultGCActivityCallback::didCollect):
(JSC::DefaultGCActivityCallback::didAbandonObjectGraph):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/API/JSContextRef.cpp (114510 => 114511)


--- trunk/Source/_javascript_Core/API/JSContextRef.cpp	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/API/JSContextRef.cpp	2012-04-18 16:18:32 UTC (rev 114511)
@@ -149,7 +149,7 @@
         globalData.heap.destroy();
     } else if (releasingGlobalObject) {
         globalData.heap.activityCallback()->synchronize();
-        (*globalData.heap.activityCallback())();
+        globalData.heap.activityCallback()->didAbandonObjectGraph();
     }
 
     globalData.deref();

Modified: trunk/Source/_javascript_Core/ChangeLog (114510 => 114511)


--- trunk/Source/_javascript_Core/ChangeLog	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/ChangeLog	2012-04-18 16:18:32 UTC (rev 114511)
@@ -1,3 +1,58 @@
+2012-04-18  Mark Hahnenberg  <[email protected]>
+
+        GC activity timer should be tied to allocation, not collection
+        https://bugs.webkit.org/show_bug.cgi?id=83919
+
+        Reviewed by Geoffrey Garen.
+
+        * API/JSContextRef.cpp: Used the new didAbandonObjectGraph callback to indicate that now that we've 
+        released a global object, we're abandoning a potentially large number of objects that JSC might want 
+        to collect.
+        * heap/CopiedSpace.cpp:
+        (JSC::CopiedSpace::tryAllocateSlowCase): Added the call to timer's willAllocate function to indicate 
+        that we've hit a slow path and are allocating now, so schedule the timer.
+        * heap/Heap.cpp:
+        (JSC::Heap::Heap):
+        (JSC::Heap::collectAllGarbage): Removed the call to discardAllCompiledCode because it was causing us to 
+        throw away too much code during our benchmarks (especially vp8, which is very large and thus has large 
+        amounts of compiled code).
+        (JSC::Heap::collect): Added the new call to didCollect at the conclusion of a collection so that we 
+        can cancel the timer if we no longer need to run a collection. Also added a check at the beginning of a 
+        collection to see if we should throw away our compiled code. Currently this is set to happen about once 
+        every minute.
+        * heap/Heap.h: Added field to keep track of the last time we threw away our compiled code.
+        * heap/MarkedAllocator.cpp:
+        (JSC::MarkedAllocator::allocateSlowCase): Added call to willAllocate on the allocation slow path, just like 
+        in CopiedSpace.
+        * runtime/GCActivityCallback.cpp: Added default stubs for non-CF platforms.
+        (JSC::DefaultGCActivityCallback::willAllocate):
+        (JSC):
+        (JSC::DefaultGCActivityCallback::didCollect):
+        (JSC::DefaultGCActivityCallback::didAbandonObjectGraph):
+        * runtime/GCActivityCallback.h: Added new functions to make JSC's GC timer less arcane. This includes replacing 
+        the operator () with willAllocate() and adding an explicit didCollect() to cancel the timer after a collection 
+        occurs rather than relying on the way the timer is invoked to cancel itself. Also added a callback for 
+        when somebody else (e.g. WebCore or the JSC API) to notify JSC that they have just abandoned an entire graph of 
+        objects and that JSC might want to clean them up.
+        (JSC::GCActivityCallback::~GCActivityCallback):
+        (JSC::GCActivityCallback::willAllocate):
+        (JSC::GCActivityCallback::didCollect):
+        (JSC::GCActivityCallback::didAbandonObjectGraph):
+        (JSC::GCActivityCallback::synchronize):
+        (DefaultGCActivityCallback):
+        * runtime/GCActivityCallbackCF.cpp: Re-wired all the run loop stuff to implement the aforementioned functions. 
+        We added a flag to check whether the timer was active because the call to CFRunLoopTimerSetNextFireDate actually 
+        turned out to be quite expensive (although Instruments couldn't tell us this).
+        (DefaultGCActivityCallbackPlatformData):
+        (JSC):
+        (JSC::DefaultGCActivityCallbackPlatformData::timerDidFire):
+        (JSC::DefaultGCActivityCallback::commonConstructor):
+        (JSC::scheduleTimer):
+        (JSC::cancelTimer):
+        (JSC::DefaultGCActivityCallback::willAllocate):
+        (JSC::DefaultGCActivityCallback::didCollect):
+        (JSC::DefaultGCActivityCallback::didAbandonObjectGraph):
+
 2012-04-17  Filip Pizlo  <[email protected]>
 
         DFG should not attempt to get rare case counts for op_mod on ARM

Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.cpp (114510 => 114511)


--- trunk/Source/_javascript_Core/heap/CopiedSpace.cpp	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.cpp	2012-04-18 16:18:32 UTC (rev 114511)
@@ -27,6 +27,7 @@
 #include "CopiedSpace.h"
 
 #include "CopiedSpaceInlineMethods.h"
+#include "GCActivityCallback.h"
 
 namespace JSC {
 
@@ -51,6 +52,8 @@
 
 CheckedBoolean CopiedSpace::tryAllocateSlowCase(size_t bytes, void** outPtr)
 {
+    m_heap->activityCallback()->willAllocate();
+    
     if (isOversize(bytes))
         return tryAllocateOversize(bytes, outPtr);

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (114510 => 114511)


--- trunk/Source/_javascript_Core/heap/Heap.cpp	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2012-04-18 16:18:32 UTC (rev 114511)
@@ -328,8 +328,8 @@
     , m_isSafeToCollect(false)
     , m_globalData(globalData)
     , m_lastGCLength(0)
+    , m_lastCodeDiscardTime(WTF::currentTime())
 {
-    (*m_activityCallback)();
     m_numberOfFreeBlocks = 0;
     m_blockFreeingThread = createThread(blockFreeingThreadStartFunc, this, "_javascript_Core::BlockFree");
     
@@ -783,11 +783,12 @@
 {
     if (!m_isSafeToCollect)
         return;
-    discardAllCompiledCode();
 
     collect(DoSweep);
 }
 
+static double minute = 60.0;
+
 void Heap::collect(SweepToggle sweepToggle)
 {
     SamplingRegion samplingRegion("Garbage Collection");
@@ -796,7 +797,13 @@
     ASSERT(globalData()->identifierTable == wtfThreadData().currentIdentifierTable());
     ASSERT(m_isSafeToCollect);
     _javascript_CORE_GC_BEGIN();
+
     double lastGCStartTime = WTF::currentTime();
+    if (lastGCStartTime - m_lastCodeDiscardTime > minute) {
+        discardAllCompiledCode();
+        m_lastCodeDiscardTime = WTF::currentTime();
+    }
+
 #if ENABLE(GGC)
     bool fullGC = sweepToggle == DoSweep;
     if (!fullGC)
@@ -855,7 +862,7 @@
     m_lastGCLength = lastGCEndTime - lastGCStartTime;
     _javascript_CORE_GC_END();
 
-    (*m_activityCallback)();
+    m_activityCallback->didCollect();
 }
 
 void Heap::canonicalizeCellLivenessData()

Modified: trunk/Source/_javascript_Core/heap/Heap.h (114510 => 114511)


--- trunk/Source/_javascript_Core/heap/Heap.h	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2012-04-18 16:18:32 UTC (rev 114511)
@@ -247,6 +247,7 @@
 
         JSGlobalData* m_globalData;
         double m_lastGCLength;
+        double m_lastCodeDiscardTime;
 
         DoublyLinkedList<FunctionExecutable> m_functions;
     };

Modified: trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp (114510 => 114511)


--- trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp	2012-04-18 16:18:32 UTC (rev 114511)
@@ -1,6 +1,7 @@
 #include "config.h"
 #include "MarkedAllocator.h"
 
+#include "GCActivityCallback.h"
 #include "Heap.h"
 
 namespace JSC {
@@ -41,6 +42,8 @@
     ASSERT(m_heap->m_operationInProgress == NoOperation);
 #endif
     
+    m_heap->activityCallback()->willAllocate();
+    
     void* result = tryAllocate();
     
     if (LIKELY(result != 0))

Modified: trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp (114510 => 114511)


--- trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp	2012-04-18 16:18:32 UTC (rev 114511)
@@ -42,10 +42,18 @@
 {
 }
 
-void DefaultGCActivityCallback::operator()()
+void DefaultGCActivityCallback::willAllocate()
 {
 }
 
+void DefaultGCActivityCallback::didCollect()
+{
+}
+
+void DefaultGCActivityCallback::didAbandonObjectGraph()
+{
+}
+
 void DefaultGCActivityCallback::synchronize()
 {
 }

Modified: trunk/Source/_javascript_Core/runtime/GCActivityCallback.h (114510 => 114511)


--- trunk/Source/_javascript_Core/runtime/GCActivityCallback.h	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/runtime/GCActivityCallback.h	2012-04-18 16:18:32 UTC (rev 114511)
@@ -42,9 +42,11 @@
 
 class GCActivityCallback {
 public:
-    virtual ~GCActivityCallback() {}
-    virtual void operator()() {}
-    virtual void synchronize() {}
+    virtual ~GCActivityCallback() { }
+    virtual void willAllocate() { }
+    virtual void didCollect() { }
+    virtual void didAbandonObjectGraph() { }
+    virtual void synchronize() { }
 
 protected:
     GCActivityCallback() {}
@@ -57,10 +59,12 @@
     static PassOwnPtr<DefaultGCActivityCallback> create(Heap*);
 
     DefaultGCActivityCallback(Heap*);
-    ~DefaultGCActivityCallback();
+    virtual ~DefaultGCActivityCallback();
 
-    void operator()();
-    void synchronize();
+    virtual void willAllocate();
+    virtual void didCollect();
+    virtual void didAbandonObjectGraph();
+    virtual void synchronize();
 
 #if USE(CF)
 protected:

Modified: trunk/Source/_javascript_Core/runtime/GCActivityCallbackCF.cpp (114510 => 114511)


--- trunk/Source/_javascript_Core/runtime/GCActivityCallbackCF.cpp	2012-04-18 16:18:24 UTC (rev 114510)
+++ trunk/Source/_javascript_Core/runtime/GCActivityCallbackCF.cpp	2012-04-18 16:18:32 UTC (rev 114511)
@@ -45,21 +45,24 @@
 namespace JSC {
 
 struct DefaultGCActivityCallbackPlatformData {
-    static void trigger(CFRunLoopTimerRef, void *info);
+    static void timerDidFire(CFRunLoopTimerRef, void *info);
 
     RetainPtr<CFRunLoopTimerRef> timer;
     RetainPtr<CFRunLoopRef> runLoop;
     CFRunLoopTimerContext context;
+    bool timerIsActive;
 };
 
+const double gcCPUBudget = 0.025;
+const double gcTimerIntervalMultiplier = 1.0 / gcCPUBudget;
 const CFTimeInterval decade = 60 * 60 * 24 * 365 * 10;
+const CFTimeInterval hour = 60 * 60;
 
-void DefaultGCActivityCallbackPlatformData::trigger(CFRunLoopTimerRef timer, void *info)
+void DefaultGCActivityCallbackPlatformData::timerDidFire(CFRunLoopTimerRef, void *info)
 {
     Heap* heap = static_cast<Heap*>(info);
     APIEntryShim shim(heap->globalData());
     heap->collectAllGarbage();
-    CFRunLoopTimerSetNextFireDate(timer, CFAbsoluteTimeGetCurrent() + decade);
 }
 
 DefaultGCActivityCallback::DefaultGCActivityCallback(Heap* heap)
@@ -88,16 +91,43 @@
     memset(&d->context, 0, sizeof(CFRunLoopTimerContext));
     d->context.info = heap;
     d->runLoop = runLoop;
-    d->timer.adoptCF(CFRunLoopTimerCreate(0, decade, decade, 0, 0, DefaultGCActivityCallbackPlatformData::trigger, &d->context));
+    d->timer.adoptCF(CFRunLoopTimerCreate(0, decade, decade, 0, 0, DefaultGCActivityCallbackPlatformData::timerDidFire, &d->context));
+    d->timerIsActive = false;
     CFRunLoopAddTimer(d->runLoop.get(), d->timer.get(), kCFRunLoopCommonModes);
 }
 
-void DefaultGCActivityCallback::operator()()
+static void scheduleTimer(DefaultGCActivityCallbackPlatformData* d)
 {
-    CFTimeInterval triggerInterval = static_cast<Heap*>(d->context.info)->lastGCLength() * 100.0; 
+    if (d->timerIsActive)
+        return;
+    d->timerIsActive = true;
+    CFTimeInterval triggerInterval = static_cast<Heap*>(d->context.info)->lastGCLength() * gcTimerIntervalMultiplier; 
     CFRunLoopTimerSetNextFireDate(d->timer.get(), CFAbsoluteTimeGetCurrent() + triggerInterval);
 }
 
+static void cancelTimer(DefaultGCActivityCallbackPlatformData* d)
+{
+    if (!d->timerIsActive)
+        return;
+    d->timerIsActive = false;
+    CFRunLoopTimerSetNextFireDate(d->timer.get(), CFAbsoluteTimeGetCurrent() + decade);
+}
+
+void DefaultGCActivityCallback::willAllocate()
+{
+    scheduleTimer(d.get());
+}
+
+void DefaultGCActivityCallback::didCollect()
+{
+    cancelTimer(d.get());
+}
+
+void DefaultGCActivityCallback::didAbandonObjectGraph()
+{
+    scheduleTimer(d.get());
+}
+
 void DefaultGCActivityCallback::synchronize()
 {
     if (CFRunLoopGetCurrent() == d->runLoop.get())

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to