<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[207855] trunk/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/207855">207855</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2016-10-25 16:22:48 -0700 (Tue, 25 Oct 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapEdenGCActivityCallbackcpp">trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapEdenGCActivityCallbackh">trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapFullGCActivityCallbackcpp">trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapFullGCActivityCallbackh">trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapGCActivityCallbackcpp">trunk/Source/JavaScriptCore/heap/GCActivityCallback.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapGCActivityCallbackh">trunk/Source/JavaScriptCore/heap/GCActivityCallback.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapcpp">trunk/Source/JavaScriptCore/heap/Heap.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeaph">trunk/Source/JavaScriptCore/heap/Heap.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapTimercpp">trunk/Source/JavaScriptCore/heap/HeapTimer.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapTimerh">trunk/Source/JavaScriptCore/heap/HeapTimer.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapIncrementalSweepercpp">trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapIncrementalSweeperh">trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformiosWebSafeGCActivityCallbackIOSh">trunk/Source/WebCore/platform/ios/WebSafeGCActivityCallbackIOS.h</a></li>
<li><a href="#trunkSourceWebCoreplatformiosWebSafeIncrementalSweeperIOSh">trunk/Source/WebCore/platform/ios/WebSafeIncrementalSweeperIOS.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -1,3 +1,55 @@
</span><ins>+2016-10-25 Filip Pizlo <fpizlo@apple.com>
+
+ 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.
+
+ 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:
+
</ins><span class="cx"> 2016-10-25 JF Bastien <jfbastien@apple.com>
</span><span class="cx">
</span><span class="cx"> Remove redundant argument count check
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapEdenGCActivityCallbackcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.cpp 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -94,6 +94,6 @@
</span><span class="cx"> return 0;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-#endif // USE(CF) || PLATFORM(EFL)
</del><ins>+#endif // USE(CF) || USE(GLIB)
</ins><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapEdenGCActivityCallbackh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.h (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.h 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/EdenGCActivityCallback.h 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -36,13 +36,6 @@
</span><span class="cx"> void doCollection() override;
</span><span class="cx">
</span><span class="cx"> protected:
</span><del>-#if USE(CF)
- EdenGCActivityCallback(Heap* heap, CFRunLoopRef runLoop)
- : GCActivityCallback(heap, runLoop)
- {
- }
-#endif
-
</del><span class="cx"> double lastGCLength() override;
</span><span class="cx"> double gcTimeSlice(size_t bytes) override;
</span><span class="cx"> double deathRate() override;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapFullGCActivityCallbackcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -110,6 +110,6 @@
</span><span class="cx"> return 0;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-#endif // USE(CF) || PLATFORM(EFL)
</del><ins>+#endif // USE(CF) || USE(GLIB)
</ins><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapFullGCActivityCallbackh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.h (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.h 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.h 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -39,13 +39,6 @@
</span><span class="cx"> void setDidSyncGCRecently() { m_didSyncGCRecently = true; }
</span><span class="cx">
</span><span class="cx"> protected:
</span><del>-#if USE(CF)
- FullGCActivityCallback(Heap* heap, CFRunLoopRef runLoop)
- : GCActivityCallback(heap, runLoop)
- {
- }
-#endif
-
</del><span class="cx"> double lastGCLength() override;
</span><span class="cx"> double gcTimeSlice(size_t bytes) override;
</span><span class="cx"> double deathRate() override;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapGCActivityCallbackcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/GCActivityCallback.cpp (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/GCActivityCallback.cpp 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/GCActivityCallback.cpp 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -50,14 +50,9 @@
</span><span class="cx">
</span><span class="cx"> #if USE(CF)
</span><span class="cx"> GCActivityCallback::GCActivityCallback(Heap* heap)
</span><del>- : GCActivityCallback(heap->vm(), CFRunLoopGetCurrent())
</del><ins>+ : GCActivityCallback(heap->vm())
</ins><span class="cx"> {
</span><span class="cx"> }
</span><del>-
-GCActivityCallback::GCActivityCallback(Heap* heap, CFRunLoopRef runLoop)
- : GCActivityCallback(heap->vm(), runLoop)
-{
-}
</del><span class="cx"> #elif PLATFORM(EFL)
</span><span class="cx"> GCActivityCallback::GCActivityCallback(Heap* heap)
</span><span class="cx"> : GCActivityCallback(heap->vm(), WTF::isMainThread())
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapGCActivityCallbackh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/GCActivityCallback.h (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/GCActivityCallback.h 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/GCActivityCallback.h 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -70,8 +70,8 @@
</span><span class="cx"> virtual double deathRate() = 0;
</span><span class="cx">
</span><span class="cx"> #if USE(CF)
</span><del>- GCActivityCallback(VM* vm, CFRunLoopRef runLoop)
- : HeapTimer(vm, runLoop)
</del><ins>+ GCActivityCallback(VM* vm)
+ : HeapTimer(vm)
</ins><span class="cx"> , m_enabled(true)
</span><span class="cx"> , m_delay(s_decade)
</span><span class="cx"> {
</span><span class="lines">@@ -101,10 +101,6 @@
</span><span class="cx">
</span><span class="cx"> bool m_enabled;
</span><span class="cx">
</span><del>-#if USE(CF)
-protected:
- GCActivityCallback(Heap*, CFRunLoopRef);
-#endif
</del><span class="cx"> #if USE(CF) || USE(GLIB)
</span><span class="cx"> protected:
</span><span class="cx"> void cancelTimer();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.cpp (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.cpp 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/Heap.cpp 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -219,13 +219,12 @@
</span><span class="cx"> // schedule the timer if we've never done a collection.
</span><span class="cx"> , m_lastFullGCLength(0.01)
</span><span class="cx"> , m_lastEdenGCLength(0.01)
</span><ins>+#if USE(CF)
+ , m_runLoop(CFRunLoopGetCurrent())
+#endif // USE(CF)
</ins><span class="cx"> , m_fullActivityCallback(GCActivityCallback::createFullTimer(this))
</span><span class="cx"> , m_edenActivityCallback(GCActivityCallback::createEdenTimer(this))
</span><del>-#if USE(CF)
- , m_sweeper(std::make_unique<IncrementalSweeper>(this, CFRunLoopGetCurrent()))
-#else
</del><span class="cx"> , m_sweeper(std::make_unique<IncrementalSweeper>(this))
</span><del>-#endif
</del><span class="cx"> , m_deferralDepth(0)
</span><span class="cx"> #if USE(FOUNDATION)
</span><span class="cx"> , m_delayedReleaseRecursionCount(0)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.h (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.h 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/Heap.h 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -270,6 +270,10 @@
</span><span class="cx"> unsigned barrierThreshold() const { return m_barrierThreshold; }
</span><span class="cx"> const unsigned* addressOfBarrierThreshold() const { return &m_barrierThreshold; }
</span><span class="cx">
</span><ins>+#if USE(CF)
+ CFRunLoopRef runLoop() const { return m_runLoop.get(); }
+#endif // USE(CF)
+
</ins><span class="cx"> private:
</span><span class="cx"> friend class AllocatingScope;
</span><span class="cx"> friend class CodeBlock;
</span><span class="lines">@@ -444,6 +448,9 @@
</span><span class="cx"> Vector<WeakBlock*> m_logicallyEmptyWeakBlocks;
</span><span class="cx"> size_t m_indexOfNextLogicallyEmptyWeakBlockToSweep { WTF::notFound };
</span><span class="cx">
</span><ins>+#if USE(CF)
+ RetainPtr<CFRunLoopRef> m_runLoop;
+#endif // USE(CF)
</ins><span class="cx"> RefPtr<FullGCActivityCallback> m_fullActivityCallback;
</span><span class="cx"> RefPtr<GCActivityCallback> m_edenActivityCallback;
</span><span class="cx"> std::unique_ptr<IncrementalSweeper> m_sweeper;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapTimercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/HeapTimer.cpp (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/HeapTimer.cpp 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/HeapTimer.cpp 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2012 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2012, 2016 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -46,69 +46,71 @@
</span><span class="cx">
</span><span class="cx"> const CFTimeInterval HeapTimer::s_decade = 60 * 60 * 24 * 365 * 10;
</span><span class="cx">
</span><del>-static const void* retainAPILock(const void* info)
</del><ins>+HeapTimer::HeapTimer(VM* vm)
+ : m_vm(vm)
+ , m_apiLock(&vm->apiLock())
</ins><span class="cx"> {
</span><del>- static_cast<JSLock*>(const_cast<void*>(info))->ref();
- return info;
</del><ins>+ setRunLoop(vm->heap.runLoop());
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-static void releaseAPILock(const void* info)
</del><ins>+void HeapTimer::setRunLoop(CFRunLoopRef runLoop)
</ins><span class="cx"> {
</span><del>- static_cast<JSLock*>(const_cast<void*>(info))->deref();
</del><ins>+ 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);
+ }
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-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);
-}
-
</del><span class="cx"> HeapTimer::~HeapTimer()
</span><span class="cx"> {
</span><del>- CFRunLoopRemoveTimer(m_runLoop.get(), m_timer.get(), kCFRunLoopCommonModes);
- CFRunLoopTimerInvalidate(m_timer.get());
</del><ins>+ setRunLoop(0);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void HeapTimer::timerDidFire(CFRunLoopTimerRef timer, void* context)
</del><ins>+void HeapTimer::timerDidFire(CFRunLoopTimerRef, void* contextPtr)
</ins><span class="cx"> {
</span><del>- JSLock* apiLock = static_cast<JSLock*>(context);
- apiLock->lock();
</del><ins>+ HeapTimer* timer = static_cast<HeapTimer*>(contextPtr);
+ timer->m_apiLock->lock();
</ins><span class="cx">
</span><del>- RefPtr<VM> vm = apiLock->vm();
</del><ins>+ RefPtr<VM> vm = timer->m_apiLock->vm();
</ins><span class="cx"> if (!vm) {
</span><span class="cx"> // The VM has been destroyed, so we should just give up.
</span><del>- apiLock->unlock();
</del><ins>+ timer->m_apiLock->unlock();
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- 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();
-
</del><span class="cx"> {
</span><span class="cx"> JSLockHolder locker(vm.get());
</span><del>- heapTimer->doWork();
</del><ins>+ timer->doWork();
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- apiLock->unlock();
</del><ins>+ timer->m_apiLock->unlock();
</ins><span class="cx"> }
</span><span class="cx">
</span><ins>+void HeapTimer::scheduleTimer(double intervalInSeconds)
+{
+ CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + intervalInSeconds);
+}
+
+void HeapTimer::cancelTimer()
+{
+ CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + s_decade);
+}
+
</ins><span class="cx"> #elif PLATFORM(EFL)
</span><span class="cx">
</span><span class="cx"> HeapTimer::HeapTimer(VM* vm)
</span><span class="cx"> : m_vm(vm)
</span><ins>+ , m_apiLock(&vm->apiLock())
</ins><span class="cx"> , m_timer(0)
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="lines">@@ -143,6 +145,19 @@
</span><span class="cx"> return ECORE_CALLBACK_CANCEL;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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);
+}
</ins><span class="cx"> #elif USE(GLIB)
</span><span class="cx">
</span><span class="cx"> static GSourceFuncs heapTimerSourceFunctions = {
</span><span class="lines">@@ -197,6 +212,19 @@
</span><span class="cx"> m_apiLock->unlock();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+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);
+}
</ins><span class="cx"> #else
</span><span class="cx"> HeapTimer::HeapTimer(VM* vm)
</span><span class="cx"> : m_vm(vm)
</span><span class="lines">@@ -211,6 +239,13 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void HeapTimer::scheduleTimer(double)
+{
+}
+
+void HeapTimer::cancelTimer()
+{
+}
</ins><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapTimerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/HeapTimer.h (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/HeapTimer.h 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/HeapTimer.h 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2012, 2015 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2012, 2015-2016 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -26,7 +26,9 @@
</span><span class="cx"> #pragma once
</span><span class="cx">
</span><span class="cx"> #include <wtf/Lock.h>
</span><ins>+#include <wtf/RefPtr.h>
</ins><span class="cx"> #include <wtf/RetainPtr.h>
</span><ins>+#include <wtf/ThreadSafeRefCounted.h>
</ins><span class="cx"> #include <wtf/Threading.h>
</span><span class="cx">
</span><span class="cx"> #if USE(CF)
</span><span class="lines">@@ -44,24 +46,31 @@
</span><span class="cx">
</span><span class="cx"> class HeapTimer {
</span><span class="cx"> public:
</span><ins>+ HeapTimer(VM*);
</ins><span class="cx"> #if USE(CF)
</span><del>- HeapTimer(VM*, CFRunLoopRef);
</del><span class="cx"> static void timerDidFire(CFRunLoopTimerRef, void*);
</span><del>-#else
- HeapTimer(VM*);
</del><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> JS_EXPORT_PRIVATE virtual ~HeapTimer();
</span><span class="cx"> virtual void doWork() = 0;
</span><ins>+
+ void scheduleTimer(double intervalInSeconds);
+ void cancelTimer();
+
+#if USE(CF)
+ JS_EXPORT_PRIVATE void setRunLoop(CFRunLoopRef);
+#endif // USE(CF)
</ins><span class="cx">
</span><span class="cx"> protected:
</span><span class="cx"> VM* m_vm;
</span><span class="cx">
</span><ins>+ RefPtr<JSLock> m_apiLock;
</ins><span class="cx"> #if USE(CF)
</span><span class="cx"> static const CFTimeInterval s_decade;
</span><span class="cx">
</span><span class="cx"> RetainPtr<CFRunLoopTimerRef> m_timer;
</span><span class="cx"> RetainPtr<CFRunLoopRef> m_runLoop;
</span><ins>+
</ins><span class="cx"> CFRunLoopTimerContext m_context;
</span><span class="cx">
</span><span class="cx"> Lock m_shutdownMutex;
</span><span class="lines">@@ -72,7 +81,6 @@
</span><span class="cx"> Ecore_Timer* m_timer;
</span><span class="cx"> #elif USE(GLIB)
</span><span class="cx"> void timerDidFire();
</span><del>- RefPtr<JSLock> m_apiLock;
</del><span class="cx"> GRefPtr<GSource> m_timer;
</span><span class="cx"> #endif
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapIncrementalSweepercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/IncrementalSweeper.cpp 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -31,39 +31,19 @@
</span><span class="cx"> #include "JSString.h"
</span><span class="cx"> #include "MarkedBlock.h"
</span><span class="cx"> #include "JSCInlines.h"
</span><del>-
-#if PLATFORM(EFL)
-#include <Ecore.h>
</del><span class="cx"> #include <wtf/CurrentTime.h>
</span><del>-#elif USE(GLIB)
-#include <glib.h>
-#endif
</del><span class="cx">
</span><span class="cx"> namespace JSC {
</span><span class="cx">
</span><del>-#if USE(CF) || PLATFORM(EFL) || USE(GLIB)
-
</del><span class="cx"> static const double sweepTimeSlice = .01; // seconds
</span><span class="cx"> static const double sweepTimeTotal = .10;
</span><span class="cx"> static const double sweepTimeMultiplier = 1.0 / sweepTimeTotal;
</span><span class="cx">
</span><del>-#if USE(CF)
-IncrementalSweeper::IncrementalSweeper(Heap* heap, CFRunLoopRef runLoop)
- : HeapTimer(heap->vm(), runLoop)
- , m_currentAllocator(nullptr)
-{
-}
-
</del><span class="cx"> void IncrementalSweeper::scheduleTimer()
</span><span class="cx"> {
</span><del>- CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + (sweepTimeSlice * sweepTimeMultiplier));
</del><ins>+ HeapTimer::scheduleTimer(sweepTimeSlice * sweepTimeMultiplier);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>-void IncrementalSweeper::cancelTimer()
-{
- CFRunLoopTimerSetNextFireDate(m_timer.get(), CFAbsoluteTimeGetCurrent() + s_decade);
-}
-#elif PLATFORM(EFL)
</del><span class="cx"> IncrementalSweeper::IncrementalSweeper(Heap* heap)
</span><span class="cx"> : HeapTimer(heap->vm())
</span><span class="cx"> , m_currentAllocator(nullptr)
</span><span class="lines">@@ -70,41 +50,6 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void IncrementalSweeper::scheduleTimer()
-{
- if (ecore_timer_freeze_get(m_timer))
- ecore_timer_thaw(m_timer);
-
- double targetTime = currentTime() + (sweepTimeSlice * sweepTimeMultiplier);
- ecore_timer_interval_set(m_timer, targetTime);
-}
-
-void IncrementalSweeper::cancelTimer()
-{
- ecore_timer_freeze(m_timer);
-}
-#elif USE(GLIB)
-IncrementalSweeper::IncrementalSweeper(Heap* heap)
- : HeapTimer(heap->vm())
- , m_currentAllocator(nullptr)
-{
-}
-
-void IncrementalSweeper::scheduleTimer()
-{
- auto delayDuration = std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(sweepTimeSlice * sweepTimeMultiplier));
- 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 IncrementalSweeper::cancelTimer()
-{
- g_source_set_ready_time(m_timer.get(), -1);
-}
-#endif
-
</del><span class="cx"> void IncrementalSweeper::doWork()
</span><span class="cx"> {
</span><span class="cx"> doSweep(WTF::monotonicallyIncreasingTime());
</span><span class="lines">@@ -157,30 +102,4 @@
</span><span class="cx"> cancelTimer();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-#else
-
-IncrementalSweeper::IncrementalSweeper(Heap* heap)
- : HeapTimer(heap->vm())
-{
-}
-
-void IncrementalSweeper::doWork()
-{
-}
-
-void IncrementalSweeper::startSweeping()
-{
-}
-
-void IncrementalSweeper::willFinishSweeping()
-{
-}
-
-bool IncrementalSweeper::sweepNextBlock()
-{
- return false;
-}
-
-#endif
-
</del><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapIncrementalSweeperh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/JavaScriptCore/heap/IncrementalSweeper.h 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -36,11 +36,7 @@
</span><span class="cx"> class IncrementalSweeper : public HeapTimer {
</span><span class="cx"> WTF_MAKE_FAST_ALLOCATED;
</span><span class="cx"> public:
</span><del>-#if USE(CF)
- JS_EXPORT_PRIVATE IncrementalSweeper(Heap*, CFRunLoopRef);
-#else
- explicit IncrementalSweeper(Heap*);
-#endif
</del><ins>+ JS_EXPORT_PRIVATE explicit IncrementalSweeper(Heap*);
</ins><span class="cx">
</span><span class="cx"> void startSweeping();
</span><span class="cx">
</span><span class="lines">@@ -48,14 +44,11 @@
</span><span class="cx"> bool sweepNextBlock();
</span><span class="cx"> void willFinishSweeping();
</span><span class="cx">
</span><del>-#if USE(CF) || PLATFORM(EFL) || USE(GLIB)
</del><span class="cx"> private:
</span><span class="cx"> void doSweep(double startTime);
</span><span class="cx"> void scheduleTimer();
</span><del>- void cancelTimer();
</del><span class="cx">
</span><span class="cx"> MarkedAllocator* m_currentAllocator;
</span><del>-#endif
</del><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/WebCore/ChangeLog 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2016-10-25 Filip Pizlo <fpizlo@apple.com>
+
+ 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.
+
+ No new tests because no new behavior.
+
+ * platform/ios/WebSafeGCActivityCallbackIOS.h:
+ * platform/ios/WebSafeIncrementalSweeperIOS.h:
+
</ins><span class="cx"> 2016-10-25 Dave Hyatt <hyatt@apple.com>
</span><span class="cx">
</span><span class="cx"> [CSS Parser] Improvements to selector parsing
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformiosWebSafeGCActivityCallbackIOSh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/ios/WebSafeGCActivityCallbackIOS.h (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/ios/WebSafeGCActivityCallbackIOS.h 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/WebCore/platform/ios/WebSafeGCActivityCallbackIOS.h 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -43,8 +43,9 @@
</span><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> WebSafeFullGCActivityCallback(JSC::Heap* heap)
</span><del>- : JSC::FullGCActivityCallback(heap, WebThreadRunLoop())
</del><ins>+ : JSC::FullGCActivityCallback(heap)
</ins><span class="cx"> {
</span><ins>+ setRunLoop(WebThreadRunLoop());
</ins><span class="cx"> }
</span><span class="cx"> };
</span><span class="cx">
</span><span class="lines">@@ -59,8 +60,9 @@
</span><span class="cx">
</span><span class="cx"> private:
</span><span class="cx"> WebSafeEdenGCActivityCallback(JSC::Heap* heap)
</span><del>- : JSC::EdenGCActivityCallback(heap, WebThreadRunLoop())
</del><ins>+ : JSC::EdenGCActivityCallback(heap)
</ins><span class="cx"> {
</span><ins>+ setRunLoop(WebThreadRunLoop());
</ins><span class="cx"> }
</span><span class="cx"> };
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformiosWebSafeIncrementalSweeperIOSh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/ios/WebSafeIncrementalSweeperIOS.h (207854 => 207855)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/ios/WebSafeIncrementalSweeperIOS.h 2016-10-25 23:21:03 UTC (rev 207854)
+++ trunk/Source/WebCore/platform/ios/WebSafeIncrementalSweeperIOS.h 2016-10-25 23:22:48 UTC (rev 207855)
</span><span class="lines">@@ -34,8 +34,9 @@
</span><span class="cx"> class WebSafeIncrementalSweeper final : public JSC::IncrementalSweeper {
</span><span class="cx"> public:
</span><span class="cx"> explicit WebSafeIncrementalSweeper(JSC::Heap* heap)
</span><del>- : JSC::IncrementalSweeper(heap, WebThreadRunLoop())
</del><ins>+ : JSC::IncrementalSweeper(heap)
</ins><span class="cx"> {
</span><ins>+ setRunLoop(WebThreadRunLoop());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ~WebSafeIncrementalSweeper() override { }
</span></span></pre>
</div>
</div>
</body>
</html>