Changeset 196644 in webkit

Timestamp:

Feb 16, 2016 11:28:09 AM (8 years ago)

Author:

keith_miller@apple.com

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

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/regress/script-tests/arguments-strict-mode.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/ClonedArguments.cpp (modified) (1 diff)
• Source/JavaScriptCore/tests/stress/cloned-arguments-modification.js (added)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-02-16  Keith Miller  <keith_miller@apple.com>
+
+        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  <ryanhaddad@apple.com>
 

trunk/LayoutTests/js/regress/script-tests/arguments-strict-mode.js

@@ -1 +1 @@
 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;
 }

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-02-16  Keith Miller  <keith_miller@apple.com>
+
+        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  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/ClonedArguments.cpp

@@ -133 +133 @@
     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);
 }