Bug 1675409 - Migrated IPCMarkerPayload to Markers 2.0 API - r=gregtatum

Differential Revision: https://phabricator.services.mozilla.com/D96050
iaxat · Nov 17, 2020 · be0b6e0 · be0b6e0
1 parent b071503
commit be0b6e0
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 110 deletions.
diff --git a/ipc/chromium/src/chrome/common/ipc_channel_utils.cc b/ipc/chromium/src/chrome/common/ipc_channel_utils.cc
@@ -8,10 +8,6 @@
 
 #include "chrome/common/ipc_message.h"
 
-#ifdef MOZ_GECKO_PROFILER
-#  include "ProfilerMarkerPayload.h"
-#endif
-
 namespace IPC {
 
 void AddIPCProfilerMarker(const Message& aMessage, int32_t aOtherPid,
@@ -25,11 +21,12 @@ void AddIPCProfilerMarker(const Message& aMessage, int32_t aOtherPid,
       return;
     }
 
-    PROFILER_ADD_MARKER_WITH_PAYLOAD(
-        "IPC", IPC, IPCMarkerPayload,
-        (aOtherPid, aMessage.seqno(), aMessage.type(),
-         mozilla::ipc::UnknownSide, aDirection, aPhase, aMessage.is_sync(),
-         mozilla::TimeStamp::NowUnfuzzed()));
+    // The current timestamp must be given to the `IPCMarker` payload.
+    const mozilla::TimeStamp now = mozilla::TimeStamp::NowUnfuzzed();
+    PROFILER_MARKER("IPC", IPC, mozilla::MarkerTiming::InstantAt(now),
+                    IPCMarker, now, now, aOtherPid, aMessage.seqno(),
+                    aMessage.type(), mozilla::ipc::UnknownSide, aDirection,
+                    aPhase, aMessage.is_sync());
   }
 #endif
 }

diff --git a/ipc/glue/MessageChannel.cpp b/ipc/glue/MessageChannel.cpp
@@ -37,10 +37,6 @@
 using namespace mozilla::tasktracer;
 #endif
 
-#ifdef MOZ_GECKO_PROFILER
-#  include "ProfilerMarkerPayload.h"
-#endif
-
 // Undo the damage done by mozzconf.h
 #undef compress
 
