<!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>[201436] 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/201436">201436</a></dd>
<dt>Author</dt> <dd>ggaren@apple.com</dd>
<dt>Date</dt> <dd>2016-05-26 15:30:05 -0700 (Thu, 26 May 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>REGRESSION: JSBench spends a lot of time transitioning to/from dictionary
https://bugs.webkit.org/show_bug.cgi?id=158045

Reviewed by Saam Barati.

15% speedup on jsbench-amazon-firefox, possibly 5% speedup overall on jsbench.

This regression seems to have two parts:

(1) Transitioning the window object to/from dictionary is more expensive
than it used to be to because the window object has lots more properties.
The window object has more properties because, for WebIDL compatibility,
we reify DOM APIs as properties when you delete.

(2) DOM prototypes transition to/from dictionary upon creation
because, once again for WebIDL compatibility, we reify their static
APIs eagerly.

The solution is to chill out a bit on dictionary transitions.

* bytecode/ObjectPropertyConditionSet.cpp: Don't flatten a dictionary
if we've already done so before. This avoids pathological churn, and it
is our idiom in other places.

* interpreter/Interpreter.cpp:
(JSC::Interpreter::execute): Do flatten the global object unconditionally
if it is an uncacheable dictionary because the global object is super
important.

* runtime/BatchedTransitionOptimizer.h:
(JSC::BatchedTransitionOptimizer::BatchedTransitionOptimizer):
(JSC::BatchedTransitionOptimizer::~BatchedTransitionOptimizer): Deleted.
Don't transition away from dictionary after a batched set of property
puts because normal dictionaries are cacheable and that's a perfectly
fine state to be in -- and the transition is expensive.

* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init): Do start the global object out as a cacheable
dictionary because it will inevitably have enough properties to become
a dictionary.

