[TimeStats] Default-disable stats collection
Now that we're ramping up collection in droidfood populations, we can
default-disable now and begin ramping up.
Bug: 148116614
Test: adb shell dumpsys SurfaceFlinger --timestats -dump
Change-Id: I48ac6be7915539f681c0cbbd089513a7a0222a0a
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 12c98da..130e99a 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -40,13 +40,18 @@
status_pull_atom_return_t TimeStats::pullAtomCallback(int32_t atom_tag,
pulled_stats_event_list* data, void* cookie) {
impl::TimeStats* timeStats = reinterpret_cast<impl::TimeStats*>(cookie);
+ status_pull_atom_return_t result = STATS_PULL_SKIP;
if (atom_tag == android::util::SURFACEFLINGER_STATS_GLOBAL_INFO) {
- return timeStats->populateGlobalAtom(data);
+ result = timeStats->populateGlobalAtom(data);
} else if (atom_tag == android::util::SURFACEFLINGER_STATS_LAYER_INFO) {
- return timeStats->populateLayerAtom(data);
+ result = timeStats->populateLayerAtom(data);
}
- return STATS_PULL_SKIP;
+ // Enable timestats now. The first full pull for a given build is expected to
+ // have empty or very little stats, as stats are first enabled after the
+ // first pull is completed for either the global or layer stats.
+ timeStats->enable();
+ return result;
}
status_pull_atom_return_t TimeStats::populateGlobalAtom(pulled_stats_event_list* data) {
@@ -167,13 +172,19 @@
}
}
+TimeStats::~TimeStats() {
+ std::lock_guard<std::mutex> lock(mMutex);
+ mStatsDelegate->unregisterStatsPullAtomCallback(
+ android::util::SURFACEFLINGER_STATS_GLOBAL_INFO);
+ mStatsDelegate->unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO);
+}
+
void TimeStats::onBootFinished() {
- // Temporarily enable TimeStats by default. Telemetry is disabled while
- // we move onto statsd, so TimeStats is currently not exercised at all
- // during testing without enabling by default.
- // TODO: remove this, as we should only be paying this overhead on devices
- // where statsd exists.
- enable();
+ std::lock_guard<std::mutex> lock(mMutex);
+ mStatsDelegate->registerStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO,
+ TimeStats::pullAtomCallback, nullptr, this);
+ mStatsDelegate->registerStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO,
+ TimeStats::pullAtomCallback, nullptr, this);
}
void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, std::string& result) {
@@ -735,10 +746,6 @@
mEnabled.store(true);
mTimeStats.statsStart = static_cast<int64_t>(std::time(0));
mPowerTime.prevTime = systemTime();
- mStatsDelegate->registerStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO,
- TimeStats::pullAtomCallback, nullptr, this);
- mStatsDelegate->registerStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO,
- TimeStats::pullAtomCallback, nullptr, this);
ALOGD("Enabled");
}
@@ -751,9 +758,6 @@
flushPowerTimeLocked();
mEnabled.store(false);
mTimeStats.statsEnd = static_cast<int64_t>(std::time(0));
- mStatsDelegate->unregisterStatsPullAtomCallback(
- android::util::SURFACEFLINGER_STATS_GLOBAL_INFO);
- mStatsDelegate->unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO);
ALOGD("Disabled");
}
diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index 71f06af..67b9d10 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -182,6 +182,8 @@
std::optional<size_t> maxPulledLayers,
std::optional<size_t> maxPulledHistogramBuckets);
+ ~TimeStats() override;
+
void onBootFinished() override;
void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) override;
bool isEnabled() override;
diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
index 68e6697..f65af77 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -258,22 +258,25 @@
ASSERT_FALSE(mTimeStats->isEnabled());
}
-TEST_F(TimeStatsTest, enabledAfterBoot) {
+TEST_F(TimeStatsTest, registersCallbacksAfterBoot) {
mTimeStats->onBootFinished();
- ASSERT_TRUE(mTimeStats->isEnabled());
+ EXPECT_THAT(mDelegate->mAtomTags,
+ UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO,
+ android::util::SURFACEFLINGER_STATS_LAYER_INFO));
+}
+
+TEST_F(TimeStatsTest, unregistersCallbacksOnDestruction) {
+ EXPECT_CALL(*mDelegate,
+ unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO));
+ EXPECT_CALL(*mDelegate,
+ unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO));
+ mTimeStats.reset();
}
TEST_F(TimeStatsTest, canEnableAndDisableTimeStats) {
EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty());
ASSERT_TRUE(mTimeStats->isEnabled());
- EXPECT_THAT(mDelegate->mAtomTags,
- UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO,
- android::util::SURFACEFLINGER_STATS_LAYER_INFO));
- EXPECT_CALL(*mDelegate,
- unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO));
- EXPECT_CALL(*mDelegate,
- unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO));
EXPECT_TRUE(inputCommand(InputCommand::DISABLE, FMT_STRING).empty());
ASSERT_FALSE(mTimeStats->isEnabled());
}