Changeset 27025 in webkit for trunk/JavaScriptCore/kjs

Timestamp:

Oct 25, 2007, 12:09:44 AM (18 years ago)

Author:

ggaren

Message:

JavaScriptCore:

Reviewed by Maciej Stachowiak.

Fixed ​http://bugs.webkit.org/show_bug.cgi?id=15683
Re-order declaration initialization to avoid calling hasProperty inside
VarDeclNode::processDeclaration

.7% speedup on SunSpider.

• kjs/function.h:
• kjs/function.cpp: Merged parameter processing into FunctionBodyNode's other processing of declared symbols, so the order of execution could change.

• kjs/nodes.cpp: (KJS::VarDeclNode::getDeclarations): Added special case for the "arguments" property name, explained in the comment.

(KJS::VarDeclNode::processDeclaration): Removed call to hasProperty
in the case of function code, since we know the declared symbol
management will resolve conflicts between symbols. Yay!

(KJS::VarDeclListNode::getDeclarations): Now that VarDeclNode's
implementation of getDeclarations is non-trivial, we can't take a
short-cut here any longer -- we need to put the VarDecl node on the
stack so it gets processed normally.

(KJS::FunctionBodyNode::processDeclarations): Changed the order of
processing to enforce mutual exclusion rules.

• kjs/nodes.h: (KJS::DeclarationStacks::DeclarationStacks): Structure includes an ExecState now, for fast access to the "arguments" property name.

LayoutTests:

Layout tests for bugs that might result from changes like
​http://bugs.webkit.org/show_bug.cgi?id=15683

• fast/js/vardecl-preserve-parameters-expected.txt: Added.
• fast/js/vardecl-preserve-parameters.html: Added.
• fast/js/vardecl-preserve-vardecl-expected.txt: Added.
• fast/js/vardecl-preserve-vardecl.html: Added.

Location:

trunk/JavaScriptCore/kjs

Files:

• function.cpp (modified) (2 diffs)
• function.h (modified) (1 diff)
• nodes.cpp (modified) (6 diffs)
• nodes.h (modified) (2 diffs)

trunk/JavaScriptCore/kjs/function.cpp

@@ -77 +77 @@
   ctx.setExecState(&newExec);
 
-  passInParameters(&newExec, args);
-
   Debugger* dbg = exec->dynamicInterpreter()->debugger();
   int sid = -1;
@@ -125 +123 @@
   else
     return jsUndefined();
-}
-
-// ECMA 10.1.3q
-inline void FunctionImp::passInParameters(ExecState* exec, const List& args)
-{
-    Vector<Identifier>& parameters = body->parameters();
-
-    JSObject* variable = exec->context()->variableObject();
-
-    size_t size = parameters.size();
-    for (size_t i = 0; i < size; ++i) {
-      variable->put(exec, parameters[i], args[i], DontDelete);
-    }
 }
 

trunk/JavaScriptCore/kjs/function.h

@@ -108 +108 @@
     static JSValue* callerGetter(ExecState*, JSObject*, const Identifier&, const PropertySlot&);
     static JSValue* lengthGetter(ExecState*, JSObject*, const Identifier&, const PropertySlot&);
-
-    void passInParameters(ExecState*, const List&);
   };
 

trunk/JavaScriptCore/kjs/nodes.cpp

@@ -1767 +1767 @@
 
 void VarDeclNode::getDeclarations(DeclarationStacks& stacks)
-{ 
+{
+    // The normal check to avoid overwriting pre-existing values with variable
+    // declarations doesn't work for the "arguments" property because we 
+    // instantiate it lazily. So we need to check here instead.
+    if (ident == stacks.exec->propertyNames().arguments)
+        return;
+
     stacks.varStack.append(this); 
 }
@@ -1845 +1851 @@
 void VarDeclNode::processDeclaration(ExecState* exec)
 {
-  JSObject* variable = exec->context()->variableObject();
-
-  // If a variable by this name already exists, don't clobber it -
-  // it might be a function parameter
-  if (!variable->hasProperty(exec, ident)) {
-    int flags = Internal;
-    if (exec->context()->codeType() != EvalCode)
-      flags |= DontDelete;
-    if (varType == VarDeclNode::Constant)
-      flags |= ReadOnly;
-    variable->put(exec, ident, jsUndefined(), flags);
-  }
+  Context* context = exec->context();
+  JSObject* variable = context->variableObject();
+
+  if (context->codeType() != FunctionCode)
+    if (variable->hasProperty(exec, ident))
+        return;
+
+  int flags = Internal;
+  if (context->codeType() != EvalCode)
+    flags |= DontDelete;
+  if (varType == VarDeclNode::Constant)
+    flags |= ReadOnly;
+  variable->put(exec, ident, jsUndefined(), flags);
 }
 
@@ -1877 +1884 @@
         stacks.nodeStack.append(next.get()); 
     }
-    stacks.varStack.append(var.get()); 
+    stacks.nodeStack.append(var.get());
 }
 
@@ -2596 +2603 @@
 }
 
-void FunctionBodyNode::initializeDeclarationStacks()
+void FunctionBodyNode::initializeDeclarationStacks(ExecState* exec)
 {
     Node* node = source.get();
@@ -2603 +2610 @@
 
     DeclarationStacks::NodeStack nodeStack;
-    DeclarationStacks stacks(nodeStack, m_varStack, m_functionStack);
+    DeclarationStacks stacks(exec, nodeStack, m_varStack, m_functionStack);
     
     while (true) {
@@ -2623 +2630 @@
 {
     if (!m_initializedDeclarationStacks)
-        initializeDeclarationStacks();
+        initializeDeclarationStacks(exec);
 
     size_t i, size;
+
+    JSObject* variableObject = exec->context()->variableObject();
+    const List& args = *exec->context()->arguments();
+
+    // The order of additions to the variable object here implicitly enforces the mutual exclusion described in ECMA 10.1.3.
+    for (i = 0, size = m_varStack.size(); i < size; ++i)
+        m_varStack[i]->processDeclaration(exec);
+
+    for (i = 0, size = m_parameters.size(); i < size; ++i)
+        variableObject->put(exec, m_parameters[i], args[i], DontDelete);
 
     for (i = 0, size = m_functionStack.size(); i < size; ++i)
         m_functionStack[i]->processDeclaration(exec);
-
-    for (i = 0, size = m_varStack.size(); i < size; ++i)
-        m_varStack[i]->processDeclaration(exec);
 }
 

trunk/JavaScriptCore/kjs/nodes.h

@@ -89 +89 @@
       typedef Vector<FuncDeclNode*, 16> FunctionStack;
       
-      DeclarationStacks(NodeStack& n, VarStack& v, FunctionStack& f)
-        : nodeStack(n)
+      DeclarationStacks(ExecState* e, NodeStack& n, VarStack& v, FunctionStack& f)
+        : exec(e)
+        , nodeStack(n)
         , varStack(v)
         , functionStack(f)
       {
       }
-        
+
+      ExecState* exec;
       NodeStack& nodeStack;
       VarStack& varStack; 
@@ -1244 +1246 @@
     UString m_sourceURL;
     int m_sourceId;
-    Vector<Identifier> m_parameters;
-
-    void initializeDeclarationStacks();
+
+    void initializeDeclarationStacks(ExecState*);
     bool m_initializedDeclarationStacks;
+
     DeclarationStacks::VarStack m_varStack;
     DeclarationStacks::FunctionStack m_functionStack;
+    Vector<Identifier> m_parameters;
   };