Fixes an ASSERT in the profiler when starting multiple profiles
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jun 2008 21:44:42 +0000 (21:44 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jun 2008 21:44:42 +0000 (21:44 +0000)
        with the same name inside the same function/program.

        Reviewed by Kevin McCullough.

        * profiler/Profile.cpp:
        (KJS::Profile::Profile): Initialize m_stoppedCallDepth to zero.
        (KJS::Profile::stopProfiling): Set the current node to the parent,
        because we are in a call that will not get a didExecute call.
        (KJS::Profile::removeProfile): Increment m_stoppedCallDepth to
        account for didExecute not being called for profile.
        (KJS::Profile::willExecute): Increment m_stoppedCallDepth if stopped.
        (KJS::Profile::didExecute): Decrement m_stoppedCallDepth if stopped and
        greater than zero, and return early.
        * profiler/Profile.h: Added stoppedProfiling().
        * profiler/Profiler.cpp:
        (KJS::Profiler::findProfile): Removed.
        (KJS::Profiler::startProfiling): Don't return early for stopped profiles.
        (KJS::Profiler::stopProfiling): Skipp stopped profiles.
        (KJS::Profiler::didFinishAllExecution): Code clean-up.
        * profiler/Profiler.h: Removed findProfile.

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

JavaScriptCore/ChangeLog
JavaScriptCore/profiler/Profile.cpp
JavaScriptCore/profiler/Profile.h
JavaScriptCore/profiler/Profiler.cpp
JavaScriptCore/profiler/Profiler.h
WebCore/manual-tests/inspector/profiler-test-compare-multiple-profiles.html
WebCore/manual-tests/inspector/profiler-test-stop-then-function-call.html [new file with mode: 0644]

index 4082017..8e16a65 100644 (file)
@@ -1,3 +1,27 @@
+2008-06-25  Timothy Hatcher  <timothy@apple.com>
+
+        Fixes an ASSERT in the profiler when starting multiple profiles
+        with the same name inside the same function/program.
+
+        Reviewed by Kevin McCullough.
+
+        * profiler/Profile.cpp:
+        (KJS::Profile::Profile): Initialize m_stoppedCallDepth to zero.
+        (KJS::Profile::stopProfiling): Set the current node to the parent,
+        because we are in a call that will not get a didExecute call.
+        (KJS::Profile::removeProfile): Increment m_stoppedCallDepth to
+        account for didExecute not being called for profile.
+        (KJS::Profile::willExecute): Increment m_stoppedCallDepth if stopped.
+        (KJS::Profile::didExecute): Decrement m_stoppedCallDepth if stopped and
+        greater than zero, and return early.
+        * profiler/Profile.h: Added stoppedProfiling().
+        * profiler/Profiler.cpp:
+        (KJS::Profiler::findProfile): Removed.
+        (KJS::Profiler::startProfiling): Don't return early for stopped profiles.
+        (KJS::Profiler::stopProfiling): Skipp stopped profiles.
+        (KJS::Profiler::didFinishAllExecution): Code clean-up.
+        * profiler/Profiler.h: Removed findProfile.
+
 2008-06-25  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
 
         Reviewed by Alexey Proskuryakov.
index 7a62025..1ec5c46 100644 (file)
@@ -46,6 +46,7 @@ Profile::Profile(const UString& title, ExecState* originatingGlobalExec, unsigne
     : m_title(title)
     , m_originatingGlobalExec(originatingGlobalExec)
     , m_pageGroupIdentifier(pageGroupIdentifier)
+    , m_stoppedCallDepth(0)
     , m_client(client)
     , m_stoppedProfiling(false)
 {
@@ -61,10 +62,11 @@ void Profile::stopProfiling()
     removeProfileStart();
     removeProfileEnd();
 
-    // Already at the head, set m_currentNode to prevent
-    // didExecute from recording more nodes.
-    if (m_currentNode == m_head)
-        m_currentNode = 0;
+    ASSERT(m_currentNode);
+
+    // Set the current node to the parent, because we are in a call that
+    // will not get didExecute call.
+    m_currentNode = m_currentNode->parent();
 
     m_stoppedProfiling = true;
 }
@@ -99,6 +101,9 @@ void Profile::removeProfileStart() {
     if (currentNode->callIdentifier().name != "profile")
         return;
 
+    // Increment m_stoppedCallDepth to account for didExecute not being called for console.profile.
+    ++m_stoppedCallDepth;
+
     // Attribute the time of the node aobut to be removed to the self time of its parent
     currentNode->parent()->setSelfTime(currentNode->parent()->selfTime() + currentNode->totalTime());
 
@@ -124,8 +129,10 @@ void Profile::removeProfileEnd() {
 
 void Profile::willExecute(const CallIdentifier& callIdentifier)
 {
-    if (m_stoppedProfiling)
+    if (m_stoppedProfiling) {
+        ++m_stoppedCallDepth;
         return;
+    }
 
     ASSERT(m_currentNode);
     m_currentNode = m_currentNode->willExecute(callIdentifier);
@@ -136,6 +143,11 @@ void Profile::didExecute(const CallIdentifier& callIdentifier)
     if (!m_currentNode)
         return;
 
+    if (m_stoppedProfiling && m_stoppedCallDepth > 0) {
+        --m_stoppedCallDepth;
+        return;
+    }
+
     if (m_currentNode == m_head) {
         m_currentNode = ProfileNode::create(callIdentifier, m_head.get(), m_head.get());
         m_currentNode->setStartTime(m_head->startTime());
@@ -153,12 +165,9 @@ void Profile::didExecute(const CallIdentifier& callIdentifier)
         return;
     }
 
-    if (m_stoppedProfiling) {
-        m_currentNode = m_currentNode->parent();
-        return;
-    }
-
-    m_currentNode = m_currentNode->didExecute();
+    // Set m_currentNode to the parent (which didExecute returns). If stopped, just set the
+    // m_currentNode to the parent and don't call didExecute.
+    m_currentNode = m_stoppedProfiling ? m_currentNode->parent() : m_currentNode->didExecute();
 }
 
 void Profile::forEach(void (ProfileNode::*function)())
index 64a6004..92e7066 100644 (file)
@@ -67,6 +67,8 @@ namespace KJS {
         void exclude(const ProfileNode* profileNode);
         void restoreAll();
 
+        bool stoppedProfiling() { return m_stoppedProfiling; }
+
 #ifndef NDEBUG
         void debugPrintData() const;
         void debugPrintDataSampleStyle() const;
@@ -75,12 +77,14 @@ namespace KJS {
 
     private:
         Profile(const UString& title, ExecState* originatingGlobalExec, unsigned pageGroupIdentifier, ProfilerClient*);
+
         void removeProfileStart();
         void removeProfileEnd();
 
         UString m_title;
         ExecState* m_originatingGlobalExec;
         unsigned m_pageGroupIdentifier;
+        unsigned m_stoppedCallDepth;
         RefPtr<ProfileNode> m_head;
         RefPtr<ProfileNode> m_currentNode;
         ProfilerClient* m_client;
index 2962f61..b92b140 100644 (file)
@@ -55,15 +55,6 @@ Profiler* Profiler::profiler()
         s_sharedProfiler = new Profiler();
     return s_sharedProfiler;
 }   
-    
-Profile* Profiler::findProfile(ExecState* exec, const UString& title) const
-{
-    ExecState* globalExec = exec->lexicalGlobalObject()->globalExec();
-    for (size_t i = 0; i < m_currentProfiles.size(); ++i)
-        if (m_currentProfiles[i]->originatingGlobalExec() == globalExec && (title.isNull() || m_currentProfiles[i]->title() == title))
-            return m_currentProfiles[i].get();
-    return 0;
-}
 
 void Profiler::startProfiling(ExecState* exec, const UString& title, ProfilerClient* client)
 {
@@ -72,9 +63,12 @@ void Profiler::startProfiling(ExecState* exec, const UString& title, ProfilerCli
     // Check if we currently have a Profile for this global ExecState and title.
     // If so return early and don't create a new Profile.
     ExecState* globalExec = exec->lexicalGlobalObject()->globalExec();
-    for (size_t i = 0; i < m_currentProfiles.size(); ++i)
-        if (m_currentProfiles[i]->originatingGlobalExec() == globalExec && m_currentProfiles[i]->title() == title)
+    for (size_t i = 0; i < m_currentProfiles.size(); ++i) {
+        Profile* profile = m_currentProfiles[i].get();
+        if (!profile->stoppedProfiling() && profile->originatingGlobalExec() == globalExec && profile->title() == title)
             return;
+    }
+
     s_sharedEnabledProfilerReference = this;
     RefPtr<Profile> profile = Profile::create(title, globalExec, exec->lexicalGlobalObject()->pageGroupIdentifier(), client);
     m_currentProfiles.append(profile);
@@ -84,7 +78,8 @@ void Profiler::stopProfiling(ExecState* exec, const UString& title)
 {
     ExecState* globalExec = exec->lexicalGlobalObject()->globalExec();
     for (ptrdiff_t i = m_currentProfiles.size() - 1; i >= 0; --i) {
-        if (m_currentProfiles[i]->originatingGlobalExec() == globalExec && (title.isNull() || m_currentProfiles[i]->title() == title))
+        Profile* profile = m_currentProfiles[i].get();
+        if (!profile->stoppedProfiling() && profile->originatingGlobalExec() == globalExec && (title.isNull() || profile->title() == title))
             m_currentProfiles[i]->stopProfiling();
     }
 }
@@ -93,7 +88,8 @@ void Profiler::didFinishAllExecution(ExecState* exec)
 {
     ExecState* globalExec = exec->lexicalGlobalObject()->globalExec();
     for (ptrdiff_t i = m_currentProfiles.size() - 1; i >= 0; --i) {
-        if (m_currentProfiles[i]->originatingGlobalExec() == globalExec && m_currentProfiles[i]->didFinishAllExecution()) {
+        Profile* profile = m_currentProfiles[i].get();
+        if (profile->originatingGlobalExec() == globalExec && profile->didFinishAllExecution()) {
             PassRefPtr<Profile> prpProfile = m_currentProfiles[i].release();        
             m_currentProfiles.remove(i);
 
index 99b9f06..266ef8e 100644 (file)
@@ -50,8 +50,6 @@ namespace KJS {
         void stopProfiling(ExecState*, const UString& title);
         void didFinishAllExecution(ExecState*);
 
-        Profile* findProfile(ExecState*, const UString& title) const;
-
         void willExecute(ExecState* exec, JSObject* calledFunction);
         void willExecute(ExecState* exec, const UString& sourceURL, int startingLineNumber);
         void didExecute(ExecState* exec, JSObject* calledFunction);
index f4e491b..68fc3e2 100644 (file)
@@ -33,4 +33,4 @@ the profile.  It should not crash or hang and there should be multiple runs of t
 same named profile.
 <div id="output"></div>
 </body>
-</html>
\ No newline at end of file
+</html>
diff --git a/WebCore/manual-tests/inspector/profiler-test-stop-then-function-call.html b/WebCore/manual-tests/inspector/profiler-test-stop-then-function-call.html
new file mode 100644 (file)
index 0000000..10f9ad0
--- /dev/null
@@ -0,0 +1,29 @@
+<html>
+<head>
+<script>
+var j = 0;
+function test(len) {
+    for (var i = 0; i < len; ++i)
+        ++j;
+}
+
+function test2(len) {
+    for (var i = 0; i < len; ++i)
+        --j;
+}
+
+console.profile("Test");
+test(1000);
+console.profileEnd("Test");
+test2(1000);
+</script>
+</head>
+
+<body>
+This page has an anonymous JavaScript function that calls built-in functions.
+<br>
+<br>
+To use this test, load it in the browser then load the WebInspector and look at
+the profile. There should be two nodes in the profile, a "(program)" node with one child that is "test".
+</body>
+</html>