sf: only look for graphics frame packets when asserting on traces
Asserting on packet counts leads to extremely brittle tests as it
prevent the Perfetto team from adding new service-generated packets
as needed.
Instead, perform all assertions on packets which have already been
filtered to contain just packets of relevance to sf - namely
GraphicsFrameEvent packets.
(this is a cherry-pick of ag/12108746 onto rvc-dev-plus-aosp because
the test failure on aosp/1355782 is affecting this branch as well).
Change-Id: I0c323761acd35ca4d3b9e21c2a68d1baefdcd13a
Merged-In: I0c323761acd35ca4d3b9e21c2a68d1baefdcd13a
diff --git a/services/surfaceflinger/tests/unittests/FrameTracerTest.cpp b/services/surfaceflinger/tests/unittests/FrameTracerTest.cpp
index 68cb52f..a119e27 100644
--- a/services/surfaceflinger/tests/unittests/FrameTracerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FrameTracerTest.cpp
@@ -77,6 +77,22 @@
return tracingSession;
}
+ std::vector<perfetto::protos::TracePacket> readGraphicsFramePacketsBlocking(
+ perfetto::TracingSession* tracingSession) {
+ std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
+ perfetto::protos::Trace trace;
+ EXPECT_TRUE(trace.ParseFromArray(raw_trace.data(), int(raw_trace.size())));
+
+ std::vector<perfetto::protos::TracePacket> packets;
+ for (const auto& packet : trace.packet()) {
+ if (!packet.has_graphics_frame_event()) {
+ continue;
+ }
+ packets.emplace_back(packet);
+ }
+ return packets;
+ }
+
std::unique_ptr<FrameTracer> mFrameTracer;
FenceToFenceTimeMap fenceFactory;
};
@@ -142,40 +158,29 @@
auto tracingSession = getTracingSessionForTest();
tracingSession->StartBlocking();
- // Clean up irrelevant traces.
- tracingSession->ReadTraceBlocking();
-
mFrameTracer->traceTimestamp(layerId, bufferID, frameNumber, timestamp, type, duration);
// Create second trace packet to finalize the previous one.
mFrameTracer->traceTimestamp(layerId, 0, 0, 0, FrameTracer::FrameEvent::UNSPECIFIED);
tracingSession->StopBlocking();
- std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
- EXPECT_EQ(raw_trace.size(), 0);
+ auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 0);
}
{
auto tracingSession = getTracingSessionForTest();
tracingSession->StartBlocking();
- // Clean up irrelevant traces.
- tracingSession->ReadTraceBlocking();
-
mFrameTracer->traceNewLayer(layerId, layerName);
mFrameTracer->traceTimestamp(layerId, bufferID, frameNumber, timestamp, type, duration);
// Create second trace packet to finalize the previous one.
mFrameTracer->traceTimestamp(layerId, 0, 0, 0, FrameTracer::FrameEvent::UNSPECIFIED);
tracingSession->StopBlocking();
- std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
- ASSERT_GT(raw_trace.size(), 0);
+ auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 1);
- perfetto::protos::Trace trace;
- ASSERT_TRUE(trace.ParseFromArray(raw_trace.data(), int(raw_trace.size())));
- ASSERT_FALSE(trace.packet().empty());
- EXPECT_EQ(trace.packet().size(), 1);
-
- const auto& packet = trace.packet().Get(0);
+ const auto& packet = packets[0];
ASSERT_TRUE(packet.has_timestamp());
EXPECT_EQ(packet.timestamp(), timestamp);
ASSERT_TRUE(packet.has_graphics_frame_event());
@@ -205,24 +210,21 @@
fenceFactory.signalAllForTest(Fence::NO_FENCE, Fence::SIGNAL_TIME_PENDING);
auto tracingSession = getTracingSessionForTest();
tracingSession->StartBlocking();
- // Clean up irrelevant traces.
- tracingSession->ReadTraceBlocking();
// Trace.
mFrameTracer->traceNewLayer(layerId, layerName);
mFrameTracer->traceFence(layerId, bufferID, frameNumber, fenceTime, type);
// Create extra trace packet to (hopefully not) trigger and finalize the fence packet.
mFrameTracer->traceTimestamp(layerId, bufferID, 0, 0, FrameTracer::FrameEvent::UNSPECIFIED);
tracingSession->StopBlocking();
- std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
- EXPECT_EQ(raw_trace.size(), 0);
+
+ auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 0);
}
{
auto fenceTime = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE);
auto tracingSession = getTracingSessionForTest();
tracingSession->StartBlocking();
- // Clean up irrelevant traces.
- tracingSession->ReadTraceBlocking();
mFrameTracer->traceNewLayer(layerId, layerName);
mFrameTracer->traceFence(layerId, bufferID, frameNumber, fenceTime, type);
const nsecs_t timestamp = systemTime();
@@ -231,15 +233,10 @@
mFrameTracer->traceTimestamp(layerId, bufferID, 0, 0, FrameTracer::FrameEvent::UNSPECIFIED);
tracingSession->StopBlocking();
- std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
- ASSERT_GT(raw_trace.size(), 0);
+ auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 2); // Two packets because of the extra trace made above.
- perfetto::protos::Trace trace;
- ASSERT_TRUE(trace.ParseFromArray(raw_trace.data(), int(raw_trace.size())));
- ASSERT_FALSE(trace.packet().empty());
- EXPECT_EQ(trace.packet().size(), 2); // Two packets because of the extra trace made above.
-
- const auto& packet = trace.packet().Get(1);
+ const auto& packet = packets[1];
ASSERT_TRUE(packet.has_timestamp());
EXPECT_EQ(packet.timestamp(), timestamp);
ASSERT_TRUE(packet.has_graphics_frame_event());
@@ -266,8 +263,6 @@
auto tracingSession = getTracingSessionForTest();
tracingSession->StartBlocking();
- // Clean up irrelevant traces.
- tracingSession->ReadTraceBlocking();
mFrameTracer->traceNewLayer(layerId, layerName);
// traceFence called after fence signalled.
@@ -288,22 +283,17 @@
mFrameTracer->traceTimestamp(layerId, bufferID, 0, 0, FrameTracer::FrameEvent::UNSPECIFIED);
tracingSession->StopBlocking();
- std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
- ASSERT_GT(raw_trace.size(), 0);
+ auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 2);
- perfetto::protos::Trace trace;
- ASSERT_TRUE(trace.ParseFromArray(raw_trace.data(), int(raw_trace.size())));
- ASSERT_FALSE(trace.packet().empty());
- EXPECT_EQ(trace.packet().size(), 2);
-
- const auto& packet1 = trace.packet().Get(0);
+ const auto& packet1 = packets[0];
ASSERT_TRUE(packet1.has_timestamp());
EXPECT_EQ(packet1.timestamp(), signalTime1);
ASSERT_TRUE(packet1.has_graphics_frame_event());
ASSERT_TRUE(packet1.graphics_frame_event().has_buffer_event());
ASSERT_FALSE(packet1.graphics_frame_event().buffer_event().has_duration_ns());
- const auto& packet2 = trace.packet().Get(1);
+ const auto& packet2 = packets[1];
ASSERT_TRUE(packet2.has_timestamp());
EXPECT_EQ(packet2.timestamp(), signalTime2);
ASSERT_TRUE(packet2.has_graphics_frame_event());
@@ -323,8 +313,6 @@
auto fence = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE);
tracingSession->StartBlocking();
- // Clean up irrelevant traces.
- tracingSession->ReadTraceBlocking();
mFrameTracer->traceNewLayer(layerId, layerName);
mFrameTracer->traceFence(layerId, bufferID, frameNumber, fence, type);
fenceFactory.signalAllForTest(Fence::NO_FENCE, signalTime);
@@ -332,8 +320,8 @@
mFrameTracer->traceTimestamp(layerId, bufferID, 0, 0, FrameTracer::FrameEvent::UNSPECIFIED);
tracingSession->StopBlocking();
- std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
- EXPECT_EQ(raw_trace.size(), 0);
+ auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 0);
}
TEST_F(FrameTracerTest, traceFenceWithValidStartTime_ShouldHaveCorrectDuration) {
@@ -347,8 +335,6 @@
auto tracingSession = getTracingSessionForTest();
tracingSession->StartBlocking();
- // Clean up irrelevant traces.
- tracingSession->ReadTraceBlocking();
mFrameTracer->traceNewLayer(layerId, layerName);
// traceFence called after fence signalled.
@@ -369,15 +355,10 @@
mFrameTracer->traceTimestamp(layerId, bufferID, 0, 0, FrameTracer::FrameEvent::UNSPECIFIED);
tracingSession->StopBlocking();
- std::vector<char> raw_trace = tracingSession->ReadTraceBlocking();
- ASSERT_GT(raw_trace.size(), 0);
+ auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
+ EXPECT_EQ(packets.size(), 2);
- perfetto::protos::Trace trace;
- ASSERT_TRUE(trace.ParseFromArray(raw_trace.data(), int(raw_trace.size())));
- ASSERT_FALSE(trace.packet().empty());
- EXPECT_EQ(trace.packet().size(), 2);
-
- const auto& packet1 = trace.packet().Get(0);
+ const auto& packet1 = packets[0];
ASSERT_TRUE(packet1.has_timestamp());
EXPECT_EQ(packet1.timestamp(), startTime1);
ASSERT_TRUE(packet1.has_graphics_frame_event());
@@ -386,7 +367,7 @@
const auto& buffer_event1 = packet1.graphics_frame_event().buffer_event();
EXPECT_EQ(buffer_event1.duration_ns(), duration);
- const auto& packet2 = trace.packet().Get(1);
+ const auto& packet2 = packets[1];
ASSERT_TRUE(packet2.has_timestamp());
EXPECT_EQ(packet2.timestamp(), startTime2);
ASSERT_TRUE(packet2.has_graphics_frame_event());