Remove Commit from FrameTimingSequence (facebook#54779)

Summary:

# Changelog: [Internal]

This is not required for displaying the Frames track.

In Chrome, I've noticed that "Commit" is actually a complete event (`ph =  X`), and it has a `disabled-by-default-devtools.timeline`. Frames events, like `BeginFrame`, `DrawFrame`, `DroppedFrame` have `.frame` suffix in a category.

This event being a complete event actually makes lot of sense, since this is a work with some duration, where Host spent time committing the changes. In React Native case, this should probably reflect the `completeRoot()` call or some proxy from it. Basically the duration of committing tree operations in Fabric.

Reviewed By: sbuggay

Differential Revision: D88382285

Author: hoxyq

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/inspector/FrameTimingSequence.kt

@@ -10,8 +10,7 @@ package com.facebook.react.devsupport.inspector
 internal data class FrameTimingSequence(
     val id: Int,
     val threadId: Int,
-    val beginDrawingTimestamp: Long,
-    val commitTimestamp: Long,
-    val endDrawingTimestamp: Long,
+    val beginTimestamp: Long,
+    val endTimestamp: Long,
     val screenshot: String? = null,
 )

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/inspector/FrameTimingsObserver.kt

@@ -36,15 +36,8 @@ internal class FrameTimingsObserver(
 
   private val frameMetricsListener =
       Window.OnFrameMetricsAvailableListener { _, frameMetrics, _dropCount ->
-        val beginDrawingTimestamp = frameMetrics.getMetric(FrameMetrics.VSYNC_TIMESTAMP)
-        val commitTimestamp =
-            beginDrawingTimestamp + frameMetrics.getMetric(FrameMetrics.INPUT_HANDLING_DURATION)
-        +frameMetrics.getMetric(FrameMetrics.ANIMATION_DURATION)
-        +frameMetrics.getMetric(FrameMetrics.LAYOUT_MEASURE_DURATION)
-        +frameMetrics.getMetric(FrameMetrics.DRAW_DURATION)
-        +frameMetrics.getMetric(FrameMetrics.SYNC_DURATION)
-        val endDrawingTimestamp =
-            beginDrawingTimestamp + frameMetrics.getMetric(FrameMetrics.TOTAL_DURATION)
+        val beginTimestamp = frameMetrics.getMetric(FrameMetrics.VSYNC_TIMESTAMP)
+        val endTimestamp = beginTimestamp + frameMetrics.getMetric(FrameMetrics.TOTAL_DURATION)
 
         val frameId = frameCounter++
         val threadId = Process.myTid()
@@ -56,9 +49,8 @@ internal class FrameTimingsObserver(
               FrameTimingSequence(
                   frameId,
                   threadId,
-                  beginDrawingTimestamp,
-                  commitTimestamp,
-                  endDrawingTimestamp,
+                  beginTimestamp,
+                  endTimestamp,
                   screenshot,
               )
           )

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp

@@ -224,9 +224,8 @@ void JReactHostInspectorTarget::recordFrameTimings(
   inspectorTarget().recordFrameTimings({
       frameTimingSequence->getId(),
       frameTimingSequence->getThreadId(),
-      frameTimingSequence->getBeginDrawingTimestamp(),
-      frameTimingSequence->getCommitTimestamp(),
-      frameTimingSequence->getEndDrawingTimestamp(),
+      frameTimingSequence->getBeginTimestamp(),
+      frameTimingSequence->getEndTimestamp(),
       frameTimingSequence->getScreenshot(),
   });
 }

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h

@@ -84,23 +84,16 @@ struct JFrameTimingSequence : public jni::JavaClass<JFrameTimingSequence> {
     return static_cast<uint64_t>(getFieldValue(field));
   }
 
-  HighResTimeStamp getBeginDrawingTimestamp() const
+  HighResTimeStamp getBeginTimestamp() const
   {
-    auto field = javaClassStatic()->getField<jlong>("beginDrawingTimestamp");
+    auto field = javaClassStatic()->getField<jlong>("beginTimestamp");
     return HighResTimeStamp::fromChronoSteadyClockTimePoint(
         std::chrono::steady_clock::time_point(std::chrono::nanoseconds(getFieldValue(field))));
   }
 
-  HighResTimeStamp getCommitTimestamp() const
+  HighResTimeStamp getEndTimestamp() const
   {
-    auto field = javaClassStatic()->getField<jlong>("commitTimestamp");
-    return HighResTimeStamp::fromChronoSteadyClockTimePoint(
-        std::chrono::steady_clock::time_point(std::chrono::nanoseconds(getFieldValue(field))));
-  }
-
-  HighResTimeStamp getEndDrawingTimestamp() const
-  {
-    auto field = javaClassStatic()->getField<jlong>("endDrawingTimestamp");
+    auto field = javaClassStatic()->getField<jlong>("endTimestamp");
     return HighResTimeStamp::fromChronoSteadyClockTimePoint(
         std::chrono::steady_clock::time_point(std::chrono::nanoseconds(getFieldValue(field))));
   }

packages/react-native/ReactCommon/jsinspector-modern/HostTargetTraceRecording.cpp

@@ -23,8 +23,7 @@ HostTargetTraceRecording::HostTargetTraceRecording(
       windowSize_(windowSize) {
   if (windowSize) {
     frameTimings_ = tracing::TimeWindowedBuffer<tracing::FrameTimingSequence>(
-        [](auto& sequence) { return sequence.beginDrawingTimestamp; },
-        *windowSize);
+        [](auto& sequence) { return sequence.beginTimestamp; }, *windowSize);
   } else {
     frameTimings_ = tracing::TimeWindowedBuffer<tracing::FrameTimingSequence>();
   };

packages/react-native/ReactCommon/jsinspector-modern/tests/TracingTest.cpp

@@ -66,7 +66,6 @@ TEST_F(TracingTest, RecordsFrameTimings) {
       1, // id
       11, // threadId
       now,
-      now + HighResDuration::fromNanoseconds(10),
       now + HighResDuration::fromNanoseconds(50));
 
   page_->recordFrameTimings(frameTimingSequence);
@@ -86,12 +85,10 @@ TEST_F(TracingTest, EmitsRecordedFrameTimingSequences) {
           1, // id
           11, // threadId
           now,
-          now + HighResDuration::fromNanoseconds(10),
           now + HighResDuration::fromNanoseconds(50)));
 
   auto allTraceEvents = endTracingAndCollectEvents();
   EXPECT_THAT(allTraceEvents, Contains(AtJsonPtr("/name", "BeginFrame")));
-  EXPECT_THAT(allTraceEvents, Contains(AtJsonPtr("/name", "Commit")));
   EXPECT_THAT(allTraceEvents, Contains(AtJsonPtr("/name", "DrawFrame")));
 }
 
@@ -105,7 +102,6 @@ TEST_F(TracingTest, EmitsScreenshotEventWhenScreenshotValuePassed) {
           1, // id
           11, // threadId
           now,
-          now + HighResDuration::fromNanoseconds(10),
           now + HighResDuration::fromNanoseconds(50),
           "base64EncodedScreenshotData"));
 
@@ -227,7 +223,6 @@ TEST_F(TracingTest, EmitsToAllSessionsWithReactNativeApplicationDomainEnabled) {
           1, // id
           11, // threadId
           now,
-          now + HighResDuration::fromNanoseconds(10),
           now + HighResDuration::fromNanoseconds(50)));
 
   // Primary and secondaryFusebox sessions should receive the trace.
