Changeset 207855 in webkit for trunk/Source/JavaScriptCore/heap/HeapTimer.cpp

Timestamp:

Oct 25, 2016, 4:22:48 PM (9 years ago)

Author:

[email protected]

Message:

HeapTimer should not hardcode all of its subclasses and USE(CF) shouldn't be a bizarre special case
​https://bugs.webkit.org/show_bug.cgi?id=163947

Reviewed by Geoffrey Garen.
Source/JavaScriptCore:

I want to introduce another HeapTimer. Prior to this change, that would have meant writing
exact copies of that timer's logic for each platform that has a HeapTimer (CF, GLIB, and
EFL). That logic would in turn be a duplicate of the logic already present in
IncrementalSweeper and GCActivityCallback: all of the subclasses of HeapTimer have their
own code for scheduling timers, so a new subclass would have to duplicate that code. Then,
to add insult to injury, the USE(CF) version of HeapTimer would have to have an extra case
for that new subclass since it doesn't use virtual methods effectively.

This changes HeapTimer on USE(CF) to know to get its runloop from Heap and to use virtual
methods effectively so that it doesn't have to know about all of its subclasses.

This also moves IncrementalSweeper's code for scheduling timers into HeapTimer. This means
that future subclasses of HeapTimer could simply use that logic.

This keeps changes to GCActivityCallback to a minimum. It still has a lot of
platform-specific code and I'm not sure that this code can be trivially deduplicated since
that code has more quirks. That's fine for now, since I mainly just need a sane way of
creating new timers that use IncrementalSweeper-like scheduling logic.

• heap/EdenGCActivityCallback.cpp:
• heap/EdenGCActivityCallback.h:
• heap/FullGCActivityCallback.cpp:
• heap/FullGCActivityCallback.h:
• heap/GCActivityCallback.cpp:

(JSC::GCActivityCallback::GCActivityCallback):

• heap/GCActivityCallback.h:
• heap/Heap.cpp:

(JSC::Heap::Heap):

• heap/Heap.h:

(JSC::Heap::runLoop):

• heap/HeapTimer.cpp:

(JSC::HeapTimer::HeapTimer):
(JSC::HeapTimer::setRunLoop):
(JSC::HeapTimer::~HeapTimer):
(JSC::HeapTimer::timerDidFire):
(JSC::HeapTimer::scheduleTimer):
(JSC::HeapTimer::cancelTimer):
(JSC::retainAPILock): Deleted.
(JSC::releaseAPILock): Deleted.

• heap/HeapTimer.h:
• heap/IncrementalSweeper.cpp:

(JSC::IncrementalSweeper::scheduleTimer):
(JSC::IncrementalSweeper::cancelTimer): Deleted.

• heap/IncrementalSweeper.h:

Source/WebCore:

No new tests because no new behavior.

• platform/ios/WebSafeGCActivityCallbackIOS.h:
• platform/ios/WebSafeIncrementalSweeperIOS.h:

File:

• trunk/Source/JavaScriptCore/heap/HeapTimer.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/heap/HeapTimer.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -47 +47 @@
 const CFTimeInterval HeapTimer::s_decade = 60 * 60 * 24 * 365 * 10;
 
