<!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>[196644] trunk</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/196644">196644</a></dd>
<dt>Author</dt> <dd>keith_miller@apple.com</dd>
<dt>Date</dt> <dd>2016-02-16 11:28:09 -0800 (Tue, 16 Feb 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>ClonedArguments should not materialize its special properties unless they are being changed or deleted
https://bugs.webkit.org/show_bug.cgi?id=154128

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

Before we would materialize ClonedArguments whenever they were being accessed.
However this would cause the IC to miss every time as the structure for
the arguments object would change as we went to IC it. Thus on the next
function call we would miss the cache since the new arguments object
would not have materialized the value.

* runtime/ClonedArguments.cpp:
(JSC::ClonedArguments::getOwnPropertySlot):
* tests/stress/cloned-arguments-modification.js: Added.
(foo):

LayoutTests:

Have argumnets-strict-mode test the speed of spreading the arguments object.

* js/regress/script-tests/arguments-strict-mode.js:
(foo):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsjsregressscripttestsargumentsstrictmodejs">trunk/LayoutTests/js/regress/script-tests/arguments-strict-mode.js</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeClonedArgumentscpp">trunk/Source/JavaScriptCore/runtime/ClonedArguments.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstressclonedargumentsmodificationjs">trunk/Source/JavaScriptCore/tests/stress/cloned-arguments-modification.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (196643 => 196644)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2016-02-16 19:13:17 UTC (rev 196643)
+++ trunk/LayoutTests/ChangeLog        2016-02-16 19:28:09 UTC (rev 196644)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2016-02-16  Keith Miller  &lt;keith_miller@apple.com&gt;
+
+        ClonedArguments should not materialize its special properties unless they are being changed or deleted
+        https://bugs.webkit.org/show_bug.cgi?id=154128
+
+        Reviewed by Filip Pizlo.
+
+        Have argumnets-strict-mode test the speed of spreading the arguments object.
+
+        * js/regress/script-tests/arguments-strict-mode.js:
+        (foo):
+
</ins><span class="cx"> 2016-02-16  Ryan Haddad  &lt;ryanhaddad@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Marking fast/events/keydown-1.html as flaky on mac-wk1 debug
</span></span></pre></div>
<a id="trunkLayoutTestsjsregressscripttestsargumentsstrictmodejs"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/js/regress/script-tests/arguments-strict-mode.js (196643 => 196644)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/js/regress/script-tests/arguments-strict-mode.js        2016-02-16 19:13:17 UTC (rev 196643)
+++ trunk/LayoutTests/js/regress/script-tests/arguments-strict-mode.js        2016-02-16 19:28:09 UTC (rev 196644)
</span><span class="lines">@@ -1,12 +1,13 @@
</span><span class="cx"> function foo() {
</span><span class="cx">     &quot;use strict&quot;;
</span><del>-    return arguments[0];
</del><ins>+    return [...arguments];
+
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> noInline(foo);
</span><span class="cx"> 
</span><del>-for (var i = 0; i &lt; 1000000; ++i) {
</del><ins>+for (var i = 0; i &lt; 200000; ++i) {
</ins><span class="cx">     var result = foo(i);
</span><del>-    if (result != i)
</del><ins>+    if (result[0] != i)
</ins><span class="cx">         throw &quot;Error: bad result: &quot; + result;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (196643 => 196644)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2016-02-16 19:13:17 UTC (rev 196643)
+++ trunk/Source/JavaScriptCore/ChangeLog        2016-02-16 19:28:09 UTC (rev 196644)
</span><span class="lines">@@ -1,3 +1,21 @@
</span><ins>+2016-02-16  Keith Miller  &lt;keith_miller@apple.com&gt;
+
+        ClonedArguments should not materialize its special properties unless they are being changed or deleted
+        https://bugs.webkit.org/show_bug.cgi?id=154128
+
+        Reviewed by Filip Pizlo.
+
+        Before we would materialize ClonedArguments whenever they were being accessed.
+        However this would cause the IC to miss every time as the structure for
+        the arguments object would change as we went to IC it. Thus on the next
+        function call we would miss the cache since the new arguments object
+        would not have materialized the value.
+
+        * runtime/ClonedArguments.cpp:
+        (JSC::ClonedArguments::getOwnPropertySlot):
+        * tests/stress/cloned-arguments-modification.js: Added.
+        (foo):
+
</ins><span class="cx"> 2016-02-16  Filip Pizlo  &lt;fpizlo@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         FTL should support StringFromCharCode
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeClonedArgumentscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/ClonedArguments.cpp (196643 => 196644)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/ClonedArguments.cpp        2016-02-16 19:13:17 UTC (rev 196643)
+++ trunk/Source/JavaScriptCore/runtime/ClonedArguments.cpp        2016-02-16 19:28:09 UTC (rev 196644)
</span><span class="lines">@@ -132,16 +132,33 @@
</span><span class="cx"> {
</span><span class="cx">     ClonedArguments* thisObject = jsCast&lt;ClonedArguments*&gt;(object);
</span><span class="cx">     VM&amp; vm = exec-&gt;vm();
</span><del>-    
-    if (ident == vm.propertyNames-&gt;callee
-        || ident == vm.propertyNames-&gt;caller
-        || ident == vm.propertyNames-&gt;iteratorSymbol)
-        thisObject-&gt;materializeSpecialsIfNecessary(exec);
-    
-    if (Base::getOwnPropertySlot(thisObject, exec, ident, slot))
-        return true;
-    
-    return false;
</del><ins>+
+    if (!thisObject-&gt;specialsMaterialized()) {
+        FunctionExecutable* executable = jsCast&lt;FunctionExecutable*&gt;(thisObject-&gt;m_callee-&gt;executable());
+        bool isStrictMode = executable-&gt;isStrictMode();
+
+        if (isStrictMode) {
+            if (ident == vm.propertyNames-&gt;callee) {
+                slot.setGetterSlot(thisObject, DontDelete | DontEnum | Accessor, thisObject-&gt;globalObject()-&gt;throwTypeErrorGetterSetter(vm));
+                return true;
+            }
+            if (ident == vm.propertyNames-&gt;caller) {
+                slot.setGetterSlot(thisObject, DontDelete | DontEnum | Accessor, thisObject-&gt;globalObject()-&gt;throwTypeErrorGetterSetter(vm));
+                return true;
+            }
+
+        } else if (ident == vm.propertyNames-&gt;callee) {
+            slot.setValue(thisObject, 0, thisObject-&gt;m_callee.get());
+            return true;
+        }
+
+        if (ident == vm.propertyNames-&gt;iteratorSymbol) {
+            slot.setValue(thisObject, DontEnum, thisObject-&gt;globalObject()-&gt;arrayProtoValuesFunction());
+            return true;
+        }
+    }
+
+    return Base::getOwnPropertySlot(thisObject, exec, ident, slot);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void ClonedArguments::getOwnPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray&amp; array, EnumerationMode mode)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressclonedargumentsmodificationjs"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/cloned-arguments-modification.js (0 => 196644)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/cloned-arguments-modification.js                                (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/cloned-arguments-modification.js        2016-02-16 19:28:09 UTC (rev 196644)
</span><span class="lines">@@ -0,0 +1,37 @@
</span><ins>+function foo() {
+    &quot;use strict&quot;;
+
+    if (arguments[Symbol.iterator] !== Array.prototype.values)
+        throw &quot;Symbol.iterator is wrong&quot;;
+
+    arguments[Symbol.iterator] = 1;
+
+    if (arguments[Symbol.iterator] !== 1)
+        throw &quot;Symbol.iterator did not update&quot;;
+
+    let failed = true;
+    try {
+        arguments.callee;
+    } catch (e) {
+        failed = false;
+    }
+    if (failed)
+        throw &quot;one property stopped another from showing up&quot;;
+
+    delete arguments[Symbol.iterator];
+
+    if (Symbol.iterator in arguments)
+        throw &quot;Symbol.iterator did not get deleted&quot;;
+
+    failed = true;
+    try {
+        arguments.callee;
+    } catch (e) {
+        failed = false;
+    }
+    if (failed)
+        throw &quot;one property stopped another from showing up&quot;;
+}
+
+for (i = 0; i &lt; 10000; i++)
+    foo();
</ins></span></pre>
</div>
</div>

</body>
</html>