@@ -287,7 +282,6 @@ TEST_F(TracingTest, StashedTraceIsEmittedOnlyToFirstEligibleSession) {
           1, // id
           11, // threadId
           now,
-          now + HighResDuration::fromNanoseconds(10),
           now + HighResDuration::fromNanoseconds(50)));
 
   // Stop tracing - no eligible sessions exist, so the trace is stashed

packages/react-native/ReactCommon/jsinspector-modern/tracing/FrameTimingSequence.h

@@ -24,15 +24,13 @@ struct FrameTimingSequence {
   FrameTimingSequence(
       FrameSequenceId id,
       ThreadId threadId,
-      HighResTimeStamp beginDrawingTimestamp,
-      HighResTimeStamp commitTimestamp,
-      HighResTimeStamp endDrawingTimestamp,
+      HighResTimeStamp beginTimestamp,
+      HighResTimeStamp endTimestamp,
       std::optional<std::string> screenshot = std::nullopt)
       : id(id),
         threadId(threadId),
-        beginDrawingTimestamp(beginDrawingTimestamp),
-        commitTimestamp(commitTimestamp),
-        endDrawingTimestamp(endDrawingTimestamp),
+        beginTimestamp(beginTimestamp),
+        endTimestamp(endTimestamp),
         screenshot(std::move(screenshot))
   {
   }
@@ -47,9 +45,8 @@ struct FrameTimingSequence {
    */
   ThreadId threadId;
 
-  HighResTimeStamp beginDrawingTimestamp;
-  HighResTimeStamp commitTimestamp;
-  HighResTimeStamp endDrawingTimestamp;
+  HighResTimeStamp beginTimestamp;
+  HighResTimeStamp endTimestamp;
 
   /**
    * Optional screenshot data (base64 encoded) captured during the frame.

packages/react-native/ReactCommon/jsinspector-modern/tracing/HostTracingProfileSerializer.cpp

@@ -106,19 +106,17 @@ constexpr int FALLBACK_LAYER_TREE_ID = 1;
       chunk = generateNewChunk(chunkSize);
     }
 
-    auto [beginDrawingEvent, commitEvent, endDrawingEvent] =
+    auto [beginDrawingEvent, endDrawingEvent] =
         TraceEventGenerator::createFrameTimingsEvents(
             frameTimingSequence.id,
             FALLBACK_LAYER_TREE_ID,
-            frameTimingSequence.beginDrawingTimestamp,
-            frameTimingSequence.commitTimestamp,
-            frameTimingSequence.endDrawingTimestamp,
+            frameTimingSequence.beginTimestamp,
+            frameTimingSequence.endTimestamp,
             processId,
             frameTimingSequence.threadId);
 
     chunk.push_back(
         TraceEventSerializer::serialize(std::move(beginDrawingEvent)));
-    chunk.push_back(TraceEventSerializer::serialize(std::move(commitEvent)));
     chunk.push_back(
         TraceEventSerializer::serialize(std::move(endDrawingEvent)));
 
@@ -127,7 +125,7 @@ constexpr int FALLBACK_LAYER_TREE_ID = 1;
           frameTimingSequence.id,
           FALLBACK_LAYER_TREE_ID,
           std::move(frameTimingSequence.screenshot.value()),
-          frameTimingSequence.endDrawingTimestamp,
+          frameTimingSequence.endTimestamp,
           processId,
           frameTimingSequence.threadId);
 

packages/react-native/ReactCommon/jsinspector-modern/tracing/TraceEventGenerator.cpp

@@ -32,13 +32,12 @@ namespace facebook::react::jsinspector_modern::tracing {
   };
 }
 
-/* static */ std::tuple<TraceEvent, TraceEvent, TraceEvent>
+/* static */ std::pair<TraceEvent, TraceEvent>
 TraceEventGenerator::createFrameTimingsEvents(
     uint64_t sequenceId,
     int layerTreeId,
-    HighResTimeStamp beginDrawingTimestamp,
-    HighResTimeStamp commitTimestamp,
-    HighResTimeStamp endDrawingTimestamp,
+    HighResTimeStamp beginTimestamp,
+    HighResTimeStamp endTimestamp,
     ProcessId processId,
     ThreadId threadId) {
   folly::dynamic args = folly::dynamic::object("frameSeqId", sequenceId)(
@@ -48,17 +47,7 @@ TraceEventGenerator::createFrameTimingsEvents(
       .name = "BeginFrame",
       .cat = {Category::Frame},
       .ph = 'I',
-      .ts = beginDrawingTimestamp,
-      .pid = processId,
-      .s = 't',
-      .tid = threadId,
-      .args = args,
-  };
-  auto commitEvent = TraceEvent{
-      .name = "Commit",
-      .cat = {Category::Frame},
-      .ph = 'I',
-      .ts = commitTimestamp,
+      .ts = beginTimestamp,
       .pid = processId,
       .s = 't',
       .tid = threadId,
@@ -68,14 +57,14 @@ TraceEventGenerator::createFrameTimingsEvents(
       .name = "DrawFrame",
       .cat = {Category::Frame},
       .ph = 'I',
-      .ts = endDrawingTimestamp,
+      .ts = endTimestamp,
       .pid = processId,
       .s = 't',
       .tid = threadId,
       .args = args,
   };
 
-  return {std::move(beginEvent), std::move(commitEvent), std::move(drawEvent)};
+  return {std::move(beginEvent), std::move(drawEvent)};
 }
 
 /* static */ TraceEvent TraceEventGenerator::createScreenshotEvent(

packages/react-native/ReactCommon/jsinspector-modern/tracing/TraceEventGenerator.h

@@ -12,7 +12,7 @@
 #include <jsinspector-modern/tracing/FrameTimingSequence.h>
 #include <react/timing/primitives.h>
 
-#include <tuple>
+#include <utility>
 
 namespace facebook::react::jsinspector_modern::tracing {
 
@@ -35,12 +35,11 @@ class TraceEventGenerator {
   /**
    * Creates canonical "BeginFrame", "Commit", "DrawFrame" trace events.
    */
-  static std::tuple<TraceEvent, TraceEvent, TraceEvent> createFrameTimingsEvents(
+  static std::pair<TraceEvent, TraceEvent> createFrameTimingsEvents(
       FrameSequenceId sequenceId,
       int layerTreeId,
-      HighResTimeStamp beginDrawingTimestamp,
-      HighResTimeStamp commitTimestamp,
-      HighResTimeStamp endDrawingTimestamp,
+      HighResTimeStamp beginTimestamp,
+      HighResTimeStamp endTimestamp,
       ProcessId processId,
       ThreadId threadId);