[Libreoffice-commits] core.git: Branch 'distro/collabora/cp-6.4' - comphelper/qa include/comphelper

Tor Lillqvist (via logerrit) logerrit at kemper.freedesktop.org
Thu Jun 24 07:31:30 UTC 2021


 comphelper/qa/unit/test_traceevent.cxx |   67 ++++--------------------------
 include/comphelper/traceevent.hxx      |   72 +++++----------------------------
 2 files changed, 22 insertions(+), 117 deletions(-)

New commits:
commit a2641e3856b72d194d39d88013dad2d6f3c4c9d3
Author:     Tor Lillqvist <tml at collabora.com>
AuthorDate: Wed Jun 23 13:05:36 2021 +0300
Commit:     Tor Lillqvist <tml at collabora.com>
CommitDate: Thu Jun 24 09:30:52 2021 +0200

    The Chrome Trace Event viewer doesn't support nested async events
    
    Instead of nestable b(egin) and e(nd) events, generate the allegedly
    deprecated S(tart)' and F(inish) events.
    
    Makes the code simpler. (And AsyncEvent is still unused. Unclear
    whether adding it was based on a misunderstanding.)
    
    Change-Id: Ic029b67e0951dda775c0d0af009f2e431ae55e53
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117700
    Tested-by: Jenkins
    Reviewed-by: Tor Lillqvist <tml at collabora.com>
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117737
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>

diff --git a/comphelper/qa/unit/test_traceevent.cxx b/comphelper/qa/unit/test_traceevent.cxx
index 8bd34595e397..6f768c09d466 100644
--- a/comphelper/qa/unit/test_traceevent.cxx
+++ b/comphelper/qa/unit/test_traceevent.cxx
@@ -40,17 +40,12 @@ void trace_event_test()
         // This will not generate any 'b' and 'e' events either
         auto pAsync1(std::make_shared<comphelper::AsyncEvent>("async1"));
 
-        std::weak_ptr<comphelper::AsyncEvent> pAsync2;
         {
             // No 'X' by this either
             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 even if recording is now on.
-            pAsync2 = comphelper::AsyncEvent::createWithParent("async2in1", pAsync1);
         }
 
         // This will generate an 'i' event for instant1
@@ -70,20 +65,13 @@ void trace_event_test()
             // 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 for async3
         auto pAsync3(std::make_shared<comphelper::AsyncEvent>(
             "async3", std::map<OUString, OUString>({ { "foo", "bar" }, { "tem", "42" } })));
 
-        std::weak_ptr<comphelper::AsyncEvent> pAsync4;
-
         {
             comphelper::ProfileZone aZone3("block3");
 
-            pAsync4 = comphelper::AsyncEvent::createWithParent("async4in3", pAsync3);
-
             // Leaving this scope will generate an 'X' event for block3
         }
 
@@ -96,32 +84,8 @@ void trace_event_test()
         comphelper::TraceEvent::addInstantEvent(
             "instant2", std::map<OUString, OUString>({ { "foo2", "bar2" }, { "tem2", "42" } }));
 
-        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();
-
-        pAsync7Locked = comphelper::AsyncEvent::createWithParent("async7in3", pAsync3).lock();
-
-        CPPUNIT_ASSERT(pAsync6.expired());
-
         // Leaving this scope will generate 'X' events for test2 and a
-        // 'e' event for async4in3, async7in3, and async3. The
-        // pAsync7Locked pointer will now point to an AsyncEvent
-        // object that has already had its 'e' event generated.
+        // 'e' event for async4in3, async7in3, and async3.
     }
 
     // This incorrect use of overlapping (not nested) ProfileZones
@@ -130,9 +94,6 @@ void trace_event_test()
     auto p2 = new comphelper::ProfileZone("error2");
     delete p1;
     delete p2;
-
-    // Nothing is generated from this
-    pAsync7Locked.reset();
 }
 }
 
@@ -145,28 +106,20 @@ void TestTraceEvent::test()
         std::cerr << s << "\n";
     }
 
