Add AsyncEvent::finish() to end a nested AsyncEvent before its parent ends

Add unit testing of that, too.

Change-Id: Iae5fb6da0b7fcabe8f555d800f065b6f5b4b9982
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114800
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>

2 files changed, 76 insertions, 35 deletions

diff --git a/comphelper/qa/unit/test_traceevent.cxx b/comphelper/qa/unit/test_traceevent.cxx
index 11e1f0b3a2e8..226171b0e097 100644
--- a/comphelper/qa/unit/test_traceevent.cxx
+++ b/comphelper/qa/unit/test_traceevent.cxx
@@ -31,66 +31,87 @@ namespace
 {
 void trace_event_test()
 {
-    // When we start recording is off and this will not generate any 'X' event
-    comphelper::ProfileZone aZone0("test().0");
+    // When we start recording is off and this will not generate any 'X' event when we leave the scope
+    comphelper::ProfileZone aZone0("test0");
 
     // This will not generate any 'b' and 'e' events either
-    auto pAsync1(std::make_shared<comphelper::AsyncEvent>("async 1"));
+    auto pAsync1(std::make_shared<comphelper::AsyncEvent>("async1"));
 
     std::weak_ptr<comphelper::AsyncEvent> pAsync2;
     {
         // No 'X' by this either
-        comphelper::ProfileZone aZone1("block 1");
+        comphelper::ProfileZone aZone1("block1");
 
         // Now we turn on recording
         comphelper::TraceEvent::startRecording();
 
         // As this is nested in the parent that was created with recording turned off,
-        // this will not generate any 'b' and 'e' events either
-        pAsync2 = comphelper::AsyncEvent::createWithParent("async 2", pAsync1);
+        // this will not generate any 'b' and 'e' events either even if recording is now on.
+        pAsync2 = comphelper::AsyncEvent::createWithParent("async2in1", pAsync1);
     }
 
-    // This will generate an 'i' event
-    comphelper::TraceEvent::addInstantEvent("instant 1");
+    // This will generate an 'i' event for instant1
+    comphelper::TraceEvent::addInstantEvent("instant1");
 
     std::shared_ptr<comphelper::AsyncEvent> pAsync25;
     {
-        comphelper::ProfileZone aZone2("block 2");
+        comphelper::ProfileZone aZone2("block2");
 
         // This does not generate any 'e' event as it was created when recording was off
-        // And the nested "async 2" object will thus not generate anything either
+        // And the nested async2 object will thus not generate anything either
         pAsync1.reset();
 
         // This will generate 'b' event and an 'e' event when the pointer is reset or goes out of scope
-        pAsync25 = std::make_shared<comphelper::AsyncEvent>("async 2.5");
+        pAsync25 = std::make_shared<comphelper::AsyncEvent>("async2.5");
 
-        // Leaving this scope will generate an 'X' event for "block 2"
+        // Leaving this scope will generate an 'X' event for block2
     }
 
     // Verify that the weak_ptr to pAsync2 has expired as its parent pAsync1 has been finished off
     CPPUNIT_ASSERT(pAsync2.expired());
 
-    // This will generate a 'b' event
-    auto pAsync3(std::make_shared<comphelper::AsyncEvent>("async 3"));
+    // This will generate a 'b' event for async3
+    auto pAsync3(std::make_shared<comphelper::AsyncEvent>("async3"));
 
     std::weak_ptr<comphelper::AsyncEvent> pAsync4;
+
     {
-        comphelper::ProfileZone aZone3("block 3");
+        comphelper::ProfileZone aZone3("block3");
 
-        pAsync4 = comphelper::AsyncEvent::createWithParent("async 4", pAsync3);
+        pAsync4 = comphelper::AsyncEvent::createWithParent("async4in3", pAsync3);
 
-        // Leaving this scope will generate an 'X' event for "block 3"
+        // Leaving this scope will generate an 'X' event for block3
     }
 
-    // This will generate an 'e' event for "async 2.5"
+    // This will generate an 'e' event for async2.5
     pAsync25.reset();
 
-    comphelper::ProfileZone aZone4("test().2");
+    comphelper::ProfileZone aZone4("test2");
+
+    // This will generate an 'i' event for instant2"
+    comphelper::TraceEvent::addInstantEvent("instant2");
+
+    std::weak_ptr<comphelper::AsyncEvent> pAsync5;
+    {
+        auto pAsync4Locked = pAsync4.lock();
+        CPPUNIT_ASSERT(pAsync4Locked);
+        // This will generate a 'b' event for async5in4
+        pAsync5 = comphelper::AsyncEvent::createWithParent("async5in4", pAsync4Locked);
+    }
+
+    CPPUNIT_ASSERT(!pAsync5.expired());
+
+    // This will generate a 'b' event for async6in5
+    std::weak_ptr<comphelper::AsyncEvent> pAsync6(
+        comphelper::AsyncEvent::createWithParent("async6in5", pAsync5.lock()));
+    CPPUNIT_ASSERT(!pAsync6.expired());
+
+    // This will generate an 'e' event for async6in5 and async5in4
+    pAsync5.lock()->finish();
 
-    // This will generate an 'i' event
-    comphelper::TraceEvent::addInstantEvent("instant 2");
+    CPPUNIT_ASSERT(pAsync6.expired());
 
-    // Leaving this scope will generate 'X' events for "test().2" and a 'e' event for pAsync4 and pAsync3
+    // Leaving this scope will generate 'X' events for test2 and a 'e' event for async4in3 and async3
 }
 }
 
@@ -103,17 +124,23 @@ void TestTraceEvent::test()
         std::cerr << s << "\n";
     }
 
