<!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>[201584] 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/201584">201584</a></dd>
<dt>Author</dt> <dd>keith_miller@apple.com</dd>
<dt>Date</dt> <dd>2016-06-01 20:18:16 -0700 (Wed, 01 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>canOptimizeStringObjectAccess should use ObjectPropertyConditions rather than structure watchpoints
https://bugs.webkit.org/show_bug.cgi?id=158291
Reviewed by Benjamin Poulain.
The old StringObject primitive access code used structure watchpoints. This meant that
if you set a watchpoint on String.prototype prior to tiering up to the DFG then added
a new property to String.prototype then we would never use StringObject optimizations.
This made property caching in the LLInt bad because it meant we would watchpoint
String.prototype very early in the program, which hurt date-format-xpab.js since that
benchmark relies on the StringObject optimizations.
This patch also extends ObjectPropertyConditionSet to be able to handle a slotBase
equivalence condition. Since that makes the code for generating the DFG watchpoints
significantly cleaner.
* bytecode/ObjectPropertyCondition.cpp:
(JSC::ObjectPropertyCondition::structureEnsuresValidityAssumingImpurePropertyWatchpoint):
* bytecode/ObjectPropertyConditionSet.cpp:
(JSC::ObjectPropertyConditionSet::hasOneSlotBaseCondition):
(JSC::ObjectPropertyConditionSet::slotBaseCondition):
(JSC::generateConditionsForPrototypeEquivalenceConcurrently):
* bytecode/ObjectPropertyConditionSet.h:
* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::isStringPrototypeMethodSane):
(JSC::DFG::Graph::canOptimizeStringObjectAccess):
* dfg/DFGGraph.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeObjectPropertyConditioncpp">trunk/Source/JavaScriptCore/bytecode/ObjectPropertyCondition.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeObjectPropertyConditionSetcpp">trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeObjectPropertyConditionSeth">trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGGraphcpp">trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGGraphh">trunk/Source/JavaScriptCore/dfg/DFGGraph.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (201583 => 201584)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-06-02 01:09:55 UTC (rev 201583)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-06-02 03:18:16 UTC (rev 201584)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2016-06-01 Keith Miller <keith_miller@apple.com>
+
+ canOptimizeStringObjectAccess should use ObjectPropertyConditions rather than structure watchpoints
+ https://bugs.webkit.org/show_bug.cgi?id=158291
+
+ Reviewed by Benjamin Poulain.
+
+ The old StringObject primitive access code used structure watchpoints. This meant that
+ if you set a watchpoint on String.prototype prior to tiering up to the DFG then added
+ a new property to String.prototype then we would never use StringObject optimizations.
+ This made property caching in the LLInt bad because it meant we would watchpoint
+ String.prototype very early in the program, which hurt date-format-xpab.js since that
+ benchmark relies on the StringObject optimizations.
+
+ This patch also extends ObjectPropertyConditionSet to be able to handle a slotBase
+ equivalence condition. Since that makes the code for generating the DFG watchpoints
+ significantly cleaner.
+
+ * bytecode/ObjectPropertyCondition.cpp:
+ (JSC::ObjectPropertyCondition::structureEnsuresValidityAssumingImpurePropertyWatchpoint):
+ * bytecode/ObjectPropertyConditionSet.cpp:
+ (JSC::ObjectPropertyConditionSet::hasOneSlotBaseCondition):
+ (JSC::ObjectPropertyConditionSet::slotBaseCondition):
+ (JSC::generateConditionsForPrototypeEquivalenceConcurrently):
+ * bytecode/ObjectPropertyConditionSet.h:
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::isStringPrototypeMethodSane):
+ (JSC::DFG::Graph::canOptimizeStringObjectAccess):
+ * dfg/DFGGraph.h:
+
</ins><span class="cx"> 2016-06-01 Geoffrey Garen <ggaren@apple.com>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling in r201436.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeObjectPropertyConditioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/ObjectPropertyCondition.cpp (201583 => 201584)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/ObjectPropertyCondition.cpp 2016-06-02 01:09:55 UTC (rev 201583)
+++ trunk/Source/JavaScriptCore/bytecode/ObjectPropertyCondition.cpp 2016-06-02 03:18:16 UTC (rev 201584)
</span><span class="lines">@@ -49,7 +49,7 @@
</span><span class="cx"> bool ObjectPropertyCondition::structureEnsuresValidityAssumingImpurePropertyWatchpoint(
</span><span class="cx"> Structure* structure) const
</span><span class="cx"> {
</span><del>- return m_condition.isStillValidAssumingImpurePropertyWatchpoint(structure);
</del><ins>+ return m_condition.isStillValidAssumingImpurePropertyWatchpoint(structure, m_object);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool ObjectPropertyCondition::structureEnsuresValidityAssumingImpurePropertyWatchpoint() const
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeObjectPropertyConditionSetcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp (201583 => 201584)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp 2016-06-02 01:09:55 UTC (rev 201583)
+++ trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp 2016-06-02 03:18:16 UTC (rev 201584)
</span><span class="lines">@@ -62,7 +62,7 @@
</span><span class="cx">
</span><span class="cx"> bool ObjectPropertyConditionSet::hasOneSlotBaseCondition() const
</span><span class="cx"> {
</span><del>- return numberOfConditionsWithKind(PropertyCondition::Presence) == 1;
</del><ins>+ return (numberOfConditionsWithKind(PropertyCondition::Presence) == 1) != (numberOfConditionsWithKind(PropertyCondition::Equivalence) == 1);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ObjectPropertyCondition ObjectPropertyConditionSet::slotBaseCondition() const
</span><span class="lines">@@ -70,7 +70,8 @@
</span><span class="cx"> ObjectPropertyCondition result;
</span><span class="cx"> unsigned numFound = 0;
</span><span class="cx"> for (const ObjectPropertyCondition& condition : *this) {
</span><del>- if (condition.kind() == PropertyCondition::Presence) {
</del><ins>+ if (condition.kind() == PropertyCondition::Presence
+ || condition.kind() == PropertyCondition::Equivalence) {
</ins><span class="cx"> result = condition;
</span><span class="cx"> numFound++;
</span><span class="cx"> }
</span><span class="lines">@@ -198,6 +199,15 @@
</span><span class="cx"> vm, owner, object, uid, object->structure()->storedPrototypeObject());
</span><span class="cx"> break;
</span><span class="cx"> }
</span><ins>+ case PropertyCondition::Equivalence: {
+ unsigned attributes;
+ PropertyOffset offset = structure->getConcurrently(uid, attributes);
+ if (offset == invalidOffset)
+ return ObjectPropertyCondition();
+ JSValue value = object->getDirect(offset);
+ result = ObjectPropertyCondition::equivalence(vm, owner, object, uid, value);
+ break;
+ }
</ins><span class="cx"> default:
</span><span class="cx"> RELEASE_ASSERT_NOT_REACHED();
</span><span class="cx"> return ObjectPropertyCondition();
</span><span class="lines">@@ -240,7 +250,7 @@
</span><span class="cx"> if (value.isNull()) {
</span><span class="cx"> if (!prototype) {
</span><span class="cx"> if (verbose)
</span><del>- dataLog("Reached end up prototype chain as expected, done.\n");
</del><ins>+ dataLog("Reached end of prototype chain as expected, done.\n");
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx"> if (verbose)
</span><span class="lines">@@ -355,6 +365,21 @@
</span><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ObjectPropertyConditionSet generateConditionsForPrototypeEquivalenceConcurrently(
+ VM& vm, JSGlobalObject* globalObject, Structure* headStructure, JSObject* prototype, UniquedStringImpl* uid)
+{
+ return generateConditions(vm, globalObject, headStructure, prototype,
+ [&] (Vector<ObjectPropertyCondition>& conditions, JSObject* object) -> bool {
+ PropertyCondition::Kind kind =
+ object == prototype ? PropertyCondition::Equivalence : PropertyCondition::Absence;
+ ObjectPropertyCondition result = generateCondition(vm, nullptr, object, uid, kind);
+ if (!result)
+ return false;
+ conditions.append(result);
+ return true;
+ }, Concurrent);
+}
+
</ins><span class="cx"> ObjectPropertyConditionSet generateConditionsForPropertyMissConcurrently(
</span><span class="cx"> VM& vm, JSGlobalObject* globalObject, Structure* headStructure, UniquedStringImpl* uid)
</span><span class="cx"> {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeObjectPropertyConditionSeth"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.h (201583 => 201584)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.h 2016-06-02 01:09:55 UTC (rev 201583)
+++ trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.h 2016-06-02 03:18:16 UTC (rev 201584)
</span><span class="lines">@@ -166,7 +166,9 @@
</span><span class="cx"> VM&, JSCell* owner, ExecState*, Structure* headStructure, JSObject* prototype,
</span><span class="cx"> UniquedStringImpl* uid);
</span><span class="cx">
</span><del>-
</del><ins>+ObjectPropertyConditionSet generateConditionsForPrototypeEquivalenceConcurrently(
+ VM&, JSGlobalObject*, Structure* headStructure, JSObject* prototype,
+ UniquedStringImpl* uid);
</ins><span class="cx"> ObjectPropertyConditionSet generateConditionsForPropertyMissConcurrently(
</span><span class="cx"> VM&, JSGlobalObject*, Structure* headStructure, UniquedStringImpl* uid);
</span><span class="cx"> ObjectPropertyConditionSet generateConditionsForPropertySetterMissConcurrently(
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGGraphcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp (201583 => 201584)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp 2016-06-02 01:09:55 UTC (rev 201583)
+++ trunk/Source/JavaScriptCore/dfg/DFGGraph.cpp 2016-06-02 03:18:16 UTC (rev 201584)
</span><span class="lines">@@ -1538,27 +1538,6 @@
</span><span class="cx"> return MethodOfGettingAValueProfile();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool Graph::isStringPrototypeMethodSane(JSObject* stringPrototype, Structure* stringPrototypeStructure, UniquedStringImpl* uid)
-{
- unsigned attributesUnused;
- PropertyOffset offset = stringPrototypeStructure->getConcurrently(uid, attributesUnused);
- if (!isValidOffset(offset))
- return false;
-
- JSValue value = tryGetConstantProperty(stringPrototype, stringPrototypeStructure, offset);
- if (!value)
- return false;
-
- JSFunction* function = jsDynamicCast<JSFunction*>(value);
- if (!function)
- return false;
-
- if (function->executable()->intrinsicFor(CodeForCall) != StringPrototypeValueOfIntrinsic)
- return false;
-
- return true;
-}
-
</del><span class="cx"> bool Graph::getRegExpPrototypeProperty(JSObject* regExpPrototype, Structure* regExpPrototypeStructure, UniquedStringImpl* uid, JSValue& returnJSValue)
</span><span class="cx"> {
</span><span class="cx"> unsigned attributesUnused;
</span><span class="lines">@@ -1587,39 +1566,48 @@
</span><span class="cx"> return true;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+bool Graph::isStringPrototypeMethodSane(JSGlobalObject* globalObject, UniquedStringImpl* uid)
+{
+ ObjectPropertyConditionSet conditions = generateConditionsForPrototypeEquivalenceConcurrently(m_vm, globalObject, globalObject->stringObjectStructure(), globalObject->stringPrototype(), uid);
+
+ if (!conditions.isValid())
+ return false;
+
+ ObjectPropertyCondition equivalenceCondition = conditions.slotBaseCondition();
+ RELEASE_ASSERT(equivalenceCondition.hasRequiredValue());
+ JSFunction* function = jsDynamicCast<JSFunction*>(equivalenceCondition.condition().requiredValue());
+ if (!function)
+ return false;
+
+ if (function->executable()->intrinsicFor(CodeForCall) != StringPrototypeValueOfIntrinsic)
+ return false;
+
+ return watchConditions(conditions);
+}
+
+
</ins><span class="cx"> bool Graph::canOptimizeStringObjectAccess(const CodeOrigin& codeOrigin)
</span><span class="cx"> {
</span><span class="cx"> if (hasExitSite(codeOrigin, NotStringObject))
</span><span class="cx"> return false;
</span><span class="cx">
</span><ins>+ JSGlobalObject* globalObject = globalObjectFor(codeOrigin);
</ins><span class="cx"> Structure* stringObjectStructure = globalObjectFor(codeOrigin)->stringObjectStructure();
</span><span class="cx"> registerStructure(stringObjectStructure);
</span><span class="cx"> ASSERT(stringObjectStructure->storedPrototype().isObject());
</span><span class="cx"> ASSERT(stringObjectStructure->storedPrototype().asCell()->classInfo() == StringPrototype::info());
</span><span class="cx">
</span><del>- FrozenValue* stringPrototypeObjectValue = freeze(stringObjectStructure->storedPrototype());
- StringPrototype* stringPrototypeObject = stringPrototypeObjectValue->dynamicCast<StringPrototype*>();
- Structure* stringPrototypeStructure = stringPrototypeObjectValue->structure();
- if (registerStructure(stringPrototypeStructure) != StructureRegisteredAndWatched)
</del><ins>+ if (!watchConditions(generateConditionsForPropertyMissConcurrently(m_vm, globalObject, stringObjectStructure, m_vm.propertyNames->toPrimitiveSymbol.impl())))
</ins><span class="cx"> return false;
</span><span class="cx">
</span><del>- if (stringPrototypeStructure->isDictionary())
- return false;
-
- if (!watchConditions(generateConditionsForPropertyMissConcurrently(m_vm, globalObjectFor(codeOrigin), stringObjectStructure, m_vm.propertyNames->toPrimitiveSymbol.impl())))
- return false;
-
</del><span class="cx"> // We're being conservative here. We want DFG's ToString on StringObject to be
</span><span class="cx"> // used in both numeric contexts (that would call valueOf()) and string contexts
</span><span class="cx"> // (that would call toString()). We don't want the DFG to have to distinguish
</span><span class="cx"> // between the two, just because that seems like it would get confusing. So we
</span><span class="cx"> // just require both methods to be sane.
</span><del>- if (!isStringPrototypeMethodSane(stringPrototypeObject, stringPrototypeStructure, m_vm.propertyNames->valueOf.impl()))
</del><ins>+ if (!isStringPrototypeMethodSane(globalObject, m_vm.propertyNames->valueOf.impl()))
</ins><span class="cx"> return false;
</span><del>- if (!isStringPrototypeMethodSane(stringPrototypeObject, stringPrototypeStructure, m_vm.propertyNames->toString.impl()))
- return false;
-
- return true;
</del><ins>+ return isStringPrototypeMethodSane(globalObject, m_vm.propertyNames->toString.impl());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool Graph::willCatchExceptionInMachineFrame(CodeOrigin codeOrigin, CodeOrigin& opCatchOriginOut, HandlerInfo*& catchHandlerOut)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGGraphh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGGraph.h (201583 => 201584)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGGraph.h 2016-06-02 01:09:55 UTC (rev 201583)
+++ trunk/Source/JavaScriptCore/dfg/DFGGraph.h 2016-06-02 03:18:16 UTC (rev 201584)
</span><span class="lines">@@ -910,7 +910,7 @@
</span><span class="cx"> bool m_hasExceptionHandlers { false };
</span><span class="cx"> private:
</span><span class="cx">
</span><del>- bool isStringPrototypeMethodSane(JSObject* stringPrototype, Structure* stringPrototypeStructure, UniquedStringImpl*);
</del><ins>+ bool isStringPrototypeMethodSane(JSGlobalObject*, UniquedStringImpl*);
</ins><span class="cx">
</span><span class="cx"> void handleSuccessor(Vector<BasicBlock*, 16>& worklist, BasicBlock*, BasicBlock* successor);
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>