-    CPPUNIT_ASSERT_EQUAL(17, static_cast<int>(aEvents.size()));
+    CPPUNIT_ASSERT_EQUAL(9, 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[1].startsWith("{\"name\":\"async2.5\",\"ph\":\"S\",\"id\":1,"));
     CPPUNIT_ASSERT(aEvents[2].startsWith("{\"name\":\"block2\",\"ph\":\"X\","));
     CPPUNIT_ASSERT(aEvents[3].startsWith(
-        "{\"name\":\"async3\",\"ph\":\"b\",\"id\":2,\"args\":{\"foo\":\"bar\",\"tem\":\"42\"},"));
-    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\":\"async3\",\"ph\":\"S\",\"id\":2,\"args\":{\"foo\":\"bar\",\"tem\":\"42\"},"));
+    CPPUNIT_ASSERT(aEvents[4].startsWith("{\"name\":\"block3\",\"ph\":\"X\","));
+    CPPUNIT_ASSERT(aEvents[5].startsWith("{\"name\":\"async2.5\",\"ph\":\"F\",\"id\":1,"));
+    CPPUNIT_ASSERT(aEvents[6].startsWith(
         "{\"name:\"instant2\",\"ph\":\"i\",\"args\":{\"foo2\":\"bar2\",\"tem2\":\"42\"},"));
-    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\":\"async7in3\",\"ph\":\"b\",\"id\":2,"));
-    CPPUNIT_ASSERT(aEvents[13].startsWith("{\"name\":\"test2\",\"ph\":\"X\""));
-    CPPUNIT_ASSERT(aEvents[14].startsWith("{\"name\":\"async4in3\",\"ph\":\"e\",\"id\":2,"));
-    CPPUNIT_ASSERT(aEvents[15].startsWith("{\"name\":\"async7in3\",\"ph\":\"e\",\"id\":2,"));
-    CPPUNIT_ASSERT(aEvents[16].startsWith(
-        "{\"name\":\"async3\",\"ph\":\"e\",\"id\":2,\"args\":{\"foo\":\"bar\",\"tem\":\"42\"},"));
+    CPPUNIT_ASSERT(aEvents[7].startsWith("{\"name\":\"test2\",\"ph\":\"X\""));
+    CPPUNIT_ASSERT(aEvents[8].startsWith(
+        "{\"name\":\"async3\",\"ph\":\"F\",\"id\":2,\"args\":{\"foo\":\"bar\",\"tem\":\"42\"},"));
 }
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestTraceEvent);
diff --git a/include/comphelper/traceevent.hxx b/include/comphelper/traceevent.hxx
index cc3c390d7e59..45c4d706227d 100644
--- a/include/comphelper/traceevent.hxx
+++ b/include/comphelper/traceevent.hxx
@@ -123,33 +123,21 @@ protected:
     }
 };
 
-// An AsyncEvent generates a 'b' (begin) event when constructed and an 'e' (end) event when it
-// itself or its nesting parent (if there is one) is destructed (or earlier, if requested)
+// An AsyncEvent generates an 'S' (start) event when constructed and a 'F' (finish) event when it
+// is destructed.
 
-// There are two kinds of async event pairs: Freestanding ones that are not related to other events
-// at all, and nested ones that have to be nested between the 'b' and 'e' events of a parent Async
-// event.
+// The Trace Event specification claims that these event types are deprecated and replaces by
+// nestable 'b' (begin) and 'e' (end) events, but Chrome does not seem to support those.
 
