Changeset 33327

Timestamp:

05/12/08 23:03:29 (6 months ago)

Author:

mrowe@apple.com

Message:

2008-05-03 Geoffrey Garen <ggaren@apple.com>

Reviewed by Oliver Hunt.

A bit more efficient fix than r32832: Don't copy globals into function
register files; instead, have the RegisterFileStack track only the base
of the last *global* register file, so the global object's register
references stay good.

SunSpider reports a .3% speedup. Not sure what that's about.

Location:

branches/squirrelfish/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• JavaScriptCore.exp (modified) (1 diff)
• VM/Machine.cpp (modified) (1 diff)
• VM/RegisterFile.cpp (modified) (1 diff)
• VM/RegisterFile.h (modified) (3 diffs)
• VM/RegisterFileStack.cpp (modified) (4 diffs)
• VM/RegisterFileStack.h (modified) (3 diffs)
• kjs/JSGlobalObject.h (modified) (1 diff)

branches/squirrelfish/JavaScriptCore/ChangeLog

@@ +1 @@
+2008-05-03  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+        
+        A bit more efficient fix than r32832: Don't copy globals into function
+        register files; instead, have the RegisterFileStack track only the base
+        of the last *global* register file, so the global object's register
+        references stay good.
+        
+        SunSpider reports a .3% speedup. Not sure what that's about.
+
 2008-05-03  Oliver Hunt  <oliver@apple.com>
 

branches/squirrelfish/JavaScriptCore/JavaScriptCore.exp

@@ -129 +129 @@
 __ZN3KJS17PrototypeFunctionC1EPNS_9ExecStateEPNS_17FunctionPrototypeEiRKNS_10IdentifierEPFPNS_7JSValueES2_PNS_8JSObjectERKNS_4ListEE
 __ZN3KJS17PrototypeFunctionC1EPNS_9ExecStateEiRKNS_10IdentifierEPFPNS_7JSValueES2_PNS_8JSObjectERKNS_4ListEE
-__ZN3KJS17RegisterFileStack20allocateRegisterFileEm
+__ZN3KJS17RegisterFileStack20allocateRegisterFileEmPS0_
 __ZN3KJS19InternalFunctionImp11getCallDataERNS_8CallDataE
 __ZN3KJS19InternalFunctionImp4infoE

branches/squirrelfish/JavaScriptCore/VM/Machine.cpp

@@ -556 +556 @@
 
     RegisterFile* registerFile = registerFileStack->pushGlobalRegisterFile();
-    CodeBlock* codeBlock = &programNode->code(scopeChain, !registerFileStack->inImplicitCall());
+    CodeBlock* codeBlock = &programNode->code(scopeChain, registerFileStack->current()->isGlobal());
     registerFile->addGlobalSlots(codeBlock->numVars);
 

branches/squirrelfish/JavaScriptCore/VM/RegisterFile.cpp

@@ -98 +98 @@
 {
     m_base = base;
-    m_stack->baseChanged(this);
+    if (m_baseObserver)
+        m_baseObserver->baseChanged(this);
 }
 

branches/squirrelfish/JavaScriptCore/VM/RegisterFile.h

@@ -88 +88 @@
     public:
         enum { DefaultRegisterFileSize = 2 * 1024 * 1024 };
-        RegisterFile(RegisterFileStack* stack, size_t maxSize)
-            : m_isForImplicitCall(false)
-            , m_size(0)
+        RegisterFile(size_t maxSize, RegisterFileStack* m_baseObserver)
+            : m_size(0)
             , m_capacity(0)
             , m_maxSize(maxSize)
             , m_base(0)
             , m_buffer(0)
-            , m_stack(stack)
+            , m_baseObserver(m_baseObserver)
         {
         }
@@ -150 +149 @@
             Collector::markStackObjectsConservatively(m_buffer, m_base + m_size);
         }
-        void setIsForImplicitCall(bool isForImplicitCall) { m_isForImplicitCall = isForImplicitCall; }
-        bool isForImplicitCall() { return m_isForImplicitCall; }
+
+        bool isGlobal() { return !!m_baseObserver; }
+
     private:
         size_t newBuffer(size_t size, size_t capacity, size_t minCapacity, size_t maxSize, size_t offset);
@@ -170 +170 @@
         Register* m_base;
         Register* m_buffer;
-        RegisterFileStack* m_stack;
+        RegisterFileStack* m_baseObserver;
     };
     

branches/squirrelfish/JavaScriptCore/VM/RegisterFileStack.cpp

@@ -36 +36 @@
 RegisterFileStack::~RegisterFileStack()
 {
-    for (size_t i = 0; i < m_stack.size(); ++i)
-        delete m_stack[i];
+    ASSERT(m_stack.size() == 1 && m_stack[0]->size() == 0); // The stack shouldn't be destroyed during execution.
+    delete m_stack[0];
 }
 
@@ -48 +48 @@
         return current;
 
-    RegisterFile* previous = current;
-
     // Slow case: Existing register file is in use: Create a nested
     // register file with a copy of this register file's globals.
-    RegisterFile* registerFile = allocateRegisterFile(current->maxSize() - current->size());
-    registerFile->addGlobalSlots(previous->numGlobalSlots());
-    registerFile->copyGlobals(previous);
+    RegisterFile* lastGlobal = this->lastGlobal();
 
