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