<!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>[210745] 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/210745">210745</a></dd>
<dt>Author</dt> <dd>sbarati@apple.com</dd>
<dt>Date</dt> <dd>2017-01-13 15:30:45 -0800 (Fri, 13 Jan 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Initialize the ArraySpecies watchpoint as Clear and transition to IsWatched once slice is called for the first time
https://bugs.webkit.org/show_bug.cgi?id=167017
<rdar://problem/30019309>
Reviewed by Keith Miller and Filip Pizlo.
This patch is to reverse the JSBench regression from <a href="http://trac.webkit.org/projects/webkit/changeset/210695">r210695</a>.
The new state diagram for the array species watchpoint is as
follows:
1. On GlobalObject construction, it starts life out as ClearWatchpoint.
2. When slice is called for the first time, we observe the state
of the world, and either transition it to IsWatched if we were able
to set up the object property conditions, or to IsInvalidated if we
were not.
3. The DFG compiler will now only lower slice as an intrinsic if
it observed the speciesWatchpoint.state() as IsWatched.
4. The IsWatched => IsInvalidated transition happens only when
one of the object property condition watchpoints fire.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleIntrinsicCall):
* runtime/ArrayPrototype.cpp:
(JSC::speciesWatchpointIsValid):
(JSC::speciesConstructArray):
(JSC::arrayProtoPrivateFuncConcatMemcpy):
(JSC::ArrayPrototype::tryInitializeSpeciesWatchpoint):
(JSC::ArrayPrototype::initializeSpeciesWatchpoint): Deleted.
* runtime/ArrayPrototype.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::JSGlobalObject):
(JSC::JSGlobalObject::init):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp">trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeArrayPrototypecpp">trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeArrayPrototypeh">trunk/Source/JavaScriptCore/runtime/ArrayPrototype.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSGlobalObjectcpp">trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (210744 => 210745)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2017-01-13 23:02:58 UTC (rev 210744)
+++ trunk/Source/JavaScriptCore/ChangeLog 2017-01-13 23:30:45 UTC (rev 210745)
</span><span class="lines">@@ -1,3 +1,39 @@
</span><ins>+2017-01-13 Saam Barati <sbarati@apple.com>
+
+ Initialize the ArraySpecies watchpoint as Clear and transition to IsWatched once slice is called for the first time
+ https://bugs.webkit.org/show_bug.cgi?id=167017
+ <rdar://problem/30019309>
+
+ Reviewed by Keith Miller and Filip Pizlo.
+
+ This patch is to reverse the JSBench regression from r210695.
+
+ The new state diagram for the array species watchpoint is as
+ follows:
+
+ 1. On GlobalObject construction, it starts life out as ClearWatchpoint.
+ 2. When slice is called for the first time, we observe the state
+ of the world, and either transition it to IsWatched if we were able
+ to set up the object property conditions, or to IsInvalidated if we
+ were not.
+ 3. The DFG compiler will now only lower slice as an intrinsic if
+ it observed the speciesWatchpoint.state() as IsWatched.
+ 4. The IsWatched => IsInvalidated transition happens only when
+ one of the object property condition watchpoints fire.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::handleIntrinsicCall):
+ * runtime/ArrayPrototype.cpp:
+ (JSC::speciesWatchpointIsValid):
+ (JSC::speciesConstructArray):
+ (JSC::arrayProtoPrivateFuncConcatMemcpy):
+ (JSC::ArrayPrototype::tryInitializeSpeciesWatchpoint):
+ (JSC::ArrayPrototype::initializeSpeciesWatchpoint): Deleted.
+ * runtime/ArrayPrototype.h:
+ * runtime/JSGlobalObject.cpp:
+ (JSC::JSGlobalObject::JSGlobalObject):
+ (JSC::JSGlobalObject::init):
+
</ins><span class="cx"> 2017-01-13 Yusuke Suzuki <utatane.tea@gmail.com>
</span><span class="cx">
</span><span class="cx"> Reserve capacity for StringBuilder in unescape
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (210744 => 210745)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp 2017-01-13 23:02:58 UTC (rev 210744)
+++ trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp 2017-01-13 23:30:45 UTC (rev 210745)
</span><span class="lines">@@ -2278,7 +2278,7 @@
</span><span class="cx"> InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
</span><span class="cx">
</span><span class="cx"> // FIXME: We could easily relax the Array/Object.prototype transition as long as we OSR exitted if we saw a hole.
</span><del>- if (globalObject->arraySpeciesWatchpoint().isStillValid()
</del><ins>+ if (globalObject->arraySpeciesWatchpoint().state() == IsWatched
</ins><span class="cx"> && globalObject->havingABadTimeWatchpoint()->isStillValid()
</span><span class="cx"> && arrayPrototypeTransition.isStillValid()
</span><span class="cx"> && objectPrototypeTransition.isStillValid()
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeArrayPrototypecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp (210744 => 210745)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp 2017-01-13 23:02:58 UTC (rev 210744)
+++ trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp 2017-01-13 23:30:45 UTC (rev 210745)
</span><span class="lines">@@ -191,12 +191,19 @@
</span><span class="cx"> throwTypeError(exec, scope, ASCIILiteral(ReadonlyPropertyWriteError));
</span><span class="cx"> }
</span><span class="cx">
</span><del>-inline bool speciesWatchpointIsValid(JSObject* thisObject)
</del><ins>+ALWAYS_INLINE bool speciesWatchpointIsValid(ExecState* exec, JSObject* thisObject)
</ins><span class="cx"> {
</span><del>- ArrayPrototype* arrayPrototype = thisObject->globalObject()->arrayPrototype();
</del><ins>+ JSGlobalObject* globalObject = thisObject->globalObject();
+ ArrayPrototype* arrayPrototype = globalObject->arrayPrototype();
+
+ if (globalObject->arraySpeciesWatchpoint().stateOnJSThread() == ClearWatchpoint) {
+ arrayPrototype->tryInitializeSpeciesWatchpoint(exec);
+ ASSERT(globalObject->arraySpeciesWatchpoint().stateOnJSThread() != ClearWatchpoint);
+ }
+
</ins><span class="cx"> return !thisObject->hasCustomProperties()
</span><span class="cx"> && arrayPrototype == thisObject->getPrototypeDirect()
</span><del>- && arrayPrototype->globalObject()->arraySpeciesWatchpoint().isStillValid();
</del><ins>+ && globalObject->arraySpeciesWatchpoint().stateOnJSThread() == IsWatched;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> enum class SpeciesConstructResult {
</span><span class="lines">@@ -221,7 +228,7 @@
</span><span class="cx"> if (LIKELY(thisIsArray)) {
</span><span class="cx"> // Fast path in the normal case where the user has not set an own constructor and the Array.prototype.constructor is normal.
</span><span class="cx"> // We need prototype check for subclasses of Array, which are Array objects but have a different prototype by default.
</span><del>- bool isValid = speciesWatchpointIsValid(thisObject);
</del><ins>+ bool isValid = speciesWatchpointIsValid(exec, thisObject);
</ins><span class="cx"> if (LIKELY(isValid))
</span><span class="cx"> return std::make_pair(SpeciesConstructResult::FastPath, nullptr);
</span><span class="cx">
</span><span class="lines">@@ -1239,7 +1246,7 @@
</span><span class="cx"> return JSValue::encode(jsNull());
</span><span class="cx">
</span><span class="cx"> // We need to check the species constructor here since checking it in the JS wrapper is too expensive for the non-optimizing tiers.
</span><del>- bool isValid = speciesWatchpointIsValid(firstArray);
</del><ins>+ bool isValid = speciesWatchpointIsValid(exec, firstArray);
</ins><span class="cx"> if (UNLIKELY(!isValid))
</span><span class="cx"> return JSValue::encode(jsNull());
</span><span class="cx">
</span><span class="lines">@@ -1331,7 +1338,7 @@
</span><span class="cx"> ArrayPrototype* m_arrayPrototype;
</span><span class="cx"> };
</span><span class="cx">
</span><del>-void ArrayPrototype::initializeSpeciesWatchpoint(ExecState* exec)
</del><ins>+void ArrayPrototype::tryInitializeSpeciesWatchpoint(ExecState* exec)
</ins><span class="cx"> {
</span><span class="cx"> VM& vm = exec->vm();
</span><span class="cx">
</span><span class="lines">@@ -1339,7 +1346,6 @@
</span><span class="cx"> RELEASE_ASSERT(!m_constructorSpeciesWatchpoint);
</span><span class="cx">
</span><span class="cx"> auto scope = DECLARE_THROW_SCOPE(vm);
</span><del>- UNUSED_PARAM(scope);
</del><span class="cx">
</span><span class="cx"> if (verbose)
</span><span class="cx"> dataLog("Initializing Array species watchpoints for Array.prototype: ", pointerDump(this), " with structure: ", pointerDump(this->structure()), "\nand Array: ", pointerDump(this->globalObject()->arrayConstructor()), " with structure: ", pointerDump(this->globalObject()->arrayConstructor()->structure()), "\n");
</span><span class="lines">@@ -1355,12 +1361,19 @@
</span><span class="cx"> JSGlobalObject* globalObject = this->globalObject();
</span><span class="cx"> ArrayConstructor* arrayConstructor = globalObject->arrayConstructor();
</span><span class="cx">
</span><ins>+ auto invalidateWatchpoint = [&] {
+ globalObject->arraySpeciesWatchpoint().invalidate(vm, StringFireDetail("Was not able to set up array species watchpoint."));
+ };
+
</ins><span class="cx"> PropertySlot constructorSlot(this, PropertySlot::InternalMethodType::VMInquiry);
</span><span class="cx"> this->getOwnPropertySlot(this, exec, vm.propertyNames->constructor, constructorSlot);
</span><del>- ASSERT(!scope.exception());
- ASSERT(constructorSlot.slotBase() == this);
- ASSERT(constructorSlot.isCacheableValue());
- RELEASE_ASSERT(constructorSlot.getValue(exec, vm.propertyNames->constructor) == arrayConstructor);
</del><ins>+ ASSERT_UNUSED(scope, !scope.exception());
+ if (constructorSlot.slotBase() != this
+ || !constructorSlot.isCacheableValue()
+ || constructorSlot.getValue(exec, vm.propertyNames->constructor) != arrayConstructor) {
+ invalidateWatchpoint();
+ return;
+ }
</ins><span class="cx">
</span><span class="cx"> Structure* constructorStructure = arrayConstructor->structure(vm);
</span><span class="cx"> if (constructorStructure->isDictionary())
</span><span class="lines">@@ -1368,10 +1381,13 @@
</span><span class="cx">
</span><span class="cx"> PropertySlot speciesSlot(arrayConstructor, PropertySlot::InternalMethodType::VMInquiry);
</span><span class="cx"> arrayConstructor->getOwnPropertySlot(arrayConstructor, exec, vm.propertyNames->speciesSymbol, speciesSlot);
</span><del>- ASSERT(!scope.exception());
- ASSERT(speciesSlot.slotBase() == arrayConstructor);
- ASSERT(speciesSlot.isCacheableGetter());
- RELEASE_ASSERT(speciesSlot.getterSetter() == globalObject->speciesGetterSetter());
</del><ins>+ ASSERT_UNUSED(scope, !scope.exception());
+ if (speciesSlot.slotBase() != arrayConstructor
+ || !speciesSlot.isCacheableGetter()
+ || speciesSlot.getterSetter() != globalObject->speciesGetterSetter()) {
+ invalidateWatchpoint();
+ return;
+ }
</ins><span class="cx">
</span><span class="cx"> // Now we need to setup the watchpoints to make sure these conditions remain valid.
</span><span class="cx"> prototypeStructure->startWatchingPropertyForReplacements(vm, constructorSlot.cachedOffset());
</span><span class="lines">@@ -1380,8 +1396,10 @@
</span><span class="cx"> ObjectPropertyCondition constructorCondition = ObjectPropertyCondition::equivalence(vm, this, this, vm.propertyNames->constructor.impl(), arrayConstructor);
</span><span class="cx"> ObjectPropertyCondition speciesCondition = ObjectPropertyCondition::equivalence(vm, this, arrayConstructor, vm.propertyNames->speciesSymbol.impl(), globalObject->speciesGetterSetter());
</span><span class="cx">
</span><del>- RELEASE_ASSERT(constructorCondition.isWatchable());
- RELEASE_ASSERT(speciesCondition.isWatchable());
</del><ins>+ if (!constructorCondition.isWatchable() || !speciesCondition.isWatchable()) {
+ invalidateWatchpoint();
+ return;
+ }
</ins><span class="cx">
</span><span class="cx"> m_constructorWatchpoint = std::make_unique<ArrayPrototypeAdaptiveInferredPropertyWatchpoint>(constructorCondition, this);
</span><span class="cx"> m_constructorWatchpoint->install();
</span><span class="lines">@@ -1388,6 +1406,10 @@
</span><span class="cx">
</span><span class="cx"> m_constructorSpeciesWatchpoint = std::make_unique<ArrayPrototypeAdaptiveInferredPropertyWatchpoint>(speciesCondition, this);
</span><span class="cx"> m_constructorSpeciesWatchpoint->install();
</span><ins>+
+ // We only watch this from the DFG, and the DFG makes sure to only start watching if the watchpoint is in the IsWatched state.
+ RELEASE_ASSERT(!globalObject->arraySpeciesWatchpoint().isBeingWatched());
+ globalObject->arraySpeciesWatchpoint().touch(vm, "Set up array species watchpoint.");
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ArrayPrototypeAdaptiveInferredPropertyWatchpoint::ArrayPrototypeAdaptiveInferredPropertyWatchpoint(const ObjectPropertyCondition& key, ArrayPrototype* prototype)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeArrayPrototypeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/ArrayPrototype.h (210744 => 210745)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/ArrayPrototype.h 2017-01-13 23:02:58 UTC (rev 210744)
+++ trunk/Source/JavaScriptCore/runtime/ArrayPrototype.h 2017-01-13 23:30:45 UTC (rev 210745)
</span><span class="lines">@@ -49,7 +49,7 @@
</span><span class="cx"> return Structure::create(vm, globalObject, prototype, TypeInfo(DerivedArrayType, StructureFlags), info(), ArrayClass);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- void initializeSpeciesWatchpoint(ExecState*);
</del><ins>+ void tryInitializeSpeciesWatchpoint(ExecState*);
</ins><span class="cx">
</span><span class="cx"> static const bool needsDestruction = false;
</span><span class="cx"> // We don't need destruction since we use a finalizer.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSGlobalObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp (210744 => 210745)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp 2017-01-13 23:02:58 UTC (rev 210744)
+++ trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp 2017-01-13 23:30:45 UTC (rev 210745)
</span><span class="lines">@@ -332,7 +332,7 @@
</span><span class="cx"> , m_varInjectionWatchpoint(adoptRef(new WatchpointSet(IsWatched)))
</span><span class="cx"> , m_weakRandom(Options::forceWeakRandomSeed() ? Options::forcedWeakRandomSeed() : static_cast<unsigned>(randomNumber() * (std::numeric_limits<unsigned>::max() + 1.0)))
</span><span class="cx"> , m_arrayIteratorProtocolWatchpoint(IsWatched)
</span><del>- , m_arraySpeciesWatchpoint(IsWatched)
</del><ins>+ , m_arraySpeciesWatchpoint(ClearWatchpoint)
</ins><span class="cx"> , m_templateRegistry(vm)
</span><span class="cx"> , m_evalEnabled(true)
</span><span class="cx"> , m_runtimeFlags()
</span><span class="lines">@@ -947,8 +947,6 @@
</span><span class="cx"> m_arrayPrototypeSymbolIteratorWatchpoint = std::make_unique<ArrayIteratorAdaptiveWatchpoint>(condition, this);
</span><span class="cx"> m_arrayPrototypeSymbolIteratorWatchpoint->install();
</span><span class="cx"> }
</span><del>-
- this->arrayPrototype()->initializeSpeciesWatchpoint(exec);
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> resetPrototype(vm, getPrototypeDirect());
</span></span></pre>
</div>
</div>
</body>
</html>