Changeset 28923 in webkit for trunk/JavaScriptCore/kjs/nodes.cpp

Timestamp:

Dec 20, 2007, 6:18:11 PM (17 years ago)

Author:

[email protected]

Message:

Reviewed by Oliver Hunt.

AST optimization: Avoid NULL-checking ForNode's child nodes.

0.6% speedup on SunSpider.

This is a proof of concept patch that demonstrates how to optimize
grammar productions with optional components, like

for (optional; optional; optional) {

...

}

The parser emits NULL for an optional component that is not present.

Instead of checking for a NULL child at execution time, a node that
expects an optional component to be present more often than not checks
for a NULL child at construction time, and substitutes a viable
alternative node in its place.

(We'd like the parser to start emitting NULL a lot more once we teach
it to emit NULL for certain no-op productions like EmptyStatement and
VariableStatement, so, as a foundation, it's important for nodes with
NULL optional components to be fast.)

• kjs/Parser.cpp: (KJS::Parser::didFinishParsing): Check for NULL SourceElements. Also, moved didFinishParsing into the .cpp file because adding a branch while it was in the header file caused a substantial and inexplicable performance regression. (Did I mention that GCC is crazy?)

• kjs/grammar.y:

• kjs/nodes.cpp: (KJS::BlockNode::BlockNode): Check for NULL SourceElements. (KJS::ForNode::optimizeVariableAccess): No need to check for NULL here. (KJS::ForNode::execute): No need to check for NULL here.
• kjs/nodes.h: (KJS::ForNode::): Check for NULL SourceElements. Substitute a TrueNode because it's semantically harmless, and it evaluates to boolean in an efficient manner.

File:

• trunk/JavaScriptCore/kjs/nodes.cpp (modified) (4 diffs)

trunk/JavaScriptCore/kjs/nodes.cpp

@@ -3606 +3606 @@
 
 BlockNode::BlockNode(SourceElements* children)
-    : m_children(children)
+    : m_children(children ? children : new SourceElements)
 {
     ASSERT(m_children);
@@ -3773 +3773 @@
 {
     nodeStack.append(statement.get());
-    if (expr3)
-        nodeStack.append(expr3.get());
-    if (expr2)
-        nodeStack.append(expr2.get());
-    if (expr1)
-        nodeStack.append(expr1.get());
+    nodeStack.append(expr3.get());
+    nodeStack.append(expr2.get());
+    nodeStack.append(expr1.get());
 }
 
@@ -3786 +3783 @@
     JSValue* value = 0;
 
-    if (expr1) {
-        expr1->evaluate(exec);
+    expr1->evaluate(exec);
+    KJS_CHECKEXCEPTION
+
+    while (1) {
+        bool b = expr2->evaluateToBoolean(exec);
         KJS_CHECKEXCEPTION
-    }
-
-    while (1) {
-        if (expr2) {
-            bool b = expr2->evaluateToBoolean(exec);
-            KJS_CHECKEXCEPTION
-            if (!b)
-                break;
-        }
+        if (!b)
+            break;
 
         exec->pushIteration();
@@ -3817 +3810 @@
 
 continueForLoop:
-        if (expr3) {
-            expr3->evaluate(exec);
-            KJS_CHECKEXCEPTION
-        }
+        expr3->evaluate(exec);
+        KJS_CHECKEXCEPTION
     }