- Revision
- 264750
- Author
- shvaikal...@gmail.com
- Date
- 2020-07-23 01:25:12 -0700 (Thu, 23 Jul 2020)
Log Message
Remove ArrayNode::m_optional
https://bugs.webkit.org/show_bug.cgi?id=214294
Reviewed by Darin Adler.
JSTests:
* microbenchmarks/destructuring-array-literal.js: Added.
* microbenchmarks/function-dot-apply-array-literal.js: Added.
* stress/apply-second-argument-must-be-array-like.js:
* stress/destructuring-assignment-syntax.js:
Source/_javascript_Core:
m_optional, which dates back to KJS era, means "is this an array with optional trailing comma,
with elision, or an empty array". It was used by ArrayNode::streamTo() to preserve a trailing
comma when converting array node to source string, as well as in few other places,
before ECMA-262 clarified trailing comma in array literals [1].
Currently, m_optional is used only by ArrayNode::isSimpleArray(), along with m_elision.
Checking m_elision is enough since trailing comma doesn't add extra `undefined` element.
This patch completely removes m_optional, speeding up destructuring and function.apply()
with empty arrays and arrays with trailing commas by ~55% and a factor of 11 respectively.
Reflect.apply() optimization (https://webkit.org/b/190668) will also benefit from this change.
Also, this change converts isSpreadExpression() check to an ASSERT (was enabled by r196323).
[1]: https://tc39.es/ecma262/#sec-array-initializer
* bytecompiler/NodesCodegen.cpp:
(JSC::ArrayNode::isSimpleArray const):
(JSC::ArrayNode::toArgumentList const):
(JSC::ArrayNode::emitDirectBinding):
* parser/NodeConstructors.h:
(JSC::ArrayNode::ArrayNode):
* parser/Nodes.h:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (264749 => 264750)
--- trunk/JSTests/ChangeLog 2020-07-23 07:50:11 UTC (rev 264749)
+++ trunk/JSTests/ChangeLog 2020-07-23 08:25:12 UTC (rev 264750)
@@ -1,3 +1,15 @@
+2020-07-23 Alexey Shvayka <shvaikal...@gmail.com>
+
+ Remove ArrayNode::m_optional
+ https://bugs.webkit.org/show_bug.cgi?id=214294
+
+ Reviewed by Darin Adler.
+
+ * microbenchmarks/destructuring-array-literal.js: Added.
+ * microbenchmarks/function-dot-apply-array-literal.js: Added.
+ * stress/apply-second-argument-must-be-array-like.js:
+ * stress/destructuring-assignment-syntax.js:
+
2020-07-22 Angelos Oikonomopoulos <ange...@igalia.com>
Skip failing intl tests on ARM
Added: trunk/JSTests/microbenchmarks/destructuring-array-literal.js (0 => 264750)
--- trunk/JSTests/microbenchmarks/destructuring-array-literal.js (rev 0)
+++ trunk/JSTests/microbenchmarks/destructuring-array-literal.js 2020-07-23 08:25:12 UTC (rev 264750)
@@ -0,0 +1,5 @@
+for (var i = 0; i < 1e6; ++i) {
+ var [] = [];
+ var [a] = [1,];
+ var [b, c] = [1, 2,];
+}
Added: trunk/JSTests/microbenchmarks/function-dot-apply-array-literal.js (0 => 264750)
--- trunk/JSTests/microbenchmarks/function-dot-apply-array-literal.js (rev 0)
+++ trunk/JSTests/microbenchmarks/function-dot-apply-array-literal.js 2020-07-23 08:25:12 UTC (rev 264750)
@@ -0,0 +1,8 @@
+function fn() {}
+noInline(Function.prototype.apply);
+
+for (var i = 0; i < 1e7; ++i) {
+ fn.apply(null, []);
+ fn.apply(null, [1,]);
+ fn.apply(null, [1, 2,]);
+}
Modified: trunk/JSTests/stress/apply-second-argument-must-be-array-like.js (264749 => 264750)
--- trunk/JSTests/stress/apply-second-argument-must-be-array-like.js 2020-07-23 07:50:11 UTC (rev 264749)
+++ trunk/JSTests/stress/apply-second-argument-must-be-array-like.js 2020-07-23 08:25:12 UTC (rev 264750)
@@ -18,19 +18,6 @@
}
}
-function shouldNotThrow(expr) {
- let testFunc = new Function(expr);
- for (let i = 0; i < 10000; i++) {
- let error;
- try {
- testFunc();
- } catch (e) {
- error = e;
- }
- assert(!error);
- }
-}
-
function foo() { }
shouldThrow("foo.apply(undefined, true)");
@@ -42,8 +29,16 @@
shouldThrow("foo.apply(undefined, 'hello')");
shouldThrow("foo.apply(undefined, Symbol())");
-shouldNotThrow("foo.apply(undefined, undefined)");
-shouldNotThrow("foo.apply(undefined, null)");
-shouldNotThrow("foo.apply(undefined, {})");
-shouldNotThrow("foo.apply(undefined, [])");
-shouldNotThrow("foo.apply(undefined, function(){})");
+function bar() {
+ return arguments.length;
+}
+
+for (let i = 0; i < 10000; i++) {
+ new Function(`
+ assert(bar.apply(undefined, undefined) === 0);
+ assert(bar.apply(undefined, null) === 0);
+ assert(bar.apply(undefined, {}) === 0);
+ assert(bar.apply(undefined, []) === 0);
+ assert(bar.apply(undefined, function() {}) === 0);
+ `)();
+}
Modified: trunk/JSTests/stress/destructuring-assignment-syntax.js (264749 => 264750)
--- trunk/JSTests/stress/destructuring-assignment-syntax.js 2020-07-23 07:50:11 UTC (rev 264749)
+++ trunk/JSTests/stress/destructuring-assignment-syntax.js 2020-07-23 08:25:12 UTC (rev 264750)
@@ -22,6 +22,11 @@
throw new Error("Bad error: " + String(error));
}
+testSyntax("[] = []");
+testSyntax("[] = [,]");
+testSyntax("[,] = [,]");
+testSyntax("[,] = []");
+
testSyntax("({ a: this.a } = {})");
testSyntax("({ a: this['a'] } = {})");
testSyntax("({ a: this[\"a\"] } = {})");
Modified: trunk/Source/_javascript_Core/ChangeLog (264749 => 264750)
--- trunk/Source/_javascript_Core/ChangeLog 2020-07-23 07:50:11 UTC (rev 264749)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-07-23 08:25:12 UTC (rev 264750)
@@ -1,5 +1,36 @@
2020-07-23 Alexey Shvayka <shvaikal...@gmail.com>
+ Remove ArrayNode::m_optional
+ https://bugs.webkit.org/show_bug.cgi?id=214294
+
+ Reviewed by Darin Adler.
+
+ m_optional, which dates back to KJS era, means "is this an array with optional trailing comma,
+ with elision, or an empty array". It was used by ArrayNode::streamTo() to preserve a trailing
+ comma when converting array node to source string, as well as in few other places,
+ before ECMA-262 clarified trailing comma in array literals [1].
+
+ Currently, m_optional is used only by ArrayNode::isSimpleArray(), along with m_elision.
+ Checking m_elision is enough since trailing comma doesn't add extra `undefined` element.
+
+ This patch completely removes m_optional, speeding up destructuring and function.apply()
+ with empty arrays and arrays with trailing commas by ~55% and a factor of 11 respectively.
+ Reflect.apply() optimization (https://webkit.org/b/190668) will also benefit from this change.
+
+ Also, this change converts isSpreadExpression() check to an ASSERT (was enabled by r196323).
+
+ [1]: https://tc39.es/ecma262/#sec-array-initializer
+
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::ArrayNode::isSimpleArray const):
+ (JSC::ArrayNode::toArgumentList const):
+ (JSC::ArrayNode::emitDirectBinding):
+ * parser/NodeConstructors.h:
+ (JSC::ArrayNode::ArrayNode):
+ * parser/Nodes.h:
+
+2020-07-23 Alexey Shvayka <shvaikal...@gmail.com>
+
Remove emitIsUndefined() from ClassExprNode::emitBytecode()
https://bugs.webkit.org/show_bug.cgi?id=214645
Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (264749 => 264750)
--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2020-07-23 07:50:11 UTC (rev 264749)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2020-07-23 08:25:12 UTC (rev 264750)
@@ -506,7 +506,7 @@
bool ArrayNode::isSimpleArray() const
{
- if (m_elision || m_optional)
+ if (m_elision)
return false;
for (ElementNode* ptr = m_element; ptr; ptr = ptr->next()) {
if (ptr->elision())
@@ -519,7 +519,7 @@
ArgumentListNode* ArrayNode::toArgumentList(ParserArena& parserArena, int lineNumber, int startPosition) const
{
- ASSERT(!m_elision && !m_optional);
+ ASSERT(!m_elision);
ElementNode* ptr = m_element;
if (!ptr)
return nullptr;
@@ -5103,8 +5103,7 @@
Vector<ExpressionNode*> elements;
for (; elementNodes; elementNodes = elementNodes->next()) {
ExpressionNode* value = elementNodes->value();
- if (value->isSpreadExpression())
- return nullptr;
+ ASSERT(!value->isSpreadExpression());
elements.append(value);
}
Modified: trunk/Source/_javascript_Core/parser/NodeConstructors.h (264749 => 264750)
--- trunk/Source/_javascript_Core/parser/NodeConstructors.h 2020-07-23 07:50:11 UTC (rev 264749)
+++ trunk/Source/_javascript_Core/parser/NodeConstructors.h 2020-07-23 08:25:12 UTC (rev 264750)
@@ -226,7 +226,6 @@
: ExpressionNode(location)
, m_element(nullptr)
, m_elision(elision)
- , m_optional(true)
{
}
@@ -234,7 +233,6 @@
: ExpressionNode(location)
, m_element(element)
, m_elision(0)
- , m_optional(false)
{
}
@@ -242,7 +240,6 @@
: ExpressionNode(location)
, m_element(element)
, m_elision(elision)
- , m_optional(true)
{
}
Modified: trunk/Source/_javascript_Core/parser/Nodes.h (264749 => 264750)
--- trunk/Source/_javascript_Core/parser/Nodes.h 2020-07-23 07:50:11 UTC (rev 264749)
+++ trunk/Source/_javascript_Core/parser/Nodes.h 2020-07-23 08:25:12 UTC (rev 264750)
@@ -716,7 +716,6 @@
ElementNode* m_element;
int m_elision;
- bool m_optional;
};
enum class ClassElementTag : uint8_t { No, Instance, Static, LastTag };