@@ -2789,11 +2785,11 @@ void MessageChannel::AddProfilerMarker(const IPC::Message& aMessage,
   if (profiler_feature_active(ProfilerFeature::IPCMessages)) {
     int32_t pid = mListener->OtherPidMaybeInvalid();
     if (pid != kInvalidProcessId) {
-      PROFILER_ADD_MARKER_WITH_PAYLOAD(
-          "IPC", IPC, IPCMarkerPayload,
-          (pid, aMessage.seqno(), aMessage.type(), mSide, aDirection,
-           MessagePhase::Endpoint, aMessage.is_sync(),
-           TimeStamp::NowUnfuzzed()));
+      // The current timestamp must be given to the `IPCMarker` payload.
+      const TimeStamp now = TimeStamp::NowUnfuzzed();
+      PROFILER_MARKER("IPC", IPC, MarkerTiming::InstantAt(now), IPCMarker, now,
+                      now, pid, aMessage.seqno(), aMessage.type(), mSide,
+                      aDirection, MessagePhase::Endpoint, aMessage.is_sync());
     }
   }
 #endif

diff --git a/ipc/glue/MessageChannel.h b/ipc/glue/MessageChannel.h
@@ -29,6 +29,10 @@
 #include "MessageLink.h"
 #include "mozilla/ipc/Transport.h"
 
+#ifdef MOZ_GECKO_PROFILER
+#  include "mozilla/ProfilerMarkersPrerequisites.h"
+#endif
+
 class nsIEventTarget;
 
 namespace mozilla {
@@ -879,4 +883,74 @@ struct ParamTraits<mozilla::ipc::ResponseRejectReason>
           mozilla::ipc::ResponseRejectReason::EndGuard_> {};
 }  // namespace IPC
 
+#ifdef MOZ_GECKO_PROFILER
+namespace geckoprofiler::markers {
+
+struct IPCMarker {
+  static constexpr mozilla::Span<const char> MarkerTypeName() {
+    return mozilla::MakeStringSpan("IPC");
+  }
+  static void StreamJSONMarkerData(
+      mozilla::baseprofiler::SpliceableJSONWriter& aWriter,
+      mozilla::TimeStamp aStart, mozilla::TimeStamp aEnd, int32_t aOtherPid,
+      int32_t aMessageSeqno, IPC::Message::msgid_t aMessageType,
+      mozilla::ipc::Side aSide, mozilla::ipc::MessageDirection aDirection,
+      mozilla::ipc::MessagePhase aPhase, bool aSync) {
+    using namespace mozilla::ipc;
+    // This payload still streams a startTime and endTime property because it
+    // made the migration to MarkerTiming on the front-end easier.
+    mozilla::baseprofiler::WritePropertyTime(aWriter, "startTime", aStart);
+    mozilla::baseprofiler::WritePropertyTime(aWriter, "endTime", aEnd);
+
+    aWriter.IntProperty("otherPid", aOtherPid);
+    aWriter.IntProperty("messageSeqno", aMessageSeqno);
+    aWriter.StringProperty(
+        "messageType",
+        mozilla::MakeStringSpan(IPC::StringFromIPCMessageType(aMessageType)));
+    aWriter.StringProperty("side", IPCSideToString(aSide));
+    aWriter.StringProperty("direction",
+                           aDirection == MessageDirection::eSending
+                               ? mozilla::MakeStringSpan("sending")
+                               : mozilla::MakeStringSpan("receiving"));
+    aWriter.StringProperty("phase", IPCPhaseToString(aPhase));
+    aWriter.BoolProperty("sync", aSync);
+  }
+  static mozilla::MarkerSchema MarkerTypeDisplay() {
+    return mozilla::MarkerSchema::SpecialFrontendLocation{};
+  }
+
+ private:
+  static mozilla::Span<const char> IPCSideToString(mozilla::ipc::Side aSide) {
+    switch (aSide) {
+      case mozilla::ipc::ParentSide:
+        return mozilla::MakeStringSpan("parent");
+      case mozilla::ipc::ChildSide:
+        return mozilla::MakeStringSpan("child");
+      case mozilla::ipc::UnknownSide:
+        return mozilla::MakeStringSpan("unknown");
+      default:
+        MOZ_ASSERT_UNREACHABLE("Invalid IPC side");
+        return mozilla::MakeStringSpan("<invalid IPC side>");
+    }
+  }
+
+  static mozilla::Span<const char> IPCPhaseToString(
+      mozilla::ipc::MessagePhase aPhase) {
+    switch (aPhase) {
+      case mozilla::ipc::MessagePhase::Endpoint:
+        return mozilla::MakeStringSpan("endpoint");
+      case mozilla::ipc::MessagePhase::TransferStart:
+        return mozilla::MakeStringSpan("transferStart");
+      case mozilla::ipc::MessagePhase::TransferEnd:
+        return mozilla::MakeStringSpan("transferEnd");
+      default:
+        MOZ_ASSERT_UNREACHABLE("Invalid IPC phase");
+        return mozilla::MakeStringSpan("<invalid IPC phase>");
+    }
+  }
+};
+
+}  // namespace geckoprofiler::markers
+#endif
+
 #endif  // ifndef ipc_glue_MessageChannel_h
diff --git a/tools/profiler/public/ProfilerMarkerTypes.h b/tools/profiler/public/ProfilerMarkerTypes.h
@@ -25,7 +25,6 @@
 
 #  include "js/ProfilingFrameIterator.h"
 #  include "js/Utility.h"
-#  include "mozilla/ipc/ProtocolUtils.h"
 #  include "mozilla/Preferences.h"
 #  include "mozilla/ServoTraversalStatistics.h"
 
@@ -38,69 +37,6 @@ using UserTimingMeasure = mozilla::baseprofiler::markers::UserTimingMeasure;
 using MediaSampleMarker = mozilla::baseprofiler::markers::MediaSampleMarker;
 using ContentBuildMarker = mozilla::baseprofiler::markers::ContentBuildMarker;
 
-struct IPCMarkerPayload {
-  static constexpr mozilla::Span<const char> MarkerTypeName() {
-    return mozilla::MakeStringSpan("IPC");
-  }
-  static void StreamJSONMarkerData(
-      mozilla::baseprofiler::SpliceableJSONWriter& aWriter, int32_t aOtherPid,
-      int32_t aMessageSeqno, IPC::Message::msgid_t aMessageType,
-      mozilla::ipc::Side aSide, mozilla::ipc::MessageDirection aDirection,
-      mozilla::ipc::MessagePhase aPhase, bool aSync,
-      const mozilla::TimeStamp& aTime) {
-    // TODO: Remove these Legacy times when frontend is updated.
-    mozilla::baseprofiler::WritePropertyTime(aWriter, "startTime", aTime);
-    mozilla::baseprofiler::WritePropertyTime(aWriter, "endTime", aTime);
-
-    using namespace mozilla::ipc;
-    aWriter.IntProperty("otherPid", aOtherPid);
-    aWriter.IntProperty("messageSeqno", aMessageSeqno);
-    aWriter.StringProperty(
-        "messageType",
-        mozilla::MakeStringSpan(IPC::StringFromIPCMessageType(aMessageType)));
-    aWriter.StringProperty("side", IPCSideToString(aSide));
-    aWriter.StringProperty("direction",
-                           aDirection == MessageDirection::eSending
-                               ? mozilla::MakeStringSpan("sending")
-                               : mozilla::MakeStringSpan("receiving"));
-    aWriter.StringProperty("phase", IPCPhaseToString(aPhase));
-    aWriter.BoolProperty("sync", aSync);
-  }
-  static mozilla::MarkerSchema MarkerTypeDisplay() {
-    return mozilla::MarkerSchema::SpecialFrontendLocation{};
-  }
-
- private:
-  static mozilla::Span<const char> IPCSideToString(mozilla::ipc::Side aSide) {
-    switch (aSide) {
-      case mozilla::ipc::ParentSide:
-        return mozilla::MakeStringSpan("parent");
-      case mozilla::ipc::ChildSide:
-        return mozilla::MakeStringSpan("child");
-      case mozilla::ipc::UnknownSide:
-        return mozilla::MakeStringSpan("unknown");
-      default:
-        MOZ_ASSERT_UNREACHABLE("Invalid IPC side");
-        return mozilla::MakeStringSpan("<invalid IPC side>");
-    }
-  }
-
-  static mozilla::Span<const char> IPCPhaseToString(
-      mozilla::ipc::MessagePhase aPhase) {
-    switch (aPhase) {
-      case mozilla::ipc::MessagePhase::Endpoint:
-        return mozilla::MakeStringSpan("endpoint");
-      case mozilla::ipc::MessagePhase::TransferStart:
-        return mozilla::MakeStringSpan("transferStart");
-      case mozilla::ipc::MessagePhase::TransferEnd:
-        return mozilla::MakeStringSpan("transferEnd");
-      default:
-        MOZ_ASSERT_UNREACHABLE("Invalid IPC phase");
-        return mozilla::MakeStringSpan("<invalid IPC phase>");
-    }
-  }
-};
-
 }  // namespace geckoprofiler::markers
 
 #endif  // MOZ_GECKO_PROFILER

