Changeset 72351 in webkit for trunk/JavaScriptCore/profiler/ProfileGenerator.cpp

Timestamp:

Nov 18, 2010, 4:54:26 PM (15 years ago)

Author:

[email protected]

Message:

Bug 49635 - Profiler implementation is fragile

Reviewed by Oliver Hunt.

JavaScriptCore:

The profile presently requires the exception handling mechanism to explicitly
remove all stack frames that are exited during the exception unwind mechanism.
This is fragile in a number of ways:

• We have to change bytecode register allocation when compiling code to run when profiling, to preserve the callee function (this is also required to call did_call after the call has returned).
• In the JIT we have to maintain additional data structures (CodeBlock::RareData::m_functionRegisterInfos) to map back to the register containing the callee.
• In the interpreter we use 'magic values' to offset into the instruction stream to rediscover the register containing the function.

Instead, move profiling into the head and tail of functions.

• This correctly accounts the cost of the call itself to the caller.
• This allows us to access the callee function object from the callframe.
• This means that at the point a call is made we can track the stack depth on the ProfileNode.
• When unwinding we can simply report the depth at which the exception is being handled - all call frames above this level are freed.

• bytecode/CodeBlock.cpp:

(JSC::CodeBlock::shrinkToFit):

• bytecode/CodeBlock.h:

(JSC::CodeBlock::bytecodeOffset):
(JSC::CodeBlock::methodCallLinkInfo):

• bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::emitCall):
(JSC::BytecodeGenerator::emitCallVarargs):

• interpreter/Interpreter.cpp:

(JSC::Interpreter::unwindCallFrame):
(JSC::Interpreter::throwException):
(JSC::Interpreter::execute):
(JSC::Interpreter::executeCall):
(JSC::Interpreter::executeConstruct):
(JSC::Interpreter::privateExecute):

• jit/JITStubs.cpp:

(JSC::DEFINE_STUB_FUNCTION):

• profiler/Profile.cpp:

(JSC::Profile::Profile):

• profiler/ProfileGenerator.cpp:

(JSC::ProfileGenerator::addParentForConsoleStart):
(JSC::ProfileGenerator::willExecute):
(JSC::ProfileGenerator::didExecute):
(JSC::ProfileGenerator::exceptionUnwind):
(JSC::ProfileGenerator::stopProfiling):

• profiler/ProfileGenerator.h:
• profiler/ProfileNode.cpp:

(JSC::ProfileNode::ProfileNode):
(JSC::ProfileNode::willExecute):

• profiler/ProfileNode.h:

(JSC::ProfileNode::create):
(JSC::ProfileNode::callerCallFrame):

• profiler/Profiler.cpp:

(JSC::dispatchFunctionToProfiles):
(JSC::Profiler::_willExecute):
(JSC::Profiler::_didExecute):
(JSC::Profiler::exceptionUnwind):

• profiler/Profiler.h:

LayoutTests:

Fixes previously failing tests - output was incorrect, showing duplicate entries
for '(program) (no file) (line 1)'.

• fast/profiler/throw-exception-from-eval-expected.txt:

File:

• trunk/JavaScriptCore/profiler/ProfileGenerator.cpp (modified) (7 diffs)

trunk/JavaScriptCore/profiler/ProfileGenerator.cpp

@@ -64 +64 @@
 
     exec->interpreter()->retrieveLastCaller(exec, lineNumber, sourceID, sourceURL, function);
-    m_currentNode = ProfileNode::create(Profiler::createCallIdentifier(exec, function ? function.toThisObject(exec) : 0, sourceURL, lineNumber), m_head.get(), m_head.get());
+    m_currentNode = ProfileNode::create(exec, Profiler::createCallIdentifier(exec, function ? function.toThisObject(exec) : 0, sourceURL, lineNumber), m_head.get(), m_head.get());
     m_head->insertNode(m_currentNode.get());
 }
@@ -73 +73 @@
 }
 
-void ProfileGenerator::willExecute(const CallIdentifier& callIdentifier)
+void ProfileGenerator::willExecute(ExecState* callerCallFrame, const CallIdentifier& callIdentifier)
 {
     if (JAVASCRIPTCORE_PROFILE_WILL_EXECUTE_ENABLED()) {
@@ -84 +84 @@
         return;
 
-    ASSERT_ARG(m_currentNode, m_currentNode);
-    m_currentNode = m_currentNode->willExecute(callIdentifier);
+    ASSERT(m_currentNode);
+    m_currentNode = m_currentNode->willExecute(callerCallFrame, callIdentifier);
 }
 
-void ProfileGenerator::didExecute(const CallIdentifier& callIdentifier)
+void ProfileGenerator::didExecute(ExecState* callerCallFrame, const CallIdentifier& callIdentifier)
 {
     if (JAVASCRIPTCORE_PROFILE_DID_EXECUTE_ENABLED()) {
@@ -99 +99 @@
         return;
 
-    ASSERT_ARG(m_currentNode, m_currentNode);
+    ASSERT(m_currentNode);
     if (m_currentNode->callIdentifier() != callIdentifier) {
-        RefPtr<ProfileNode> returningNode = ProfileNode::create(callIdentifier, m_head.get(), m_currentNode.get());
+        RefPtr<ProfileNode> returningNode = ProfileNode::create(callerCallFrame, callIdentifier, m_head.get(), m_currentNode.get());
         returningNode->setStartTime(m_currentNode->startTime());
         returningNode->didExecute();
@@ -111 +111 @@
 }
 
+void ProfileGenerator::exceptionUnwind(ExecState* handlerCallFrame, const CallIdentifier&)
+{
+    // If the current node was called by the handler (==) or any
+    // more nested function (>) the we have exited early from it.
+    ASSERT(m_currentNode);
+    while (m_currentNode->callerCallFrame() >= handlerCallFrame) {
+        didExecute(m_currentNode->callerCallFrame(), m_currentNode->callIdentifier());
+        ASSERT(m_currentNode);
+    }
+}
+
 void ProfileGenerator::stopProfiling()
 {
@@ -118 +129 @@
     removeProfileEnd();
 
-    ASSERT_ARG(m_currentNode, m_currentNode);
+    ASSERT(m_currentNode);
 
     // Set the current node to the parent, because we are in a call that
@@ -125 +136 @@
 
    if (double headSelfTime = m_head->selfTime()) {
-        RefPtr<ProfileNode> idleNode = ProfileNode::create(CallIdentifier(NonJSExecution, UString(), 0), m_head.get(), m_head.get());
+        RefPtr<ProfileNode> idleNode = ProfileNode::create(0, CallIdentifier(NonJSExecution, UString(), 0), m_head.get(), m_head.get());
 
         idleNode->setTotalTime(headSelfTime);