Bug 20874: op_resolve does not do any form of caching
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Sep 2008 05:53:15 +0000 (05:53 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Sep 2008 05:53:15 +0000 (05:53 +0000)
<https://bugs.webkit.org/show_bug.cgi?id=20874>

Reviewed by Cameron Zwarich

This patch adds an op_resolve_global opcode to handle (and cache)
property lookup we can statically determine must occur on the global
object (if at all).

3% progression on sunspider, 3.2x improvement to bitops-bitwise-and, and
10% in math-partial-sums

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@36480 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/VM/CTI.cpp
JavaScriptCore/VM/CTI.h
JavaScriptCore/VM/CodeBlock.cpp
JavaScriptCore/VM/CodeGenerator.cpp
JavaScriptCore/VM/Machine.cpp
JavaScriptCore/VM/Machine.h
JavaScriptCore/VM/Opcode.h

index 4bf2bd3..342ff10 100644 (file)
@@ -1,3 +1,32 @@
+2008-09-15  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Cameron Zwarich.
+
+        Bug 20874: op_resolve does not do any form of caching
+        <https://bugs.webkit.org/show_bug.cgi?id=20874>
+
+        This patch adds an op_resolve_global opcode to handle (and cache)
+        property lookup we can statically determine must occur on the global
+        object (if at all).
+
+        3% progression on sunspider, 3.2x improvement to bitops-bitwise-and, and
+        10% in math-partial-sums
+
+        * VM/CTI.cpp:
+        (JSC::CTI::privateCompileMainPass):
+        * VM/CTI.h:
+        * VM/CodeBlock.cpp:
+        (JSC::CodeBlock::dump):
+        * VM/CodeGenerator.cpp:
+        (JSC::CodeGenerator::findScopedProperty):
+        (JSC::CodeGenerator::emitResolve):
+        * VM/Machine.cpp:
+        (JSC::resolveGlobal):
+        (JSC::Machine::privateExecute):
+        (JSC::Machine::cti_op_resolve_global):
+        * VM/Machine.h:
+        * VM/Opcode.h:
+
 2008-09-15  Sam Weinig  <sam@webkit.org>
 
 
index 77247ec..b9b3be7 100644 (file)
@@ -952,6 +952,39 @@ void CTI::privateCompileMainPass()
             i += 4;
             break;
         }
+        case op_resolve_global: {
+            // Fast case
+            unsigned globalObject = reinterpret_cast<unsigned>(instruction[i + 2].u.jsCell);
+            Identifier* ident = &(m_codeBlock->identifiers[instruction[i + 3].u.operand]);
+            void* structureIDAddr = reinterpret_cast<void*>(instruction + i + 4);
+            void* offsetAddr = reinterpret_cast<void*>(instruction + i + 5);
+
+            // Check StructureID of global object
+            m_jit.movl_i32r(globalObject, X86::eax);
+            m_jit.movl_mr(structureIDAddr, X86::edx);
+            m_jit.cmpl_rm(X86::edx, OBJECT_OFFSET(JSCell, m_structureID), X86::eax);
+            X86Assembler::JmpSrc slowCase = m_jit.emitUnlinkedJne(); // StructureIDs don't match
+            m_slowCases.append(SlowCaseEntry(slowCase, i));
+
+            // Load cached property
+            m_jit.movl_mr(OBJECT_OFFSET(JSGlobalObject, m_propertyStorage), X86::eax, X86::eax);
+            m_jit.movl_mr(offsetAddr, X86::edx);
+            m_jit.movl_mr(0, X86::eax, X86::edx, sizeof(JSValue*), X86::eax);
+            emitPutResult(instruction[i + 1].u.operand);
+            X86Assembler::JmpSrc end = m_jit.emitUnlinkedJmp();
+
+            // Slow case
+            m_jit.link(slowCase, m_jit.label());
+            emitPutArgConstant(globalObject, 0);
+            emitPutArgConstant(reinterpret_cast<unsigned>(ident), 4);
+            emitPutArgConstant(reinterpret_cast<unsigned>(instruction + i), 8);
+            emitCall(i, Machine::cti_op_resolve_global);
+            emitPutResult(instruction[i + 1].u.operand);
+            m_jit.link(end, m_jit.label());
+            i += 6;
+            ++structureIDInstructionIndex;
+            break;
+        }
         CTI_COMPILE_BINARY_OP(op_div)
         case op_pre_dec: {
             int srcDst = instruction[i + 1].u.operand;
@@ -1625,6 +1658,11 @@ void CTI::privateCompileSlowCases()
             i += 8;
             break;
         }
