Changeset 27126 in webkit for trunk/JavaScriptCore/kjs

Timestamp:

Oct 26, 2007, 3:43:03 PM (18 years ago)

Author:

ggaren

Message:

Reviewed by Maciej Stachowiak.

Switched ActivationImp to using a symbol table. For now, though, all
clients take the slow path.

Net .6% speedup on SunSpider.

Slowdowns:

• ActivationImp now mallocs in its constructor
• Local variable hits use an extra level of indirection to retrieve data
• Local variable misses do two lookups

Speedups:

• Fast initialization of local variables upon function entry

• JavaScriptCore.xcodeproj/project.pbxproj: Added SymbolTable.h

• kjs/function.cpp: (KJS::ActivationImp::ActivationImp): Malloc a private structure to hold data that won't fit in a JSCell. (KJS::ActivationImp::argumentsGetter): Use slow symbol table path for lookup. (KJS::ActivationImp::getOwnPropertySlot): ditto (KJS::ActivationImp::deleteProperty): ditto (KJS::ActivationImp::put): ditto (KJS::ActivationImp::createArgumentsObject): ditto

(KJS::ActivationImp::mark): Call JSObject::mark first so that one of
our properties doesn't try to recursively mark us. (This caused a crash
in earlier testing. Not sure why we haven't run into it before.)

• kjs/nodes.cpp: Functions now build a symbol table the first time they're called. (KJS::VarDeclNode::evaluate): (KJS::FunctionBodyNode::FunctionBodyNode): (KJS::FunctionBodyNode::initializeSymbolTable): (KJS::FunctionBodyNode::processDeclarations): (KJS::FunctionBodyNode::processDeclarationsForFunctionCode): (KJS::FunctionBodyNode::processDeclarationsForProgramCode):

• kjs/nodes.h: (KJS::FunctionBodyNode::symbolTable):

• wtf/Forward.h: Added Vector.

Location:

trunk/JavaScriptCore/kjs

Files:

• function.cpp (modified) (7 diffs)
• function.h (modified) (4 diffs)
• nodes.cpp (modified) (5 diffs)
• nodes.h (modified) (3 diffs)

trunk/JavaScriptCore/kjs/function.cpp

@@ -391 +391 @@
 const ClassInfo ActivationImp::info = {"Activation", 0, 0, 0};
 
-// ECMA 10.1.6
 ActivationImp::ActivationImp(FunctionImp* function, const List& arguments)