-static const void* retainAPILock(const void* info)
-{
-    static_cast<JSLock*>(const_cast<void*>(info))->ref();
-    return info;
-}
-
-static void releaseAPILock(const void* info)
-{
-    static_cast<JSLock*>(const_cast<void*>(info))->deref();
-}
-
-HeapTimer::HeapTimer(VM* vm, CFRunLoopRef runLoop)
-    : m_vm(vm)
-    , m_runLoop(runLoop)
-{
-    memset(&m_context, 0, sizeof(CFRunLoopTimerContext));
-    m_context.info = &vm->apiLock();
-    m_context.retain = retainAPILock;
-    m_context.release = releaseAPILock;
-    m_timer = adoptCF(CFRunLoopTimerCreate(kCFAllocatorDefault, s_decade, s_decade, 0, 0, HeapTimer::timerDidFire, &m_context));
-    CFRunLoopAddTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
-}
-
-HeapTimer::~HeapTimer()
-{
-    CFRunLoopRemoveTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
-    CFRunLoopTimerInvalidate(m_timer.get());
-}
-
-void HeapTimer::timerDidFire(CFRunLoopTimerRef timer, void* context)
-{
-    JSLock* apiLock = static_cast<JSLock*>(context);
-    apiLock->lock();
-
-    RefPtr<VM> vm = apiLock->vm();
+HeapTimer::HeapTimer(VM* vm)
+    : m_vm(vm)
+    , m_apiLock(&vm->apiLock())
+{
+    setRunLoop(vm->heap.runLoop());
+}
+
+void HeapTimer::setRunLoop(CFRunLoopRef runLoop)
+{
+    if (m_runLoop) {
+        CFRunLoopRemoveTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
+        CFRunLoopTimerInvalidate(m_timer.get());
+        m_runLoop.clear();
+        m_timer.clear();
+    }
+    
+    if (runLoop) {
+        m_runLoop = runLoop;
+        memset(&m_context, 0, sizeof(CFRunLoopTimerContext));
+        m_context.info = this;
+        m_timer = adoptCF(CFRunLoopTimerCreate(kCFAllocatorDefault, s_decade, s_decade, 0, 0, HeapTimer::timerDidFire, &m_context));
+        CFRunLoopAddTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
+    }
+}
+
+HeapTimer::~HeapTimer()
+{
+    setRunLoop(0);
+}
+
+void HeapTimer::timerDidFire(CFRunLoopTimerRef, void* contextPtr)
+{
+    HeapTimer* timer = static_cast<HeapTimer*>(contextPtr);
+    timer->m_apiLock->lock();
+
+    RefPtr<VM> vm = timer->m_apiLock->vm();
     if (!vm) {
         // The VM has been destroyed, so we should just give up.
-        apiLock->unlock();
+        timer->m_apiLock->unlock();
         return;
     }
-
-    HeapTimer* heapTimer = 0;
-    if (vm->heap.fullActivityCallback() && vm->heap.fullActivityCallback()->m_timer.get() == timer)
-        heapTimer = vm->heap.fullActivityCallback();
-    else if (vm->heap.edenActivityCallback() && vm->heap.edenActivityCallback()->m_timer.get() == timer)
-        heapTimer = vm->heap.edenActivityCallback();
-    else if (vm->heap.sweeper()->m_timer.get() == timer)
-        heapTimer = vm->heap.sweeper();
-    else
-        RELEASE_ASSERT_NOT_REACHED();
 
     {
         JSLockHolder locker(vm.get());
-        heapTimer->doWork();
-    }
-
-    apiLock->unlock();
+        timer->doWork();
+    }
+
+    timer->m_apiLock->unlock();
+}
+
+void HeapTimer::scheduleTimer(double intervalInSeconds)
+{
+    CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + intervalInSeconds);
+}
+
+void HeapTimer::cancelTimer()
+{
+    CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + s_decade);
 }
 
@@ -110 +111 @@
 HeapTimer::HeapTimer(VM* vm)
     : m_vm(vm)
+    , m_apiLock(&vm->apiLock())
     , m_timer(0)
 {
@@ -144 +146 @@
 }
 
+void HeapTimer::scheduleTimer(double intervalInSeconds)
+{
+    if (ecore_timer_freeze_get(m_timer))
+        ecore_timer_thaw(m_timer);
+
+    double targetTime = currentTime() + intervalInSeconds;
+    ecore_timer_interval_set(m_timer, targetTime);
+}
+
+void HeapTimer::cancelTimer()
+{
+    ecore_timer_freeze(m_timer);
+}
 #elif USE(GLIB)
 
@@ -198 +213 @@
 }
 
+void HeapTimer::scheduleTimer(double intervalInSeconds)
+{
+    auto delayDuration = std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(intervalInSeconds));
+    gint64 currentTime = g_get_monotonic_time();
+    gint64 targetTime = currentTime + std::min<gint64>(G_MAXINT64 - currentTime, delayDuration.count());
+    ASSERT(targetTime >= currentTime);
+    g_source_set_ready_time(m_timer.get(), targetTime);
+}
+
+void HeapTimer::cancelTimer()
+{
+    g_source_set_ready_time(m_timer.get(), -1);
+}
 #else
 HeapTimer::HeapTimer(VM* vm)
@@ -212 +240 @@
 }
 
+void HeapTimer::scheduleTimer(double)
+{
+}
+
+void HeapTimer::cancelTimer()
+{
+}
 #endif