<!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>[179609] 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/179609">179609</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-02-04 10:00:05 -0800 (Wed, 04 Feb 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>Remove concept of makeUsableFromMultipleThreads().
&lt;https://webkit.org/b/141221&gt;

Reviewed by Mark Hahnenberg.

Source/JavaScriptCore:

Currently, we rely on VM::makeUsableFromMultipleThreads() being called before we
start acquiring the JSLock and entering the VM from different threads.
Acquisition of the JSLock will register the acquiring thread with the VM's thread
registry if not already registered.  However, it will only do this if the VM's
thread specific key has been initialized by makeUsableFromMultipleThreads().

This is fragile, and also does not read intuitively because one would expect to
acquire the JSLock before calling any methods on the VM.  This is exactly what
JSGlobalContextCreateInGroup() did (i.e. acquire the lock before calling
makeUsableFromMultipleThreads()), but is wrong.  The result is that the invoking
thread will not have been registered with the VM during that first entry into
the VM.

The fix is to make it so that we initialize the VM's thread specific key on
construction of the VM's MachineThreads registry instead of relying on
makeUsableFromMultipleThreads() being called.  With this, we can eliminate
makeUsableFromMultipleThreads() altogether.

Performance results are neutral in aggregate.

* API/JSContextRef.cpp:
(JSGlobalContextCreateInGroup):
* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::MachineThreads):
(JSC::MachineThreads::~MachineThreads):
(JSC::MachineThreads::addCurrentThread):
(JSC::MachineThreads::removeThread):
(JSC::MachineThreads::gatherConservativeRoots):
(JSC::MachineThreads::makeUsableFromMultipleThreads): Deleted.
* heap/MachineStackMarker.h:
* runtime/VM.cpp:
(JSC::VM::sharedInstance):
* runtime/VM.h:
(JSC::VM::makeUsableFromMultipleThreads): Deleted.

Source/WebCore:

No new tests.

