<!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>[183212] trunk/Source/JavaScriptCore</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/183212">183212</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2015-04-23 14:56:23 -0700 (Thu, 23 Apr 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Make FunctionRareData allocation thread-safe
https://bugs.webkit.org/show_bug.cgi?id=144001
Patch by Basile Clement <basile_clement@apple.com> on 2015-04-23
Reviewed by Mark Lam.
The two things we want to prevent are:
1. A thread seeing a pointer to a not-yet-fully-created rare data from
a JSFunction
2. A thread seeing a pointer to a not-yet-fully-created Structure from
an ObjectAllocationProfile
For 1., only the JS thread can be creating the rare data (in
runtime/CommonSlowPaths.cpp or in dfg/DFGOperations.cpp), so we don't need to
worry about concurrent writes, and we don't need any fences when *reading* the
rare data from the JS thread. Thus we only need a storeStoreFence between the
rare data creation and assignment to m_rareData in
JSFunction::createAndInitializeRareData() to ensure that when the store to
m_rareData is issued, the rare data has been properly created.
For the DFG compilation threads, the only place they can access the
rare data is through JSFunction::rareData(), and so we only need a
loadLoadFence there to ensure that when we see a non-null pointer in
m_rareData, the pointed object will be seen as a fully created
FunctionRareData.
For 2., the structure is created in
ObjectAllocationProfile::initialize() (which appears to be called only by the
JS thread as well, in bytecode/CodeBlock.cpp and on rare data initialization,
which always happen in the JS thread), and read through
ObjectAllocationProfile::structure() and
ObjectAllocationProfile::inlineCapacity(), so following the same reasoning we
put a storeStoreFence in ObjectAllocationProfile::initialize() and a
loadLoadFence in ObjectAllocationProfile::structure() (and change
ObjectAllocationProfile::inlineCapacity() to go through
ObjectAllocationProfile::structure()).
We don't need a fence in ObjectAllocationProfile::clear() because
clearing the structure is already as atomic as it gets.
Finally, notice that we don't care about the ObjectAllocationProfile's
m_allocator as that is only used by ObjectAllocationProfile::initialize() and
ObjectAllocationProfile::clear() that are always run in the JS thread.
ObjectAllocationProfile::isNull() could cause some trouble, but it is
currently only used in the ObjectAllocationProfile::clear()'s ASSERT in the JS
thread. Doing isNull()-style pre-checks would be wrong in any other concurrent
thread anyway.
* bytecode/ObjectAllocationProfile.h:
(JSC::ObjectAllocationProfile::initialize):
(JSC::ObjectAllocationProfile::structure):
(JSC::ObjectAllocationProfile::inlineCapacity):
* runtime/JSFunction.cpp:
(JSC::JSFunction::allocateAndInitializeRareData):
* runtime/JSFunction.h:
(JSC::JSFunction::rareData):
(JSC::JSFunction::allocationStructure): Deleted.
This is no longer used, as all the accesses to the ObjectAllocationProfile go through the rare data.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeObjectAllocationProfileh">trunk/Source/JavaScriptCore/bytecode/ObjectAllocationProfile.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSFunctioncpp">trunk/Source/JavaScriptCore/runtime/JSFunction.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSFunctionh">trunk/Source/JavaScriptCore/runtime/JSFunction.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (183211 => 183212)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-04-23 21:33:23 UTC (rev 183211)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-04-23 21:56:23 UTC (rev 183212)
</span><span class="lines">@@ -1,3 +1,65 @@
</span><ins>+2015-04-23 Basile Clement <basile_clement@apple.com>
+
+ Make FunctionRareData allocation thread-safe
+ https://bugs.webkit.org/show_bug.cgi?id=144001
+
+ Reviewed by Mark Lam.
+
+ The two things we want to prevent are:
+
+ 1. A thread seeing a pointer to a not-yet-fully-created rare data from
+ a JSFunction
+ 2. A thread seeing a pointer to a not-yet-fully-created Structure from
+ an ObjectAllocationProfile
+
+ For 1., only the JS thread can be creating the rare data (in
+ runtime/CommonSlowPaths.cpp or in dfg/DFGOperations.cpp), so we don't need to
+ worry about concurrent writes, and we don't need any fences when *reading* the
+ rare data from the JS thread. Thus we only need a storeStoreFence between the
+ rare data creation and assignment to m_rareData in
+ JSFunction::createAndInitializeRareData() to ensure that when the store to
+ m_rareData is issued, the rare data has been properly created.
+
+ For the DFG compilation threads, the only place they can access the
+ rare data is through JSFunction::rareData(), and so we only need a
+ loadLoadFence there to ensure that when we see a non-null pointer in
+ m_rareData, the pointed object will be seen as a fully created
+ FunctionRareData.
+
+
+ For 2., the structure is created in
+ ObjectAllocationProfile::initialize() (which appears to be called only by the
+ JS thread as well, in bytecode/CodeBlock.cpp and on rare data initialization,
+ which always happen in the JS thread), and read through
+ ObjectAllocationProfile::structure() and
+ ObjectAllocationProfile::inlineCapacity(), so following the same reasoning we
+ put a storeStoreFence in ObjectAllocationProfile::initialize() and a
+ loadLoadFence in ObjectAllocationProfile::structure() (and change
+ ObjectAllocationProfile::inlineCapacity() to go through
+ ObjectAllocationProfile::structure()).
+
+ We don't need a fence in ObjectAllocationProfile::clear() because
+ clearing the structure is already as atomic as it gets.
+
+ Finally, notice that we don't care about the ObjectAllocationProfile's
+ m_allocator as that is only used by ObjectAllocationProfile::initialize() and
+ ObjectAllocationProfile::clear() that are always run in the JS thread.
+ ObjectAllocationProfile::isNull() could cause some trouble, but it is
+ currently only used in the ObjectAllocationProfile::clear()'s ASSERT in the JS
+ thread. Doing isNull()-style pre-checks would be wrong in any other concurrent
+ thread anyway.
+
+ * bytecode/ObjectAllocationProfile.h:
+ (JSC::ObjectAllocationProfile::initialize):
+ (JSC::ObjectAllocationProfile::structure):
+ (JSC::ObjectAllocationProfile::inlineCapacity):
+ * runtime/JSFunction.cpp:
+ (JSC::JSFunction::allocateAndInitializeRareData):
+ * runtime/JSFunction.h:
+ (JSC::JSFunction::rareData):
+ (JSC::JSFunction::allocationStructure): Deleted.
+ This is no longer used, as all the accesses to the ObjectAllocationProfile go through the rare data.
+
</ins><span class="cx"> 2015-04-22 Filip Pizlo <fpizlo@apple.com>
</span><span class="cx">
</span><span class="cx"> DFG should insert Phantoms late using BytecodeKills and block-local OSR availability
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeObjectAllocationProfileh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/ObjectAllocationProfile.h (183211 => 183212)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/ObjectAllocationProfile.h 2015-04-23 21:33:23 UTC (rev 183211)
+++ trunk/Source/JavaScriptCore/bytecode/ObjectAllocationProfile.h 2015-04-23 21:56:23 UTC (rev 183212)
</span><span class="lines">@@ -89,13 +89,23 @@
</span><span class="cx"> if (inlineCapacity > JSFinalObject::maxInlineCapacity())
</span><span class="cx"> inlineCapacity = JSFinalObject::maxInlineCapacity();
</span><span class="cx">
</span><ins>+ Structure* structure = vm.prototypeMap.emptyObjectStructureForPrototype(prototype, inlineCapacity);
+
+ // Ensure that if another thread sees the structure, it will see it properly created
+ WTF::storeStoreFence();
+
</ins><span class="cx"> m_allocator = allocator;
</span><del>- m_structure.set(vm, owner,
- vm.prototypeMap.emptyObjectStructureForPrototype(prototype, inlineCapacity));
</del><ins>+ m_structure.set(vm, owner, structure);
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- Structure* structure() { return m_structure.get(); }
- unsigned inlineCapacity() { return m_structure->inlineCapacity(); }
</del><ins>+ Structure* structure()
+ {
+ Structure* structure = m_structure.get();
+ // Ensure that if we see the structure, it has been properly created
+ WTF::loadLoadFence();
+ return structure;
+ }
+ unsigned inlineCapacity() { return structure()->inlineCapacity(); }
</ins><span class="cx">
</span><span class="cx"> void clear()
</span><span class="cx"> {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSFunctioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSFunction.cpp (183211 => 183212)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSFunction.cpp 2015-04-23 21:33:23 UTC (rev 183211)
+++ trunk/Source/JavaScriptCore/runtime/JSFunction.cpp 2015-04-23 21:56:23 UTC (rev 183212)
</span><span class="lines">@@ -116,6 +116,11 @@
</span><span class="cx"> if (!prototype)
</span><span class="cx"> prototype = globalObject()->objectPrototype();
</span><span class="cx"> FunctionRareData* rareData = FunctionRareData::create(vm, prototype, inlineCapacity);
</span><ins>+
+ // A DFG compilation thread may be trying to read the rare data
+ // We want to ensure that it sees it properly allocated
+ WTF::storeStoreFence();
+
</ins><span class="cx"> m_rareData.set(vm, this, rareData);
</span><span class="cx"> return m_rareData.get();
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSFunctionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSFunction.h (183211 => 183212)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSFunction.h 2015-04-23 21:33:23 UTC (rev 183211)
+++ trunk/Source/JavaScriptCore/runtime/JSFunction.h 2015-04-23 21:56:23 UTC (rev 183212)
</span><span class="lines">@@ -118,14 +118,15 @@
</span><span class="cx"> return m_rareData.get();
</span><span class="cx"> }
</span><span class="cx">
</span><del>- FunctionRareData* rareData() { return m_rareData.get(); }
-
- Structure* allocationStructure()
</del><ins>+ FunctionRareData* rareData()
</ins><span class="cx"> {
</span><del>- if (!m_rareData)
- return nullptr;
</del><ins>+ FunctionRareData* rareData = m_rareData.get();
</ins><span class="cx">
</span><del>- return m_rareData.get()->allocationStructure();
</del><ins>+ // The JS thread may be concurrently creating the rare data
+ // If we see it, we want to ensure it has been properly created
+ WTF::loadLoadFence();
+
+ return rareData;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool isHostOrBuiltinFunction() const;
</span></span></pre>
</div>
</div>
</body>
</html>