-// To generate a pair of 'b' and 'e' events, create an AsyncEvent object using the AsyncEvent(const
-// char* sName) constructor when you want the 'b' event to be generated, and destroy it when you
-// want the corresponding 'e' event to be generated.
-
-// To generate a pair of 'b' and 'e' events that is nested inside an outer 'b' and 'e' event pair,
-// create an AsyncEvent object using the createWithParent() function. It returns a weak reference
-// (weak_ptr) to the AsyncEvent. The parent keeps a strong reference (shared_ptr) to it.
-
-// The 'e' event will be generated when the parent is about to go away, before the parent's 'e'
-// event. When the parent has gone away, the weak reference will have expired. You can also generate
-// it explicitly by calling the finish() function. (But in that case you could as well have used a
-// freestanding AsyncEvent object, I think.)
+// To generate a pair of 'S' and 'F' events, create an AsyncEvent object using the AsyncEvent(const
+// char* sName) constructor when you want the 'S' event to be generated, and destroy it when you
+// want the corresponding 'F' event to be generated.
 
 class COMPHELPER_DLLPUBLIC AsyncEvent : public NamedEvent,
                                         public std::enable_shared_from_this<AsyncEvent>
 {
     static int s_nIdCounter;
     int m_nId;
-    std::vector<std::shared_ptr<AsyncEvent>> m_aChildren;
-    std::weak_ptr<AsyncEvent> m_pParent;
     bool m_bBeginRecorded;
 
     AsyncEvent(const char* sName, int nId, const std::map<OUString, OUString>& args)
@@ -161,12 +149,12 @@ class COMPHELPER_DLLPUBLIC AsyncEvent : public NamedEvent,
         {
             long long nNow = getNow();
 
-            // Generate a "Begin " (type b) event
+            // Generate a "Start" (type S) event
             TraceEvent::addRecording("{"
                                      "\"name\":\""
                                      + OUString(m_sName, strlen(m_sName), RTL_TEXTENCODING_UTF8)
                                      + "\","
-                                       "\"ph\":\"b\""
+                                       "\"ph\":\"S\""
                                        ","
                                        "\"id\":"
                                      + OUString::number(m_nId) + m_sArgs
@@ -189,21 +177,13 @@ class COMPHELPER_DLLPUBLIC AsyncEvent : public NamedEvent,
         {
             m_bBeginRecorded = false;
 
-            // In case somebody is holding on to a hard reference to a child we need to tell the
-            // children to finish up explicitly, we can't rely on our pointers to them being the
-            // only ones.
-            for (auto& i : m_aChildren)
-                i->generateEnd();
-
-            m_aChildren.clear();
-
             long long nNow = getNow();
-            // Generate a "Env " (type e) event
+            // Generate a "Finish" (type F) event
             TraceEvent::addRecording("{"
                                      "\"name\":\""
                                      + OUString(m_sName, strlen(m_sName), RTL_TEXTENCODING_UTF8)
                                      + "\","
-                                       "\"ph\":\"e\""
+                                       "\"ph\":\"F\""
                                        ","
                                        "\"id\":"
                                      + OUString::number(m_nId) + m_sArgs
@@ -228,35 +208,7 @@ public:
 
     ~AsyncEvent() { generateEnd(); }
 
-    static std::weak_ptr<AsyncEvent>
-    createWithParent(const char* sName, std::shared_ptr<AsyncEvent> pParent,
-                     const std::map<OUString, OUString>& args = std::map<OUString, OUString>())
-    {
-        std::shared_ptr<AsyncEvent> pResult;
-
-        if (s_bRecording && pParent->m_bBeginRecorded)
-        {
-            pResult.reset(new AsyncEvent(sName, pParent->m_nId, args));
-            pParent->m_aChildren.push_back(pResult);
-            pResult->m_pParent = pParent;
-        }
-
-        return pResult;
-    }
-
-    void finish()
-    {
-        generateEnd();
-
-        auto pParent = m_pParent.lock();
-        if (!pParent)
-            return;
-
-        pParent->m_aChildren.erase(std::remove(pParent->m_aChildren.begin(),
-                                               pParent->m_aChildren.end(), shared_from_this()),
-                                   pParent->m_aChildren.end());
-        m_pParent.reset();
-    }
+    void finish() { generateEnd(); }
 };
 
 } // namespace comphelper


More information about the Libreoffice-commits mailing list