+        case op_resolve_global: {
+            ++structureIDInstructionIndex;
+            i += 6;
+            break;
+        }
         case op_loop_if_lesseq: {
             emitSlowScriptCheck(i);
 
index 22bbdfc..befdca2 100644 (file)
@@ -89,6 +89,9 @@
 #define ARG_registers1 ((Register*)((ARGS)[1]))
 #define ARG_regexp1 ((RegExp*)((ARGS)[1]))
 #define ARG_pni1 ((JSPropertyNameIterator*)((ARGS)[1]))
+#define ARG_instr1 ((Instruction*)((ARGS)[1]))
+#define ARG_instr2 ((Instruction*)((ARGS)[2]))
+#define ARG_instr3 ((Instruction*)((ARGS)[3]))
 #define ARG_instr4 ((Instruction*)((ARGS)[4]))
 #define ARG_instr5 ((Instruction*)((ARGS)[5]))
 
index 7bff4b2..6b40c85 100644 (file)
@@ -214,6 +214,10 @@ void CodeBlock::printStructureIDs(const Instruction* vPC) const
         printStructureID("put_by_id_replace", vPC, 4);
         return;
     }
+    if (vPC[0].u.opcode == machine->getOpcode(op_resolve_global)) {
+        printStructureID("resolve_global", vPC, 4);
+        return;
+    }
 
     // These instructions doesn't ref StructureIDs.
     ASSERT(vPC[0].u.opcode == machine->getOpcode(op_get_by_id_generic) || vPC[0].u.opcode == machine->getOpcode(op_put_by_id_generic));
@@ -540,6 +544,14 @@ void CodeBlock::dump(ExecState* exec, const Vector<Instruction>::const_iterator&
             printf("[%4d] resolve_skip\t %s, %s, %d\n", location, registerName(r0).c_str(), idName(id0, identifiers[id0]).c_str(), skipLevels);
             break;
         }
+        case op_resolve_global: {
+            int r0 = (++it)->u.operand;
+            JSValue* scope = static_cast<JSValue*>((++it)->u.jsCell);
+            int id0 = (++it)->u.operand;
+            printf("[%4d] resolve_global\t %s, %s, %s\n", location, registerName(r0).c_str(), valueToSourceString(exec, scope).ascii(), idName(id0, identifiers[id0]).c_str());
+            it += 2;
+            break;
+        }
         case op_get_scoped_var: {
             int r0 = (++it)->u.operand;
             int index = (++it)->u.operand;
@@ -921,6 +933,11 @@ void CodeBlock::derefStructureIDs(Instruction* vPC) const
         vPC[4].u.structureID->deref();
         return;
     }
+    if (vPC[0].u.opcode == machine->getOpcode(op_resolve_global)) {
+        if(vPC[4].u.structureID)
+            vPC[4].u.structureID->deref();
+        return;
+    }
     
     // These instructions don't ref their StructureIDs.
     ASSERT(vPC[0].u.opcode == machine->getOpcode(op_get_by_id) || vPC[0].u.opcode == machine->getOpcode(op_put_by_id) || vPC[0].u.opcode == machine->getOpcode(op_get_by_id_generic) || vPC[0].u.opcode == machine->getOpcode(op_put_by_id_generic) || vPC[0].u.opcode == machine->getOpcode(op_get_array_length) || vPC[0].u.opcode == machine->getOpcode(op_get_string_length));