* runtime/Operations.h:
(JSC::normalizePrototypeChain): Same as ObjectPropertyConditionSet.cpp.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeObjectPropertyConditionSetcpp">trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreinterpreterInterpretercpp">trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeBatchedTransitionOptimizerh">trunk/Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSGlobalObjectcpp">trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeOperationscpp">trunk/Source/JavaScriptCore/runtime/Operations.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeOperationsh">trunk/Source/JavaScriptCore/runtime/Operations.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (201435 => 201436)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2016-05-26 22:05:26 UTC (rev 201435)
+++ trunk/Source/JavaScriptCore/ChangeLog        2016-05-26 22:30:05 UTC (rev 201436)
</span><span class="lines">@@ -1,3 +1,49 @@
</span><ins>+2016-05-26  Geoffrey Garen  &lt;ggaren@apple.com&gt;
+
+        REGRESSION: JSBench spends a lot of time transitioning to/from dictionary
+        https://bugs.webkit.org/show_bug.cgi?id=158045
+
+        Reviewed by Saam Barati.
+
+        15% speedup on jsbench-amazon-firefox, possibly 5% speedup overall on jsbench.
+
+        This regression seems to have two parts:
+
+        (1) Transitioning the window object to/from dictionary is more expensive
+        than it used to be to because the window object has lots more properties.
+        The window object has more properties because, for WebIDL compatibility,
+        we reify DOM APIs as properties when you delete.
+
+        (2) DOM prototypes transition to/from dictionary upon creation
+        because, once again for WebIDL compatibility, we reify their static
+        APIs eagerly.
+
+        The solution is to chill out a bit on dictionary transitions.
+
+        * bytecode/ObjectPropertyConditionSet.cpp: Don't flatten a dictionary
+        if we've already done so before. This avoids pathological churn, and it
+        is our idiom in other places.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::execute): Do flatten the global object unconditionally
+        if it is an uncacheable dictionary because the global object is super
+        important.
+
+        * runtime/BatchedTransitionOptimizer.h:
+        (JSC::BatchedTransitionOptimizer::BatchedTransitionOptimizer):
+        (JSC::BatchedTransitionOptimizer::~BatchedTransitionOptimizer): Deleted.
+        Don't transition away from dictionary after a batched set of property
+        puts because normal dictionaries are cacheable and that's a perfectly
+        fine state to be in -- and the transition is expensive.
+
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init): Do start the global object out as a cacheable
+        dictionary because it will inevitably have enough properties to become
+        a dictionary.
+
+        * runtime/Operations.h:
+        (JSC::normalizePrototypeChain): Same as ObjectPropertyConditionSet.cpp.
+
</ins><span class="cx"> 2016-05-25  Geoffrey Garen  &lt;ggaren@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         replaceable own properties seem to ignore replacement after property caching
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeObjectPropertyConditionSetcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp (201435 => 201436)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp        2016-05-26 22:05:26 UTC (rev 201435)
+++ trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp        2016-05-26 22:30:05 UTC (rev 201436)
</span><span class="lines">@@ -263,10 +263,13 @@
</span><span class="cx">         JSObject* object = jsCast&lt;JSObject*&gt;(value);
</span><span class="cx">         structure = object-&gt;structure(vm);
</span><span class="cx">         
</span><del>-        // Since we're accessing a prototype repeatedly, it's a good bet that it should not be
-        // treated as a dictionary.
</del><span class="cx">         if (structure-&gt;isDictionary()) {
</span><span class="cx">             if (concurrency == MainThread) {
</span><ins>+                if (structure-&gt;hasBeenFlattenedBefore()) {
+                    if (verbose)
+                        dataLog(&quot;Dictionary has been flattened before, so invalid.\n&quot;);
+                    return ObjectPropertyConditionSet::invalid();
+                }
</ins><span class="cx">                 if (verbose)
</span><span class="cx">                     dataLog(&quot;Flattening &quot;, pointerDump(structure));
</span><span class="cx">                 structure-&gt;flattenDictionaryStructure(vm, object);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreinterpreterInterpretercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp (201435 => 201436)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp        2016-05-26 22:05:26 UTC (rev 201435)
+++ trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp        2016-05-26 22:30:05 UTC (rev 201436)
</span><span class="lines">@@ -938,6 +938,9 @@
</span><span class="cx">     if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
</span><span class="cx">         return throwTerminatedExecutionException(callFrame);
</span><span class="cx"> 
</span><ins>+    if (scope-&gt;structure()-&gt;isUncacheableDictionary())
+        scope-&gt;flattenDictionaryObject(vm);
+
</ins><span class="cx">     ASSERT(codeBlock-&gt;numParameters() == 1); // 1 parameter for 'this'.
</span><span class="cx"> 
</span><span class="cx">     ProtoCallFrame protoCallFrame;
</span><span class="lines">@@ -1186,6 +1189,9 @@
</span><span class="cx">         }
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    if (variableObject-&gt;structure()-&gt;isUncacheableDictionary())
+        variableObject-&gt;flattenDictionaryObject(vm);
+
</ins><span class="cx">     if (numVariables || numFunctions) {
</span><span class="cx">         BatchedTransitionOptimizer optimizer(vm, variableObject);
</span><span class="cx">         if (variableObject-&gt;next())
</span><span class="lines">@@ -1243,6 +1249,9 @@
</span><span class="cx">     if (UNLIKELY(vm.shouldTriggerTermination(callFrame)))
</span><span class="cx">         return throwTerminatedExecutionException(callFrame);
</span><span class="cx"> 
</span><ins>+    if (scope-&gt;structure()-&gt;isUncacheableDictionary())
+        scope-&gt;flattenDictionaryObject(vm);
+
</ins><span class="cx">     ASSERT(codeBlock-&gt;numParameters() == 1); // 1 parameter for 'this'.
</span><span class="cx"> 
</span><span class="cx">     // The |this| of the module is always `undefined`.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeBatchedTransitionOptimizerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h (201435 => 201436)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h        2016-05-26 22:05:26 UTC (rev 201435)
+++ trunk/Source/JavaScriptCore/runtime/BatchedTransitionOptimizer.h        2016-05-26 22:30:05 UTC (rev 201436)
</span><span class="lines">@@ -35,22 +35,10 @@
</span><span class="cx">     WTF_MAKE_NONCOPYABLE(BatchedTransitionOptimizer);
</span><span class="cx"> public:
</span><span class="cx">     BatchedTransitionOptimizer(VM&amp; vm, JSObject* object)
</span><del>-        : m_vm(&amp;vm)
-        , m_object(object)
</del><span class="cx">     {
</span><del>-        if (!m_object-&gt;structure(vm)-&gt;isDictionary())
-            m_object-&gt;convertToDictionary(vm);
</del><ins>+        if (!object-&gt;structure(vm)-&gt;isDictionary())
+            object-&gt;convertToDictionary(vm);
</ins><span class="cx">     }
</span><del>-
-    ~BatchedTransitionOptimizer()
-    {
-        if (m_object-&gt;structure()-&gt;isDictionary())
-            m_object-&gt;flattenDictionaryObject(*m_vm);
-    }
-
-private:
-    VM* m_vm;
-    JSObject* m_object;
</del><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSGlobalObjectcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp (201435 => 201436)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp        2016-05-26 22:05:26 UTC (rev 201435)
+++ trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp        2016-05-26 22:30:05 UTC (rev 201436)
</span><span class="lines">@@ -319,6 +319,8 @@
</span><span class="cx"> {
</span><span class="cx">     ASSERT(vm.currentThreadIsHoldingAPILock());
</span><span class="cx"> 
</span><ins>+    Base::setStructure(vm, Structure::toCacheableDictionaryTransition(vm, structure()));
+
</ins><span class="cx">     JSGlobalObject::globalExec()-&gt;init(0, 0, CallFrame::noCaller(), 0, 0);
</span><span class="cx"> 
</span><span class="cx">     m_debugger = 0;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeOperationscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Operations.cpp (201435 => 201436)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Operations.cpp        2016-05-26 22:05:26 UTC (rev 201435)
+++ trunk/Source/JavaScriptCore/runtime/Operations.cpp        2016-05-26 22:30:05 UTC (rev 201436)
</span><span class="lines">@@ -120,4 +120,27 @@
</span><span class="cx">     return false;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+size_t normalizePrototypeChain(CallFrame* callFrame, Structure* structure)
+{
+    VM&amp; vm = callFrame-&gt;vm();
+    size_t count = 0;
+    while (1) {
+        if (structure-&gt;isProxy())
+            return InvalidPrototypeChain;
+        JSValue v = structure-&gt;prototypeForLookup(callFrame);
+        if (v.isNull())
+            return count;
+
+        JSCell* base = v.asCell();
+        structure = base-&gt;structure(vm);
+        if (structure-&gt;isDictionary()) {
+            if (structure-&gt;hasBeenFlattenedBefore())
+                return InvalidPrototypeChain;
+            structure-&gt;flattenDictionaryStructure(vm, asObject(base));
+        }
+
+        ++count;
+    }
+}
+
</ins><span class="cx"> } // namespace JSC
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeOperationsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Operations.h (201435 => 201436)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Operations.h        2016-05-26 22:05:26 UTC (rev 201435)
+++ trunk/Source/JavaScriptCore/runtime/Operations.h        2016-05-26 22:30:05 UTC (rev 201436)
</span><span class="lines">@@ -28,11 +28,14 @@
</span><span class="cx"> 
</span><span class="cx"> namespace JSC {
</span><span class="cx"> 
</span><ins>+#define InvalidPrototypeChain (std::numeric_limits&lt;size_t&gt;::max())
+
</ins><span class="cx"> NEVER_INLINE JSValue jsAddSlowCase(CallFrame*, JSValue, JSValue);
</span><span class="cx"> JSValue jsTypeStringForValue(CallFrame*, JSValue);
</span><span class="cx"> JSValue jsTypeStringForValue(VM&amp;, JSGlobalObject*, JSValue);
</span><span class="cx"> bool jsIsObjectTypeOrNull(CallFrame*, JSValue);
</span><span class="cx"> bool jsIsFunctionType(JSValue);
</span><ins>+size_t normalizePrototypeChain(CallFrame*, Structure*);
</ins><span class="cx"> 
</span><span class="cx"> ALWAYS_INLINE JSValue jsString(ExecState* exec, JSString* s1, JSString* s2)
</span><span class="cx"> {
</span><span class="lines">@@ -192,30 +195,6 @@
</span><span class="cx">     return jsAddSlowCase(callFrame, v1, v2);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-#define InvalidPrototypeChain (std::numeric_limits&lt;size_t&gt;::max())
-
-inline size_t normalizePrototypeChain(CallFrame* callFrame, Structure* structure)
-{
-    VM&amp; vm = callFrame-&gt;vm();
-    size_t count = 0;
-    while (1) {
-        if (structure-&gt;isProxy())
-            return InvalidPrototypeChain;
-        JSValue v = structure-&gt;prototypeForLookup(callFrame);
-        if (v.isNull())
-            return count;
-
-        JSCell* base = v.asCell();
-        structure = base-&gt;structure(vm);
-        // Since we're accessing a prototype in a loop, it's a good bet that it
-        // should not be treated as a dictionary.
-        if (structure-&gt;isDictionary())
-            structure-&gt;flattenDictionaryStructure(vm, asObject(base));
-
-        ++count;
-    }
-}
-
</del><span class="cx"> } // namespace JSC
</span><span class="cx"> 
</span><span class="cx"> #endif // Operations_h
</span></span></pre>
</div>
</div>

</body>
</html>