Title: [196644] trunk
Revision
196644
Author
[email protected]
Date
2016-02-16 11:28:09 -0800 (Tue, 16 Feb 2016)

Log Message

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/_javascript_Core:

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):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (196643 => 196644)


--- trunk/LayoutTests/ChangeLog	2016-02-16 19:13:17 UTC (rev 196643)
+++ trunk/LayoutTests/ChangeLog	2016-02-16 19:28:09 UTC (rev 196644)
@@ -1,3 +1,15 @@
+2016-02-16  Keith Miller  <[email protected]>
+
+        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):
+
 2016-02-16  Ryan Haddad  <[email protected]>
 
         Marking fast/events/keydown-1.html as flaky on mac-wk1 debug

Modified: trunk/LayoutTests/js/regress/script-tests/arguments-strict-mode.js (196643 => 196644)


--- 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)
@@ -1,12 +1,13 @@
 function foo() {
     "use strict";
-    return arguments[0];
+    return [...arguments];
+
 }
 
 noInline(foo);
 
-for (var i = 0; i < 1000000; ++i) {
+for (var i = 0; i < 200000; ++i) {
     var result = foo(i);
-    if (result != i)
+    if (result[0] != i)
         throw "Error: bad result: " + result;
 }

Modified: trunk/Source/_javascript_Core/ChangeLog (196643 => 196644)


--- trunk/Source/_javascript_Core/ChangeLog	2016-02-16 19:13:17 UTC (rev 196643)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-02-16 19:28:09 UTC (rev 196644)
@@ -1,3 +1,21 @@
+2016-02-16  Keith Miller  <[email protected]>
+
+        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):
+
 2016-02-16  Filip Pizlo  <[email protected]>
 
         FTL should support StringFromCharCode

Modified: trunk/Source/_javascript_Core/runtime/ClonedArguments.cpp (196643 => 196644)


--- trunk/Source/_javascript_Core/runtime/ClonedArguments.cpp	2016-02-16 19:13:17 UTC (rev 196643)
+++ trunk/Source/_javascript_Core/runtime/ClonedArguments.cpp	2016-02-16 19:28:09 UTC (rev 196644)
@@ -132,16 +132,33 @@
 {
     ClonedArguments* thisObject = jsCast<ClonedArguments*>(object);
     VM& vm = exec->vm();
-    
-    if (ident == vm.propertyNames->callee
-        || ident == vm.propertyNames->caller
-        || ident == vm.propertyNames->iteratorSymbol)
-        thisObject->materializeSpecialsIfNecessary(exec);
-    
-    if (Base::getOwnPropertySlot(thisObject, exec, ident, slot))
-        return true;
-    
-    return false;
+
+    if (!thisObject->specialsMaterialized()) {
+        FunctionExecutable* executable = jsCast<FunctionExecutable*>(thisObject->m_callee->executable());
+        bool isStrictMode = executable->isStrictMode();
+
+        if (isStrictMode) {
+            if (ident == vm.propertyNames->callee) {
+                slot.setGetterSlot(thisObject, DontDelete | DontEnum | Accessor, thisObject->globalObject()->throwTypeErrorGetterSetter(vm));
+                return true;
+            }
+            if (ident == vm.propertyNames->caller) {
+                slot.setGetterSlot(thisObject, DontDelete | DontEnum | Accessor, thisObject->globalObject()->throwTypeErrorGetterSetter(vm));
+                return true;
+            }
+
+        } else if (ident == vm.propertyNames->callee) {
+            slot.setValue(thisObject, 0, thisObject->m_callee.get());
+            return true;
+        }
+
+        if (ident == vm.propertyNames->iteratorSymbol) {
+            slot.setValue(thisObject, DontEnum, thisObject->globalObject()->arrayProtoValuesFunction());
+            return true;
+        }
+    }
+
+    return Base::getOwnPropertySlot(thisObject, exec, ident, slot);
 }
 
 void ClonedArguments::getOwnPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& array, EnumerationMode mode)

Added: trunk/Source/_javascript_Core/tests/stress/cloned-arguments-modification.js (0 => 196644)


--- trunk/Source/_javascript_Core/tests/stress/cloned-arguments-modification.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/cloned-arguments-modification.js	2016-02-16 19:28:09 UTC (rev 196644)
@@ -0,0 +1,37 @@
+function foo() {
+    "use strict";
+
+    if (arguments[Symbol.iterator] !== Array.prototype.values)
+        throw "Symbol.iterator is wrong";
+
+    arguments[Symbol.iterator] = 1;
+
+    if (arguments[Symbol.iterator] !== 1)
+        throw "Symbol.iterator did not update";
+
+    let failed = true;
+    try {
+        arguments.callee;
+    } catch (e) {
+        failed = false;
+    }
+    if (failed)
+        throw "one property stopped another from showing up";
+
+    delete arguments[Symbol.iterator];
+
+    if (Symbol.iterator in arguments)
+        throw "Symbol.iterator did not get deleted";
+
+    failed = true;
+    try {
+        arguments.callee;
+    } catch (e) {
+        failed = false;
+    }
+    if (failed)
+        throw "one property stopped another from showing up";
+}
+
+for (i = 0; i < 10000; i++)
+    foo();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to