-    : _function(function), _arguments(arguments), _argumentsObject(0)
-{
-  // FIXME: Do we need to support enumerating the arguments property?
+    : d(new ActivationImpPrivate(function, arguments))
+    , symbolTable(&function->body->symbolTable())
+{
 }
 
@@ -401 +400 @@
 {
   ActivationImp* thisObj = static_cast<ActivationImp*>(slot.slotBase());
-
-  // default: return builtin arguments array
-  if (!thisObj->_argumentsObject)
+  ActivationImpPrivate* d = thisObj->d.get();
+  
+  if (!d->argumentsObject)
     thisObj->createArgumentsObject(exec);
   
-  return thisObj->_argumentsObject;
+  return d->argumentsObject;
 }
 
@@ -416 +415 @@
 bool ActivationImp::getOwnPropertySlot(ExecState* exec, const Identifier& propertyName, PropertySlot& slot)
 {
-    // do this first so property map arguments property wins over the below
-    // we don't call JSObject because we won't have getter/setter properties
-    // and we don't want to support __proto__
+    // We don't call through to JSObject because there's no way to give an 
+    // acitvation object getter/setter properties, and exposing __proto__ in
+    // the scope chain would be bizarre.
+    ASSERT(!_prop.hasGetterSetterProperties());
+
+    size_t index;
+    if (symbolTable->get(propertyName, index)) {
+        slot.setValueSlot(this, &d->localStorage[index].value);
+        return true;
+    }
 
     if (JSValue** location = getDirectLocation(propertyName)) {
@@ -425 +431 @@
     }
 
+    // Only return the built-in arguments object if it wasn't overridden above.
     if (propertyName == exec->propertyNames().arguments) {
         slot.setCustom(this, getArgumentsGetter());
@@ -437 +444 @@
     if (propertyName == exec->propertyNames().arguments)
         return false;
+
+    size_t index;
+    if (symbolTable->get(propertyName, index))
+        return false;
+
     return JSObject::deleteProperty(exec, propertyName);
 }
@@ -442 +454 @@
 void ActivationImp::put(ExecState*, const Identifier& propertyName, JSValue* value, int attr)
 {
-  // There's no way that an activation object can have a prototype or getter/setter properties
+  // There's no way that an activation object can have a prototype or getter/setter properties.
   ASSERT(!_prop.hasGetterSetterProperties());
   ASSERT(prototype() == jsNull());
 
+  size_t index;
+  if (symbolTable->get(propertyName, index)) {
+    LocalStorageEntry& entry = d->localStorage[index];
+    entry.value = value;
+    entry.attributes = attr;
+    return;
+  }
+
   _prop.put(propertyName, value, attr, (attr == None || attr == DontDelete));
 }
@@ -451 +471 @@
 void ActivationImp::mark()
 {
-    if (_function && !_function->marked()) 
-        _function->mark();
-    if (_argumentsObject && !_argumentsObject->marked())
-        _argumentsObject->mark();
     JSObject::mark();
+
+    if (!d->function->marked())
+        d->function->mark();
+
+    size_t size = d->localStorage.size();
+    for (size_t i = 0; i < size; ++i) {
+        JSValue* value = d->localStorage[i].value;
+        if (!value->marked())
+            value->mark();
+    }
+
+    if (d->argumentsObject && !d->argumentsObject->marked())
+        d->argumentsObject->mark();
 }
 
 void ActivationImp::createArgumentsObject(ExecState* exec)
 {
-  _argumentsObject = new Arguments(exec, _function, _arguments, const_cast<ActivationImp*>(this));
-  // The arguments list is only needed to create the arguments object, so discard it now
-  _arguments.reset();
+  d->argumentsObject = new Arguments(exec, d->function, d->arguments, this);
+
+  // The arguments list is only needed to create the arguments object, so discard it now.
+  // This prevents lists of Lists from building up, waiting to be garbage collected.
+  d->arguments.reset();
 }
 

trunk/JavaScriptCore/kjs/function.h

@@ -25 +25 @@
 #define KJS_FUNCTION_H
 
+#include "SymbolTable.h"
 #include "object.h"
 #include <wtf/OwnPtr.h>
@@ -138 +139 @@
   class ActivationImp : public JSObject {
   public:
-    ActivationImp(FunctionImp* function, const List& arguments);
+    struct LocalStorageEntry {
+        LocalStorageEntry()
+        {
+        }
+
+        LocalStorageEntry(JSValue* v, int a)
+            : value(v)
+            , attributes(a)
+        {
+        }
+
+        JSValue* value;
+        int attributes;
+    };
+
+    typedef Vector<LocalStorageEntry, 32> LocalStorage;
+
+  private:
+    struct ActivationImpPrivate {
+        ActivationImpPrivate(FunctionImp* f, const List& a)
+            : function(f)
+            , arguments(a)
+            , argumentsObject(0)
+        {
+            ASSERT(f);
+        }
+        
+        FunctionImp* function;
+        LocalStorage localStorage;
+
+        List arguments;
+        Arguments* argumentsObject;
+    };
+
+  public:
+    ActivationImp::ActivationImp(FunctionImp* function, const List& arguments);
 
     virtual bool getOwnPropertySlot(ExecState*, const Identifier&, PropertySlot&);
@@ -151 +187 @@
     bool isActivation() { return true; }
 
-    void releaseArguments() { _arguments.reset(); }
+    void releaseArguments() { d->arguments.reset(); }
+    
+    LocalStorage& localStorage() { return d->localStorage; };
 
   private:
@@ -157 +195 @@
     static JSValue* argumentsGetter(ExecState*, JSObject*, const Identifier&, const PropertySlot& slot);
     void createArgumentsObject(ExecState*);
-
-    FunctionImp* _function;
-    List _arguments;
-    mutable Arguments* _argumentsObject;
+    
+    OwnPtr<ActivationImpPrivate> d;
+    SymbolTable* symbolTable;
   };
 

trunk/JavaScriptCore/kjs/nodes.cpp

@@ -1835 +1835 @@
             flags |= ReadOnly;
         
-        ASSERT(variableObject->getDirect(ident) || ident == exec->propertyNames().arguments);
+        ASSERT(variableObject->hasProperty(exec, ident));
         variableObject->put(exec, ident, val, flags);
     }
@@ -2579 +2579 @@
     , m_sourceId(Parser::sid)
     , m_initializedDeclarationStacks(false)
+    , m_initializedSymbolTable(false)
 {
   setLoc(-1, -1);
@@ -2607 +2608 @@
 }
 
-void FunctionBodyNode::processDeclarationsFunctionCode(ExecState* exec)
+void FunctionBodyNode::initializesymbolTable()
 {
     size_t i, size;
-
-    JSObject* variableObject = exec->variableObject();
-
+    size_t count = 0;
+
+    // The order of additions here implicitly enforces the mutual exclusion described in ECMA 10.1.3.
+    for (i = 0, size = m_varStack.size(); i < size; ++i)
+        m_symbolTable.set(m_varStack[i]->ident, count++);
+
+    for (i = 0, size = m_parameters.size(); i < size; ++i)
+        m_symbolTable.set(m_parameters[i], count++);
+
+    for (i = 0, size = m_functionStack.size(); i < size; ++i)
+        m_symbolTable.set(m_functionStack[i]->ident, count++);
+
+    m_initializedSymbolTable = true;
+}
+
+void FunctionBodyNode::processDeclarations(ExecState* exec)
+{
+    if (!m_initializedDeclarationStacks)
+        initializeDeclarationStacks(exec);
+
+    if (exec->codeType() == FunctionCode)
+        processDeclarationsForFunctionCode(exec);
+    else
+        processDeclarationsForProgramCode(exec);
+}
+
+void FunctionBodyNode::processDeclarationsForFunctionCode(ExecState* exec)
+{
+    if (!m_initializedSymbolTable)
+        initializesymbolTable();
+
+    ASSERT(exec->variableObject()->isActivation());
+    ActivationImp::LocalStorage& localStorage = static_cast<ActivationImp*>(exec->variableObject())->localStorage();
+    localStorage.reserveCapacity(m_varStack.size() + m_parameters.size() + m_functionStack.size());
+    
     int minAttributes = Internal | DontDelete;
-
-    // The order of additions to the variable object here implicitly enforces the mutual exclusion described in ECMA 10.1.3.
+    
+    size_t i, size;
+
+    // NOTE: Must match the order of addition in initializesymbolTable().
+
     for (i = 0, size = m_varStack.size(); i < size; ++i) {
         VarDeclNode* node = m_varStack[i];
@@ -2621 +2657 @@
         if (node->varType == VarDeclNode::Constant)
             attributes |= ReadOnly;
-        variableObject->put(exec, node->ident, jsUndefined(), attributes);
+        localStorage.append(ActivationImp::LocalStorageEntry(jsUndefined(), attributes));
     }
 
     const List& args = *exec->arguments();
     for (i = 0, size = m_parameters.size(); i < size; ++i)
-        variableObject->put(exec, m_parameters[i], args[i], DontDelete);
+        localStorage.append(ActivationImp::LocalStorageEntry(args[i], DontDelete));
 
     for (i = 0, size = m_functionStack.size(); i < size; ++i) {
         FuncDeclNode* node = m_functionStack[i];
-        variableObject->put(exec, node->ident, node->makeFunction(exec), minAttributes);
+        localStorage.append(ActivationImp::LocalStorageEntry(node->makeFunction(exec), minAttributes));
     }
 }
 
-void FunctionBodyNode::processDeclarationsProgramCode(ExecState* exec)
+void FunctionBodyNode::processDeclarationsForProgramCode(ExecState* exec)
 {
     size_t i, size;
@@ -2678 +2714 @@
 }
 
-void FunctionBodyNode::processDeclarations(ExecState* exec)
-{
-    if (!m_initializedDeclarationStacks)
-        initializeDeclarationStacks(exec);
-
-    if (exec->codeType() == FunctionCode)
-        processDeclarationsFunctionCode(exec);
-    else
-        processDeclarationsProgramCode(exec);
-}
-
 Completion FunctionBodyNode::execute(ExecState* exec)
 {

trunk/JavaScriptCore/kjs/nodes.h

@@ -1235 +1235 @@
 
     virtual Completion execute(ExecState*) KJS_FAST_CALL;
+    
+    SymbolTable& symbolTable() { return m_symbolTable; }
 
     void addParam(const Identifier& ident) KJS_FAST_CALL;
@@ -1242 +1244 @@
     Vector<Identifier>& parameters() KJS_FAST_CALL { return m_parameters; }
     ALWAYS_INLINE void processDeclarations(ExecState*);
-    ALWAYS_INLINE void processDeclarationsFunctionCode(ExecState*);
-    ALWAYS_INLINE void processDeclarationsProgramCode(ExecState*);
+    ALWAYS_INLINE void processDeclarationsForFunctionCode(ExecState*);
+    ALWAYS_INLINE void processDeclarationsForProgramCode(ExecState*);
   private:
     UString m_sourceURL;
@@ -1251 +1253 @@
     bool m_initializedDeclarationStacks;
 
-    // Properties that will go into the ActivationImp's symbol table. (Used for initializing the ActivationImp.)
-    DeclarationStacks::VarStack m_varStack;
-    DeclarationStacks::FunctionStack m_functionStack;
-    Vector<Identifier> m_parameters;
+    void initializesymbolTable();
+    bool m_initializedSymbolTable;
+    
+    // Properties that will go into the ActivationImp's local storage. (Used for initializing the ActivationImp.)
+     DeclarationStacks::VarStack m_varStack;
+     DeclarationStacks::FunctionStack m_functionStack;
+     Vector<Identifier> m_parameters;
+
+    // Mapping from property name -> local storage index. (Used once to transform the AST, and subsequently for residual slow case lookups.)
+    SymbolTable m_symbolTable;
   };