* bindings/js/JSDOMWindowBase.cpp:
(WebCore::JSDOMWindowBase::commonVM):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreAPIJSContextRefcpp">trunk/Source/JavaScriptCore/API/JSContextRef.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkercpp">trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkerh">trunk/Source/JavaScriptCore/heap/MachineStackMarker.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeVMcpp">trunk/Source/JavaScriptCore/runtime/VM.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeVMh">trunk/Source/JavaScriptCore/runtime/VM.h</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSDOMWindowBasecpp">trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreAPIJSContextRefcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/API/JSContextRef.cpp (179608 => 179609)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/API/JSContextRef.cpp        2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/JavaScriptCore/API/JSContextRef.cpp        2015-02-04 18:00:05 UTC (rev 179609)
</span><span class="lines">@@ -138,7 +138,6 @@
</span><span class="cx">     RefPtr&lt;VM&gt; vm = group ? PassRefPtr&lt;VM&gt;(toJS(group)) : VM::createContextGroup();
</span><span class="cx"> 
</span><span class="cx">     JSLockHolder locker(vm.get());
</span><del>-    vm-&gt;makeUsableFromMultipleThreads();
</del><span class="cx"> 
</span><span class="cx">     if (!globalObjectClass) {
</span><span class="cx">         JSGlobalObject* globalObject = JSGlobalObject::create(*vm, JSGlobalObject::createStructure(*vm, jsNull()));
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (179608 => 179609)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/JavaScriptCore/ChangeLog        2015-02-04 18:00:05 UTC (rev 179609)
</span><span class="lines">@@ -1,3 +1,45 @@
</span><ins>+2015-02-04  Mark Lam  &lt;mark.lam@apple.com&gt;
+
+        Remove concept of makeUsableFromMultipleThreads().
+        &lt;https://webkit.org/b/141221&gt;
+
+        Reviewed by Mark Hahnenberg.
+
+        Currently, we rely on VM::makeUsableFromMultipleThreads() being called before we
+        start acquiring the JSLock and entering the VM from different threads.
+        Acquisition of the JSLock will register the acquiring thread with the VM's thread
+        registry if not already registered.  However, it will only do this if the VM's
+        thread specific key has been initialized by makeUsableFromMultipleThreads().
+
+        This is fragile, and also does not read intuitively because one would expect to
+        acquire the JSLock before calling any methods on the VM.  This is exactly what
+        JSGlobalContextCreateInGroup() did (i.e. acquire the lock before calling
+        makeUsableFromMultipleThreads()), but is wrong.  The result is that the invoking
+        thread will not have been registered with the VM during that first entry into
+        the VM.
+
+        The fix is to make it so that we initialize the VM's thread specific key on
+        construction of the VM's MachineThreads registry instead of relying on
+        makeUsableFromMultipleThreads() being called.  With this, we can eliminate
+        makeUsableFromMultipleThreads() altogether.
+
+        Performance results are neutral in aggregate.
+
+        * API/JSContextRef.cpp:
+        (JSGlobalContextCreateInGroup):
+        * heap/MachineStackMarker.cpp:
+        (JSC::MachineThreads::MachineThreads):
+        (JSC::MachineThreads::~MachineThreads):
+        (JSC::MachineThreads::addCurrentThread):
+        (JSC::MachineThreads::removeThread):
+        (JSC::MachineThreads::gatherConservativeRoots):
+        (JSC::MachineThreads::makeUsableFromMultipleThreads): Deleted.
+        * heap/MachineStackMarker.h:
+        * runtime/VM.cpp:
+        (JSC::VM::sharedInstance):
+        * runtime/VM.h:
+        (JSC::VM::makeUsableFromMultipleThreads): Deleted.
+
</ins><span class="cx"> 2015-02-04  Chris Dumez  &lt;cdumez@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Add removeFirst(value) / removeAll(value) methods to WTF::Vector
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp (179608 => 179609)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp        2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp        2015-02-04 18:00:05 UTC (rev 179609)
</span><span class="lines">@@ -123,12 +123,12 @@
</span><span class="cx"> #endif
</span><span class="cx"> {
</span><span class="cx">     UNUSED_PARAM(heap);
</span><ins>+    threadSpecificKeyCreate(&amp;m_threadSpecific, removeThread);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> MachineThreads::~MachineThreads()
</span><span class="cx"> {
</span><del>-    if (m_threadSpecific)
-        threadSpecificKeyDelete(m_threadSpecific);
</del><ins>+    threadSpecificKeyDelete(m_threadSpecific);
</ins><span class="cx"> 
</span><span class="cx">     MutexLocker registeredThreadsLock(m_registeredThreadsMutex);
</span><span class="cx">     for (Thread* t = m_registeredThreads; t;) {
</span><span class="lines">@@ -160,20 +160,14 @@
</span><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void MachineThreads::makeUsableFromMultipleThreads()
-{
-    if (m_threadSpecific)
-        return;
-
-    threadSpecificKeyCreate(&amp;m_threadSpecific, removeThread);
-}
-
</del><span class="cx"> void MachineThreads::addCurrentThread()
</span><span class="cx"> {
</span><span class="cx">     ASSERT(!m_heap-&gt;vm()-&gt;hasExclusiveThread() || m_heap-&gt;vm()-&gt;exclusiveThread() == std::this_thread::get_id());
</span><span class="cx"> 
</span><del>-    if (!m_threadSpecific || threadSpecificGet(m_threadSpecific))
</del><ins>+    if (threadSpecificGet(m_threadSpecific)) {
+        ASSERT(threadSpecificGet(m_threadSpecific) == this);
</ins><span class="cx">         return;
</span><ins>+    }
</ins><span class="cx"> 
</span><span class="cx">     threadSpecificSet(m_threadSpecific, this);
</span><span class="cx">     Thread* thread = new Thread(getCurrentPlatformThread(), wtfThreadData().stack().origin());
</span><span class="lines">@@ -186,8 +180,7 @@
</span><span class="cx"> 
</span><span class="cx"> void MachineThreads::removeThread(void* p)
</span><span class="cx"> {
</span><del>-    if (p)
-        static_cast&lt;MachineThreads*&gt;(p)-&gt;removeCurrentThread();
</del><ins>+    static_cast&lt;MachineThreads*&gt;(p)-&gt;removeCurrentThread();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> template&lt;typename PlatformThread&gt;
</span><span class="lines">@@ -521,9 +514,6 @@
</span><span class="cx"> {
</span><span class="cx">     gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent, currentThreadRegisters);
</span><span class="cx"> 
</span><del>-    if (!m_threadSpecific)
-        return;
-
</del><span class="cx">     size_t size;
</span><span class="cx">     size_t capacity = 0;
</span><span class="cx">     void* buffer = nullptr;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.h (179608 => 179609)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.h        2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.h        2015-02-04 18:00:05 UTC (rev 179609)
</span><span class="lines">@@ -44,7 +44,6 @@
</span><span class="cx"> 
</span><span class="cx">         void gatherConservativeRoots(ConservativeRoots&amp;, JITStubRoutineSet&amp;, CodeBlockSet&amp;, void* stackCurrent, RegisterState&amp; registers);
</span><span class="cx"> 
</span><del>-        JS_EXPORT_PRIVATE void makeUsableFromMultipleThreads();
</del><span class="cx">         JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads.
</span><span class="cx"> 
</span><span class="cx">     private:
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/VM.cpp (179608 => 179609)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/VM.cpp        2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/JavaScriptCore/runtime/VM.cpp        2015-02-04 18:00:05 UTC (rev 179609)
</span><span class="lines">@@ -357,10 +357,8 @@
</span><span class="cx"> {
</span><span class="cx">     GlobalJSLock globalLock;
</span><span class="cx">     VM*&amp; instance = sharedInstanceInternal();
</span><del>-    if (!instance) {
</del><ins>+    if (!instance)
</ins><span class="cx">         instance = adoptRef(new VM(APIShared, SmallHeap)).leakRef();
</span><del>-        instance-&gt;makeUsableFromMultipleThreads();
-    }
</del><span class="cx">     return *instance;
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeVMh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/VM.h (179608 => 179609)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/VM.h        2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/JavaScriptCore/runtime/VM.h        2015-02-04 18:00:05 UTC (rev 179609)
</span><span class="lines">@@ -217,8 +217,6 @@
</span><span class="cx">     static PassRefPtr&lt;VM&gt; createContextGroup(HeapType = SmallHeap);
</span><span class="cx">     JS_EXPORT_PRIVATE ~VM();
</span><span class="cx"> 
</span><del>-    void makeUsableFromMultipleThreads() { heap.machineThreads().makeUsableFromMultipleThreads(); }
-
</del><span class="cx"> private:
</span><span class="cx">     RefPtr&lt;JSLock&gt; m_apiLock;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (179608 => 179609)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/WebCore/ChangeLog        2015-02-04 18:00:05 UTC (rev 179609)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2015-02-04  Mark Lam  &lt;mark.lam@apple.com&gt;
+
+        Remove concept of makeUsableFromMultipleThreads().
+        &lt;https://webkit.org/b/141221&gt;
+
+        Reviewed by Mark Hahnenberg.
+
+        No new tests.
+
+        * bindings/js/JSDOMWindowBase.cpp:
+        (WebCore::JSDOMWindowBase::commonVM):
+
</ins><span class="cx"> 2015-02-04  Simon Fraser  &lt;simon.fraser@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [iOS WK2] Assert in ScrollingTreeOverflowScrollingNodeIOS::updateAfterChildren() on tab switching
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSDOMWindowBasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp (179608 => 179609)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp        2015-02-04 18:00:00 UTC (rev 179608)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp        2015-02-04 18:00:05 UTC (rev 179609)
</span><span class="lines">@@ -212,7 +212,6 @@
</span><span class="cx">         vm-&gt;heap.setEdenActivityCallback(vm-&gt;heap.fullActivityCallback());
</span><span class="cx"> #endif
</span><span class="cx">         vm-&gt;heap.setIncrementalSweeper(std::make_unique&lt;WebSafeIncrementalSweeper&gt;(&amp;vm-&gt;heap));
</span><del>-        vm-&gt;makeUsableFromMultipleThreads();
</del><span class="cx">         vm-&gt;heap.machineThreads().addCurrentThread();
</span><span class="cx"> #endif
</span><span class="cx">         initNormalWorldClientData(vm);
</span></span></pre>
</div>
</div>

</body>
</html>