Changeset 34086 in webkit for trunk/JavaScriptCore


Ignore:
Timestamp:
May 23, 2008, 12:12:31 PM (17 years ago)
Author:
[email protected]
Message:

2008-05-23 Geoffrey Garen <​[email protected]>

Rolled out r34085 because it measured as a 7.6% performance regression.

Location:
trunk/JavaScriptCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/ChangeLog

    r34085 r34086  
    112008-05-23  Geoffrey Garen  <[email protected]>
    22
    3         Reviewed by Darin Adler, Kevin McCullough, and Oliver Hunt.
    4        
    5         Fixed <rdar://problem/5959447> Crashes and incorrect reporting in the
    6         JavaScript profiler
    7 
    8         * VM/Machine.cpp:
    9         (KJS::callEval): Made this profiler hooks slightly faster by passing in
    10         the eval function.
    11        
    12         (KJS::Machine::unwindCallFrame): Fixed incorrect reporting / a crash
    13         when unwinding from inside eval and/or program code: detect the
    14         difference, and do the right thing. Also, be sure to notify the profiler
    15         *before* deref'ing the scope chain, since the profiler uses the scope chain.
    16 
    17         (KJS::Machine::execute): Fixed incorrect reporting / crash when calling
    18         a JS function re-entrently: Machine::execute(FunctionBodyNode*...)
    19         should not invoke the didExecute hook, because op_ret already does that.
    20         Also, use the new function's ExecState when calling out to the profiler.
    21         (Not important now, but could have become a subtle bug later.)
    22 
    23         (KJS::Machine::privateExecute): Fixed a hard to reproduce crash when
    24         profiling JS functions: notify the profiler *before* deref'ing the scope
    25         chain, since the profiler uses the scope chain.
    26 
    27         * kjs/object.cpp:
    28         (KJS::JSObject::call): Removed these hooks, because they are now unnecessary.
    29 
    30         * profiler/Profile.cpp: Added a comment to explain a subtlety that only
    31         Kevin and I understood previously. (Now, the whole world can understand!)
    32 
    33         * profiler/Profiler.cpp:
    34         (KJS::shouldExcludeFunction): Don't exclude .call and .apply. That was
    35         a hack to fix bugs that no longer exist.
    36 
    37         Finally, sped things up a little bit by changing the "Is the profiler
    38         running?" check into an ASSERT, since we only call into the profiler
    39         when it's running:
    40 
    41         (KJS::Profiler::willExecute):
    42         (KJS::Profiler::didExecute):
     3        Rolled out r34085 because it measured as a 7.6% performance regression.
    434
    4452008-05-23  Adam Roben  <[email protected]>

  • trunk/JavaScriptCore/VM/Machine.cpp

    r34085 r34086  
    438438}
    439439
    440 static NEVER_INLINE JSValue* callEval(ExecState* exec, JSObject* thisObj, JSObject* evalFunction, ScopeChainNode* scopeChain, RegisterFile* registerFile, Register* r, int argv, int argc, JSValue*& exceptionValue)
     440static NEVER_INLINE JSValue* callEval(ExecState* exec, JSObject* thisObj, ScopeChainNode* scopeChain, RegisterFile* registerFile, Register* r, int argv, int argc, JSValue*& exceptionValue)
    441441{
    442442    Profiler** profiler = Profiler::enabledProfilerReference();
     443    JSObject* evalFunction = scopeChain->globalObject()->evalFunction();
    443444    if (*profiler)
    444445        (*profiler)->willExecute(exec, evalFunction);
    … …  
    576577    }
    577578
    578     Register* callFrame = r - oldCodeBlock->numLocals - CallFrameHeaderSize;
    579    
    580     if (Profiler* profiler = *Profiler::enabledProfilerReference()) {
    581         if (!isGlobalCallFrame(registerBase, r) && callFrame[Callee].u.jsObject) // Check for global and eval code
    582             profiler->didExecute(exec, callFrame[Callee].u.jsObject);
    583         else
    584             profiler->didExecute(exec, codeBlock->ownerNode->sourceURL(), codeBlock->ownerNode->lineNo());
    585     }
    586 
    587579    if (oldCodeBlock->needsFullScopeChain)
    588580        scopeChain->deref();
    … …  
    591583        return false;
    592584
     585    Register* callFrame = r - oldCodeBlock->numLocals - CallFrameHeaderSize;
     586   
    593587    codeBlock = callFrame[CallerCodeBlock].u.codeBlock;
    594588    if (!codeBlock)
    … …  
    608602    vPC = callFrame[ReturnVPC].u.vPC;
    609603
     604    if (Profiler* profiler = *Profiler::enabledProfilerReference())
     605        profiler->didExecute(exec, callFrame[Callee].u.jsObject);
    610606    return true;
    611607}
    … …  
    673669        scopeChain = scopeChain->copy();
    674670
    675     ExecState newExec(exec, this, registerFile, scopeChain, -1);
    676 
    677671    Profiler** profiler = Profiler::enabledProfilerReference();
    678672    if (*profiler)
    679673        (*profiler)->willExecute(exec, programNode->sourceURL(), programNode->lineNo());
     674
     675    ExecState newExec(exec, this, registerFile, scopeChain, -1);
    680676
    681677    m_reentryDepth++;
    … …  
    743739    JSValue* result = privateExecute(Normal, &newExec, registerFile, r, scopeChain, newCodeBlock, exception);
    744740    m_reentryDepth--;
     741
     742    if (*profiler)
     743        (*profiler)->didExecute(exec, function);
    745744
    746745    registerFile->shrink(oldSize);
    … …  
    18661865            registerFile->setSafeForReentry(true);
    18671866
    1868             JSValue* result = callEval(exec, thisObject, static_cast<JSObject*>(funcVal), scopeChain, registerFile, r, argv, argc, exceptionValue);
     1867            JSValue* result = callEval(exec, thisObject, scopeChain, registerFile, r, argv, argc, exceptionValue);
    18691868
    18701869            registerFile->setSafeForReentry(false);
    … …  
    19761975            activation->copyRegisters();
    19771976        }
    1978 
    1979         if (*enabledProfilerReference)
    1980             (*enabledProfilerReference)->didExecute(exec, callFrame[Callee].u.jsObject);
    19811977
    19821978        if (codeBlock->needsFullScopeChain)
    … …  
    20001996        int r0 = callFrame[ReturnValueRegister].u.i;
    20011997        r[r0].u.jsValue = returnValue;
     1998
     1999        if (*enabledProfilerReference)
     2000            (*enabledProfilerReference)->didExecute(exec, callFrame[Callee].u.jsObject);
    20022001
    20032002        NEXT_OPCODE;

  • trunk/JavaScriptCore/kjs/object.cpp

    r34085 r34086  
    9292  }
    9393#endif
    94   JSValue* ret = callAsFunction(exec,thisObj,args);
    95 
     94    CallData data;
     95    JSValue *ret;
     96    if (this->getCallData(data) == CallTypeJS) {
     97        // A native function will enter the VM, which will allow the profiler
     98        // to detect entry
     99        ret = callAsFunction(exec,thisObj,args);
     100    } else {
     101        Profiler** profiler = Profiler::enabledProfilerReference();
     102        if (*profiler)
     103            (*profiler)->willExecute(exec, this);
     104       
     105        ret = callAsFunction(exec,thisObj,args);
     106       
     107        if (*profiler)
     108            (*profiler)->didExecute(exec, this);
     109    }
    96110#if KJS_MAX_STACK > 0
    97111  --depth;

  • trunk/JavaScriptCore/profiler/Profile.cpp

    r34085 r34086  
    5757{
    5858    ASSERT(m_currentNode);
     59
    5960    m_currentNode = m_currentNode->willExecute(callIdentifier);
    6061}
    … …  
    6263void Profile::didExecute(const CallIdentifier& callIdentifier)
    6364{
    64     ASSERT(m_currentNode);
    65 
    66     // In case the profiler started recording calls in the middle of a stack frame,
    67     // when returning up the stack it needs to insert the calls it missed on the
    68     // way down.
    6965    if (m_currentNode == m_headNode) {
    7066        m_currentNode = ProfileNode::create(callIdentifier, m_headNode.get(), m_headNode.get());

  • trunk/JavaScriptCore/profiler/Profiler.cpp

    r34085 r34086  
    9999}
    100100
     101static inline bool shouldExcludeFunction(ExecState* exec, JSObject* calledFunction)
     102{
     103    if (!calledFunction->inherits(&InternalFunctionImp::info))
     104        return false;
     105    // Don't record a call for function.call and function.apply.
     106    if (static_cast<InternalFunctionImp*>(calledFunction)->functionName() == exec->propertyNames().call)
     107        return true;
     108    if (static_cast<InternalFunctionImp*>(calledFunction)->functionName() == exec->propertyNames().apply)
     109        return true;
     110    return false;
     111}
     112
    101113static inline void dispatchFunctionToProfiles(const Vector<RefPtr<Profile> >& profiles, Profile::ProfileFunction function, const CallIdentifier& callIdentifier, unsigned currentPageGroupIdentifier)
    102114{
    … …  
    108120void Profiler::willExecute(ExecState* exec, JSObject* calledFunction)
    109121{
    110     ASSERT(!m_currentProfiles.isEmpty());
     122    if (m_currentProfiles.isEmpty())
     123        return;
     124
     125    if (shouldExcludeFunction(exec, calledFunction))
     126        return;
    111127
    112128    dispatchFunctionToProfiles(m_currentProfiles, &Profile::willExecute, createCallIdentifier(calledFunction), exec->lexicalGlobalObject()->pageGroupIdentifier());
    … …  
    115131void Profiler::willExecute(ExecState* exec, const UString& sourceURL, int startingLineNumber)
    116132{
    117     ASSERT(!m_currentProfiles.isEmpty());
     133    if (m_currentProfiles.isEmpty())
     134        return;
    118135
    119136    CallIdentifier callIdentifier = createCallIdentifier(sourceURL, startingLineNumber);
    … …  
    124141void Profiler::didExecute(ExecState* exec, JSObject* calledFunction)
    125142{
    126     ASSERT(!m_currentProfiles.isEmpty());
     143    if (m_currentProfiles.isEmpty())
     144        return;
     145
     146    if (shouldExcludeFunction(exec, calledFunction))
     147        return;
    127148
    128149    dispatchFunctionToProfiles(m_currentProfiles, &Profile::didExecute, createCallIdentifier(calledFunction), exec->lexicalGlobalObject()->pageGroupIdentifier());
    … …  
    131152void Profiler::didExecute(ExecState* exec, const UString& sourceURL, int startingLineNumber)
    132153{
    133     ASSERT(!m_currentProfiles.isEmpty());
     154    if (m_currentProfiles.isEmpty())
     155        return;
    134156
    135157    dispatchFunctionToProfiles(m_currentProfiles, &Profile::didExecute, createCallIdentifier(sourceURL, startingLineNumber), exec->lexicalGlobalObject()->pageGroupIdentifier());
Note: See TracChangeset for help on using the changeset viewer.