diff --git a/tools/profiler/tests/gtest/GeckoProfiler.cpp b/tools/profiler/tests/gtest/GeckoProfiler.cpp
@@ -844,12 +844,6 @@ TEST(GeckoProfiler, Markers)
       (u"measure name"_ns, Some(u"start mark"_ns), Some(u"end mark"_ns), ts1,
        ts2, mozilla::Nothing()));
 
-  PROFILER_ADD_MARKER_WITH_PAYLOAD(
-      "IPCMarkerPayload marker", IPC, IPCMarkerPayload,
-      (1111, 1, 3 /* PAPZ::Msg_LayerTransforms */, mozilla::ipc::ParentSide,
-       mozilla::ipc::MessageDirection::eSending,
-       mozilla::ipc::MessagePhase::Endpoint, false, ts1));
-
   MOZ_RELEASE_ASSERT(profiler_add_marker(
       "Text in main thread with stack", geckoprofiler::category::OTHER,
       MarkerStack::Capture(), geckoprofiler::markers::Text{}, ""));
@@ -961,7 +955,6 @@ TEST(GeckoProfiler, Markers)
     S_TextMarkerPayload2,
     S_UserTimingMarkerPayload_mark,
     S_UserTimingMarkerPayload_measure,
-    S_IPCMarkerPayload,
     S_TextWithStack,
     S_TextToMTWithStack,
     S_RegThread_TextToMTWithStack,
@@ -1384,27 +1377,6 @@ TEST(GeckoProfiler, Markers)
                 EXPECT_EQ_JSON(payload["startMark"], String, "start mark");
                 EXPECT_EQ_JSON(payload["endMark"], String, "end mark");
 
-              } else if (nameString == "IPCMarkerPayload marker") {
-                EXPECT_EQ(state, S_IPCMarkerPayload);
-                state = State(S_IPCMarkerPayload + 1);
-                EXPECT_EQ(typeString, "IPC");
-                EXPECT_TIMING_INSTANT_AT(ts1Double);
-
-                // The startTime and endTime are currently duplicated in the
-                // payload.
-                EXPECT_EQ_JSON(payload["startTime"], Double, ts1Double);
-                EXPECT_EQ_JSON(payload["endTime"], Double, ts1Double);
-
-                EXPECT_TRUE(payload["stack"].isNull());
-                EXPECT_EQ_JSON(payload["otherPid"], Int, 1111);
-                EXPECT_EQ_JSON(payload["messageSeqno"], Int, 1);
-                EXPECT_EQ_JSON(payload["messageType"], String,
-                               "PAPZ::Msg_LayerTransforms");
-                EXPECT_EQ_JSON(payload["side"], String, "parent");
-                EXPECT_EQ_JSON(payload["direction"], String, "sending");
-                EXPECT_EQ_JSON(payload["phase"], String, "endpoint");
-                EXPECT_EQ_JSON(payload["sync"], Bool, false);
-
               } else if (nameString == "Text in main thread with stack") {
                 EXPECT_EQ(state, S_TextWithStack);
                 state = State(S_TextWithStack + 1);