+2007-07-22 Maciej Stachowiak <mjs@apple.com>
+
+ 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:
+
2007-07-22 Darin Adler <darin@apple.com>
Reviewed by Kevin Decker.
{
}
-// ECMA 12.2
-JSValue *VarDeclNode::evaluate(ExecState *exec)
+JSValue* VarDeclNode::handleSlowCase(ExecState* exec, const ScopeChain& chain, JSValue* val)
{
- 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;
+ 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;
+}
- ++iter;
- } while (iter != end);
+// ECMA 12.2
+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();
- unsigned flags = 0;
- base->getPropertyAttributes(ident, flags);
- if (varType == VarDeclNode::Constant)
- flags |= ReadOnly;
+ ASSERT(!chain.isEmpty());
+ ASSERT(variableObject->getDirect(ident) || ident == exec->propertyNames().arguments);
- 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();
- }
+ // 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);
-#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());
+ 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;
}
void VarDeclNode::processVarDecls(ExecState *exec)
git://git.webkit.org / WebKit-https.git / commitdiff