-    CPPUNIT_ASSERT(aEvents[0].startsWith("{\"name:\"instant 1\",\"ph\":\"i\","));
-    CPPUNIT_ASSERT(aEvents[1].startsWith("{\"name\":\"async 2.5\",\"ph\":\"b\",\"id\":1,"));
-    CPPUNIT_ASSERT(aEvents[2].startsWith("{\"name\":\"block 2\",\"ph\":\"X\","));
-    CPPUNIT_ASSERT(aEvents[3].startsWith("{\"name\":\"async 3\",\"ph\":\"b\",\"id\":2,"));
-    CPPUNIT_ASSERT(aEvents[4].startsWith("{\"name\":\"async 4\",\"ph\":\"b\",\"id\":2,"));
-    CPPUNIT_ASSERT(aEvents[5].startsWith("{\"name\":\"block 3\",\"ph\":\"X\","));
-    CPPUNIT_ASSERT(aEvents[6].startsWith("{\"name\":\"async 2.5\",\"ph\":\"e\",\"id\":1,"));
-    CPPUNIT_ASSERT(aEvents[7].startsWith("{\"name:\"instant 2\",\"ph\":\"i\","));
-    CPPUNIT_ASSERT(aEvents[8].startsWith("{\"name\":\"test().2\",\"ph\":\"X\""));
-    CPPUNIT_ASSERT(aEvents[9].startsWith("{\"name\":\"async 4\",\"ph\":\"e\",\"id\":2,"));
-    CPPUNIT_ASSERT(aEvents[10].startsWith("{\"name\":\"async 3\",\"ph\":\"e\",\"id\":2,"));
+    CPPUNIT_ASSERT_EQUAL(15, static_cast<int>(aEvents.size()));
+
+    CPPUNIT_ASSERT(aEvents[0].startsWith("{\"name:\"instant1\",\"ph\":\"i\","));
+    CPPUNIT_ASSERT(aEvents[1].startsWith("{\"name\":\"async2.5\",\"ph\":\"b\",\"id\":1,"));
+    CPPUNIT_ASSERT(aEvents[2].startsWith("{\"name\":\"block2\",\"ph\":\"X\","));
+    CPPUNIT_ASSERT(aEvents[3].startsWith("{\"name\":\"async3\",\"ph\":\"b\",\"id\":2,"));
+    CPPUNIT_ASSERT(aEvents[4].startsWith("{\"name\":\"async4in3\",\"ph\":\"b\",\"id\":2,"));
+    CPPUNIT_ASSERT(aEvents[5].startsWith("{\"name\":\"block3\",\"ph\":\"X\","));
+    CPPUNIT_ASSERT(aEvents[6].startsWith("{\"name\":\"async2.5\",\"ph\":\"e\",\"id\":1,"));
+    CPPUNIT_ASSERT(aEvents[7].startsWith("{\"name:\"instant2\",\"ph\":\"i\","));
+    CPPUNIT_ASSERT(aEvents[8].startsWith("{\"name\":\"async5in4\",\"ph\":\"b\",\"id\":2,"));
+    CPPUNIT_ASSERT(aEvents[9].startsWith("{\"name\":\"async6in5\",\"ph\":\"b\",\"id\":2,"));
+    CPPUNIT_ASSERT(aEvents[10].startsWith("{\"name\":\"async6in5\",\"ph\":\"e\",\"id\":2,"));
+    CPPUNIT_ASSERT(aEvents[11].startsWith("{\"name\":\"async5in4\",\"ph\":\"e\",\"id\":2,"));
+    CPPUNIT_ASSERT(aEvents[12].startsWith("{\"name\":\"test2\",\"ph\":\"X\""));
+    CPPUNIT_ASSERT(aEvents[13].startsWith("{\"name\":\"async4in3\",\"ph\":\"e\",\"id\":2,"));
+    CPPUNIT_ASSERT(aEvents[14].startsWith("{\"name\":\"async3\",\"ph\":\"e\",\"id\":2,"));
 }
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestTraceEvent);
diff --git a/include/comphelper/traceevent.hxx b/include/comphelper/traceevent.hxx
index 33eaf04c7047..eed67c0c6f9b 100644
--- a/include/comphelper/traceevent.hxx
+++ b/include/comphelper/traceevent.hxx
@@ -14,6 +14,7 @@
 
 #include <atomic>
 #include <memory>
+#include <set>
 #include <vector>
 
 #include <osl/process.h>
@@ -81,7 +82,8 @@ class COMPHELPER_DLLPUBLIC AsyncEvent : public NamedEvent,
     static int s_nIdCounter;
     int m_nId;
     int m_nPid;
-    std::vector<std::shared_ptr<AsyncEvent>> m_aChildren;
+    std::set<std::shared_ptr<AsyncEvent>> m_aChildren;
+    std::weak_ptr<AsyncEvent> m_pParent;
     bool m_bBeginRecorded;
 
     AsyncEvent(const char* sName, int nId)
@@ -159,11 +161,23 @@ public:
         if (s_bRecording && pParent->m_bBeginRecorded)
         {
             pResult.reset(new AsyncEvent(sName, pParent->m_nId));
-            pParent->m_aChildren.push_back(pResult);
+            pParent->m_aChildren.insert(pResult);
+            pResult->m_pParent = pParent;
         }
 
         return pResult;
     }
+
+    void finish()
+    {
+        // This makes sense to call only for a nested AsyncEvent. To finish up a non-nested AsyncEvent you
+        // just need to release your sole owning pointer to it.
+        auto pParent = m_pParent.lock();
+        if (!pParent)
+            return;
+        pParent->m_aChildren.erase(shared_from_this());
+        m_aChildren.clear();
+    }
 };
 
 } // namespace comphelper