-    return registerFile;
+    current = allocateRegisterFile(current->maxSize() - current->size(), this);
+    current->addGlobalSlots(lastGlobal->numGlobalSlots());
+    current->copyGlobals(lastGlobal);
+    m_globalBase = *current->basePointer();
+
+    return current;
 }
 
@@ -72 +73 @@
     m_stack.removeLast();
 
-    RegisterFile* previous = current();
-    ASSERT(tmp->numGlobalSlots() == previous->numGlobalSlots() || !m_implicitCallDepth);
-    previous->addGlobalSlots(tmp->numGlobalSlots() - previous->numGlobalSlots());
-    previous->copyGlobals(tmp);
-    m_base = *current()->basePointer();
+    ASSERT(tmp->isGlobal() || tmp->numGlobalSlots() == lastGlobal()->numGlobalSlots());
+    if (tmp->isGlobal()) {
+        RegisterFile* lastGlobal = this->lastGlobal();
+        lastGlobal->addGlobalSlots(tmp->numGlobalSlots() - lastGlobal->numGlobalSlots());
+        lastGlobal->copyGlobals(tmp);
+
+        m_globalBase = *lastGlobal->basePointer();
+    }
+
     delete tmp;
 }
@@ -82 +87 @@
 RegisterFile* RegisterFileStack::pushFunctionRegisterFile()
 {
-    m_implicitCallDepth++;
-
-    RegisterFile* previous = current();
-
-    RegisterFile* result = allocateRegisterFile(current()->maxSize() - current()->size());
-    result->addGlobalSlots(previous->numGlobalSlots());
-    result->copyGlobals(previous);
-    result->setIsForImplicitCall(true);
-    return result;
+    return allocateRegisterFile(current()->maxSize() - current()->size());
 }
 
 void RegisterFileStack::popFunctionRegisterFile()
 {
-    RegisterFile* tmp = m_stack.last();
+    delete m_stack.last();
     m_stack.removeLast();
-
-    RegisterFile* previous = current();
-    ASSERT(tmp->numGlobalSlots() == previous->numGlobalSlots());
-    previous->copyGlobals(tmp);
-    m_base = *current()->basePointer();
-    delete tmp;
 }
 
-RegisterFile* RegisterFileStack::allocateRegisterFile(size_t maxSize)
+RegisterFile* RegisterFileStack::allocateRegisterFile(size_t maxSize, RegisterFileStack* registerFileStack)
 {
-    RegisterFile* registerFile = new RegisterFile(this, maxSize);
+    RegisterFile* registerFile = new RegisterFile(maxSize, registerFileStack);
     m_stack.append(registerFile);
-    m_base = *registerFile->basePointer();
     return registerFile;
 }

branches/squirrelfish/JavaScriptCore/VM/RegisterFileStack.h

@@ -38 +38 @@
     public:
         RegisterFileStack()
-            : m_implicitCallDepth(0)
+            : m_globalBase(0)
         {
-            allocateRegisterFile();
+            allocateRegisterFile(RegisterFile::DefaultRegisterFileSize, this);
         }
 
@@ -60 +60 @@
         }
 
-        // Pointer to a value that holds the base of the top-most register file.
-        Register** basePointer() { return &m_base; }
+        // Pointer to a value that holds the base of the top-most global register file.
+        Register** globalBasePointer() { return &m_globalBase; }
 
         void baseChanged(RegisterFile* registerFile)
         {
-            ASSERT(registerFile == current());
-            m_base = *registerFile->basePointer();
+            ASSERT(registerFile == lastGlobal());
+            m_globalBase = *registerFile->basePointer();
         }
 
-        bool inImplicitCall() { return m_implicitCallDepth > 0; }
+    private:
+        typedef Vector<RegisterFile*, 4> Stack;
 
         RegisterFile* lastGlobal() {
             ASSERT(m_stack.size());
             for (size_t i = m_stack.size() - 1; i > 0; --i) {
-                if (!m_stack[i]->isForImplicitCall())
+                if (m_stack[i]->isGlobal())
                     return m_stack[i];
             }
-            ASSERT(!m_stack[0]->isForImplicitCall());
+            ASSERT(m_stack[0]->isGlobal());
             return m_stack[0];
         }
-    private:
-        typedef Vector<RegisterFile*, 4> Stack;
 
-        RegisterFile* allocateRegisterFile(size_t maxSize = RegisterFile::DefaultRegisterFileSize);
+        RegisterFile* allocateRegisterFile(size_t maxSize, RegisterFileStack* = 0);
 
         RegisterFile* previous() { return m_stack[m_stack.size() - 2]; }
@@ -89 +88 @@
 
         Stack m_stack;
-        Register* m_base;
-        int m_implicitCallDepth;
+        Register* m_globalBase;
     };
 

branches/squirrelfish/JavaScriptCore/kjs/JSGlobalObject.h

@@ -81 +81 @@
         struct JSGlobalObjectData : public JSVariableObjectData {
             JSGlobalObjectData(JSGlobalObject* globalObject, JSObject* thisValue)
-                : JSVariableObjectData(&symbolTable, registerFileStack.basePointer(), 0)
+                : JSVariableObjectData(&symbolTable, registerFileStack.globalBasePointer(), 0)
                 , globalScopeChain(globalObject)
                 , globalExec(new ExecState(globalObject, thisValue, globalScopeChain))