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

Timestamp:

Jul 23, 2007, 12:10:35 AM (18 years ago)

Author:

mjs

Message:

Reviewed by Darin.

-fixed <rdar://problem/5353293> REGRESSION (r24287): 1% i-Bench JS slowdown from JavaScript compatibility fix (14719)
​http://bugs.webkit.org/show_bug.cgi?id=14719

My fix for this actually resulted in JS iBench being 1% faster than before the regression
and the Celtic Kane benchmark being 5% faster than before the regression.

• kjs/nodes.cpp: (VarDeclNode::handleSlowCase): factored out the slow code path to be out of line. (VarDeclNode::evaluate): I did a couple of things: (1) Don't check if the variable is already declared by looking for the property in the variable object, that code path was dead code. (2) Special-case the common case where the top of the scope and the variable object are the same; in that case the variable must always be in the variable object. (3) Don't return a jsString() of the variable name, nothing uses the return value from this node types evaluate method.
• kjs/nodes.h:

File:

• trunk/JavaScriptCore/kjs/nodes.cpp (modified) (1 diff)

trunk/JavaScriptCore/kjs/nodes.cpp

@@ -1588 +1588 @@
 }
 
+JSValue* VarDeclNode::handleSlowCase(ExecState* exec, const ScopeChain& chain, JSValue* val)
+{
+    ScopeChainIterator iter = chain.begin();
+    ScopeChainIterator end = chain.end();        
+    
+    // we must always have something in the scope chain
+    ASSERT(iter != end);
+    
+    PropertySlot slot;
+    JSObject* base;
+    
+    do {
+        base = *iter;
+        if (base->getPropertySlot(exec, ident, slot))
+            break;
+        
+        ++iter;
+    } while (iter != end);
+    
+    unsigned flags = 0;
+    base->getPropertyAttributes(ident, flags);
+    if (varType == VarDeclNode::Constant)
+        flags |= ReadOnly;
+    
+    base->put(exec, ident, val, flags);
+    return 0;
+}
+
 // ECMA 12.2
-JSValue *VarDeclNode::evaluate(ExecState *exec)
-{
-  JSObject* variable = exec->context()->variableObject();
-
-  JSValue* val;
-  if (init) {
-      val = init->evaluate(exec);
-      KJS_CHECKEXCEPTIONVALUE
-      
-      if (variable->getDirect(ident) || ident == exec->propertyNames().arguments) {
-          const ScopeChain& chain = exec->context()->scopeChain();
-          ScopeChainIterator iter = chain.begin();
-          ScopeChainIterator end = chain.end();        
-
-          // we must always have something in the scope chain
-          ASSERT(iter != end);
-
-          PropertySlot slot;
-          JSObject* base;
-
-          do {
-              base = *iter;
-              if (base->getPropertySlot(exec, ident, slot))
-                  break;
-
-             ++iter;
-          } while (iter != end);
-
-          unsigned flags = 0;
-          base->getPropertyAttributes(ident, flags);
-          if (varType == VarDeclNode::Constant)
-              flags |= ReadOnly;
-
-          base->put(exec, ident, val, flags);
-          return jsString(ident.ustring());
-      }
-    } else {
-      // already declared? - check with getDirect so you can override
-      // built-in properties of the global object with var declarations.
-      // Also check for 'arguments' property. The 'arguments' cannot be found with
-      // getDirect, because it's created lazily by
-      // ActivationImp::getOwnPropertySlot.
-      // Since variable declarations are always in function scope, 'variable'
-      // will always contain instance of ActivationImp and ActivationImp will
-      // always have 'arguments' property
-      if (variable->getDirect(ident) || ident == exec->propertyNames().arguments)
-          return 0;
-      val = jsUndefined();
-  }
-
-#ifdef KJS_VERBOSE
-  printInfo(exec,(UString("new variable ")+ident.ustring()).cstring().c_str(),val);
-#endif
-  // We use Internal to bypass all checks in derived objects, e.g. so that
-  // "var location" creates a dynamic property instead of activating window.location.
-  int flags = Internal;
-  if (exec->context()->codeType() != EvalCode)
-    flags |= DontDelete;
-  if (varType == VarDeclNode::Constant)
-    flags |= ReadOnly;
-  variable->put(exec, ident, val, flags);
-
-  return jsString(ident.ustring());
+JSValue* VarDeclNode::evaluate(ExecState* exec)
+{
+    JSValue* val;
+    if (init) {
+        val = init->evaluate(exec);
+        KJS_CHECKEXCEPTIONVALUE
+            
+        const ScopeChain& chain = exec->context()->scopeChain();
+        JSObject* variableObject = exec->context()->variableObject();
+
+        ASSERT(!chain.isEmpty());
+        ASSERT(variableObject->getDirect(ident) || ident == exec->propertyNames().arguments);
+
+        // if the variable object is the top of the scope chain, then that must
+        // be where this variable is declared, processVarDecls would have put 
+        // it there. Don't search the scope chain, to optimize this very common case.
+        if (chain.top() != variableObject)
+            return handleSlowCase(exec, chain, val);
+
+        unsigned flags = 0;
+        variableObject->getPropertyAttributes(ident, flags);
+        if (varType == VarDeclNode::Constant)
+            flags |= ReadOnly;
+        
+        variableObject->put(exec, ident, val, flags);
+    } 
+
+    // no caller of this function actually uses the return value. 
+    // FIXME: It would be better to change the inheritence hierarchy so this
+    // node doesn't even have an evaluate method, but instead a differently named
+    // one with a void return.
+    return 0;
 }