Don't clear all of global stats when clearing layer stats.
Otherwise this produces significant server-side skew when validating
stats because pulling layer stats can destroy any pending global stats,
which can cause the loss of an entire day's worth of data.
Bug: 177999557
Test: libsurfaceflinger_unittest
Test: adb shell cmd stats pull-source 10062
Test: adb shell cmd stats pull-source 10063
Change-Id: I60599a584a189a60e7eac9eaba0800d6ade76b90
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 100354a..974ae84 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -1070,6 +1070,9 @@
mTimeStats.renderEngineTimingLegacy.hist.clear();
mTimeStats.refreshRateStatsLegacy.clear();
mPowerTime.prevTime = systemTime();
+ for (auto& globalRecord : mTimeStats.stats) {
+ globalRecord.second.clearGlobals();
+ }
mGlobalRecord.prevPresentTime = 0;
mGlobalRecord.presentFences.clear();
ALOGD("Cleared global stats");
@@ -1079,7 +1082,10 @@
ATRACE_CALL();
mTimeStatsTracker.clear();
- mTimeStats.stats.clear();
+
+ for (auto& globalRecord : mTimeStats.stats) {
+ globalRecord.second.stats.clear();
+ }
ALOGD("Cleared layer stats");
}
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h b/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
index 4556bad..5ee28ce 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
+++ b/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
@@ -127,6 +127,12 @@
Histogram displayDeadlineDeltas;
Histogram displayPresentDeltas;
std::unordered_map<LayerStatsKey, TimeStatsLayer, LayerStatsKey::Hasher> stats;
+
+ void clearGlobals() {
+ jankPayload = {};
+ displayDeadlineDeltas = {};
+ displayPresentDeltas = {};
+ }
};
class TimeStatsGlobal {
diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
index e903664..a72ab42 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -48,6 +48,7 @@
using testing::Contains;
using testing::HasSubstr;
using testing::InSequence;
+using testing::Not;
using testing::SizeIs;
using testing::StrEq;
using testing::UnorderedElementsAre;
@@ -855,6 +856,24 @@
EXPECT_THAT(result, HasSubstr("compositionStrategyChanges = 0"));
EXPECT_THAT(result, HasSubstr("averageFrameDuration = 0.000 ms"));
EXPECT_THAT(result, HasSubstr("averageRenderEngineTiming = 0.000 ms"));
+ std::string expectedResult = "totalTimelineFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "jankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfLongCpuJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfLongGpuJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfUnattributedJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "appUnattributedJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfSchedulingJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfPredictionErrorJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
}
TEST_F(TimeStatsTest, canDumpWithMaxLayers) {
@@ -1074,6 +1093,46 @@
EXPECT_EQ(0, globalProto.missed_frames());
EXPECT_EQ(0, globalProto.client_composition_frames());
EXPECT_EQ(0, globalProto.present_to_present_size());
+
+ // also check dump-only stats: expect that global stats are indeed dropped but there should
+ // still be stats for the layer
+ const std::string result(inputCommand(InputCommand::DUMP_ALL, FMT_STRING));
+ std::string expectedResult = "totalTimelineFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "totalTimelineFrames = " + std::to_string(8);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "jankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "jankyFrames = " + std::to_string(7);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfLongCpuJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfLongCpuJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfLongGpuJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfLongGpuJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfUnattributedJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfUnattributedJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "appUnattributedJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "appUnattributedJankyFrames = " + std::to_string(2);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfSchedulingJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfSchedulingJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfPredictionErrorJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfPredictionErrorJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(0);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
}
TEST_F(TimeStatsTest, layerStatsCallback_pullsAllAndClears) {
@@ -1205,6 +1264,31 @@
ASSERT_TRUE(globalProto.ParseFromString(inputCommand(InputCommand::DUMP_ALL, FMT_PROTO)));
EXPECT_EQ(0, globalProto.stats_size());
+
+ // also check dump-only stats: expect that layer stats are indeed dropped but there should still
+ // be global stats
+ const std::string result(inputCommand(InputCommand::DUMP_ALL, FMT_STRING));
+ std::string expectedResult = "totalTimelineFrames = " + std::to_string(8);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "jankyFrames = " + std::to_string(7);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfLongCpuJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfLongGpuJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfUnattributedJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "appUnattributedJankyFrames = " + std::to_string(2);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfSchedulingJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "sfPredictionErrorJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+ expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(1);
+ EXPECT_THAT(result, HasSubstr(expectedResult));
+
+ std::string expectedMissing = "uid = " + std::to_string(UID_0);
+ EXPECT_THAT(result, Not(HasSubstr(expectedMissing)));
}
TEST_F(TimeStatsTest, layerStatsCallback_pullsMultipleLayers) {