index f33d0c9..bbddc62 100644 (file)
@@ -759,17 +759,23 @@ RegisterID* CodeGenerator::emitUnexpectedLoad(RegisterID* dst, double d)
 
 bool CodeGenerator::findScopedProperty(const Identifier& property, int& index, size_t& stackDepth, bool forWriting, JSValue*& globalObject)
 {
-    // Cases where we cannot optimise the lookup
+    // Cases where we cannot statically optimise the lookup
     if (property == propertyNames().arguments || !canOptimizeNonLocals()) {
         stackDepth = 0;
         index = missingSymbolMarker();
+
+        if (shouldOptimizeLocals() && m_codeType == GlobalCode) {
+            ScopeChainIterator iter = m_scopeChain->begin();
+            globalObject = *iter;
+            ASSERT((++iter) == m_scopeChain->end());
+        }
         return false;
     }
 
+    size_t depth = 0;
+    
     ScopeChainIterator iter = m_scopeChain->begin();
     ScopeChainIterator end = m_scopeChain->end();
-    size_t depth = 0;
-
     for (; iter != end; ++iter, ++depth) {
         JSObject* currentScope = *iter;
         if (!currentScope->isVariableObject())
@@ -820,7 +826,7 @@ RegisterID* CodeGenerator::emitResolve(RegisterID* dst, const Identifier& proper
     size_t depth = 0;
     int index = 0;
     JSValue* globalObject = 0;
-    if (!findScopedProperty(property, index, depth, false, globalObject)) {
+    if (!findScopedProperty(property, index, depth, false, globalObject) && !globalObject) {
         // We can't optimise at all :-(
         emitOpcode(op_resolve);
         instructions().append(dst->index());
@@ -828,18 +834,29 @@ RegisterID* CodeGenerator::emitResolve(RegisterID* dst, const Identifier& proper
         return dst;
     }
 
-    if (index == missingSymbolMarker()) {
-        // In this case we are at least able to drop a few scope chains from the
-        // lookup chain, although we still need to hash from then on.
-        emitOpcode(op_resolve_skip);
+    if (index != missingSymbolMarker()) {
+        // Directly index the property lookup across multiple scopes.  Yay!
+        return emitGetScopedVar(dst, depth, index, globalObject);
+    }
+
+    if (globalObject) {
+        m_codeBlock->structureIDInstructions.append(instructions().size());
+        emitOpcode(op_resolve_global);
         instructions().append(dst->index());
+        instructions().append(static_cast<JSCell*>(globalObject));
         instructions().append(addConstant(property));
-        instructions().append(depth);
+        instructions().append(0);
+        instructions().append(0);
         return dst;
     }
 
-    // Directly index the property lookup across multiple scopes.  Yay!
-    return emitGetScopedVar(dst, depth, index, globalObject);
+    // In this case we are at least able to drop a few scope chains from the
+    // lookup chain, although we still need to hash from then on.
+    emitOpcode(op_resolve_skip);
+    instructions().append(dst->index());
+    instructions().append(addConstant(property));
+    instructions().append(depth);
+    return dst;
 }
 
 RegisterID* CodeGenerator::emitGetScopedVar(RegisterID* dst, size_t depth, int index, JSValue* globalObject)
index d59b54d..7d05005 100644 (file)
@@ -331,6 +331,45 @@ static bool NEVER_INLINE resolve_skip(ExecState* exec, Instruction* vPC, Registe
     return false;
 }
 
+static bool NEVER_INLINE resolveGlobal(ExecState* exec, Instruction* vPC, Register* r, CodeBlock* codeBlock, JSValue*& exceptionValue)
+{
+    int dst = (vPC + 1)->u.operand;
+    JSGlobalObject* globalObject = static_cast<JSGlobalObject*>((vPC + 2)->u.jsCell);
+    ASSERT(globalObject->isGlobalObject());
+    int property = (vPC + 3)->u.operand;
+    StructureID* structureID = (vPC + 4)->u.structureID;
+    int offset = (vPC + 5)->u.operand;
+
+    if (structureID == globalObject->structureID()) {
+        r[dst] = globalObject->getDirectOffset(offset);
+        return true;
+    }
+
+    Identifier& ident = codeBlock->identifiers[property];
+    PropertySlot slot(globalObject);
+    if (globalObject->getPropertySlot(exec, ident, slot)) {
+        JSValue* result = slot.getValue(exec, ident);
+        if (slot.isCacheable()) {
+            if (vPC[4].u.structureID)
+                vPC[4].u.structureID->deref();
+            globalObject->structureID()->ref();
+            vPC[4] = globalObject->structureID();
+            vPC[5] = slot.cachedOffset();
+            r[dst] = result;
+            return true;
+        }
+
+        exceptionValue = exec->exception();
+        if (exceptionValue)
+            return false;
+        r[dst] = result;
+        return true;
+    }
+
+    exceptionValue = createUndefinedVariableError(exec, ident, vPC, codeBlock);
+    return false;
+}
+
 ALWAYS_INLINE static JSValue* inlineResolveBase(ExecState* exec, Identifier& property, ScopeChainNode* scopeChain)
 {
     ScopeChainIterator iter = scopeChain->begin();
@@ -2221,8 +2260,23 @@ JSValue* Machine::privateExecute(ExecutionFlag flag, ExecState* exec, RegisterFi
 
         NEXT_OPCODE;
     }
+    BEGIN_OPCODE(op_resolve_global) {
+        /* resolve_skip dst(r) globalObject(c) property(id) structureID(sID) offset(n)
+         
+           Performs a dynamic property lookup for the given property, on the provided
+           global object.  If structureID matches the StructureID of the global then perform
+           a fast lookup using the case offset, otherwise fall back to a full resolve and
+           cache the new structureID and offset
+         */
+        if (UNLIKELY(!resolveGlobal(exec, vPC, r,  codeBlock, exceptionValue)))
+            goto vm_throw;
+        
+        vPC += 6;
+        
+        NEXT_OPCODE;
+    }
     BEGIN_OPCODE(op_get_global_var) {
-        /* get_global_var dst(r) index(n)
+        /* get_global_var dst(r) globalObject(c) index(n)
 
            Gets the global var at global slot index and places it in register dst.
          */
@@ -4780,6 +4834,36 @@ JSValue* Machine::cti_op_resolve_skip(CTI_ARGS)
     return 0;
 }
 
+JSValue* Machine::cti_op_resolve_global(CTI_ARGS)
+{
+    ExecState* exec = ARG_exec;
+    JSGlobalObject* globalObject = static_cast<JSGlobalObject*>(ARG_src1);
+    Identifier& ident = *ARG_id2;
+    Instruction* vPC = ARG_instr3;
+    ASSERT(globalObject->isGlobalObject());
+
+    PropertySlot slot(globalObject);
+    if (globalObject->getPropertySlot(exec, ident, slot)) {
+        JSValue* result = slot.getValue(exec, ident);
+        if (slot.isCacheable()) {
+            if (vPC[4].u.structureID)
+                vPC[4].u.structureID->deref();
+            globalObject->structureID()->ref();
+            vPC[4] = globalObject->structureID();
+            vPC[5] = slot.cachedOffset();
+            return result;
+        }
+
+        VM_CHECK_EXCEPTION_AT_END();
+        return result;
+    }
+    
+    exec->setException(createUndefinedVariableError(exec, ident, vPC, ARG_codeBlock));
+    
+    VM_CHECK_EXCEPTION_AT_END();
+    return 0;
+}
+
 JSValue* Machine::cti_op_div(CTI_ARGS)
 {
     ExecState* exec = ARG_exec;
index d75af90..4060391 100644 (file)
@@ -163,6 +163,7 @@ namespace JSC {
         static JSValue* SFX_CALL cti_op_ret(CTI_ARGS);
         static JSValue* SFX_CALL cti_op_new_array(CTI_ARGS);
         static JSValue* SFX_CALL cti_op_resolve(CTI_ARGS);
+        static JSValue* SFX_CALL cti_op_resolve_global(CTI_ARGS);
         static void* SFX_CALL cti_op_construct_JSConstruct(CTI_ARGS);
         static JSValue* SFX_CALL cti_op_construct_NotJSConstruct(CTI_ARGS);
         static void SFX_CALL cti_op_construct_verify(CTI_ARGS);
index 580da83..7181bc7 100644 (file)
@@ -88,6 +88,7 @@ namespace JSC {
         \
         macro(op_resolve) \
         macro(op_resolve_skip) \
+        macro(op_resolve_global) \
         macro(op_get_scoped_var) \
         macro(op_put_scoped_var) \
         macro(op_get_global_var) \