Revert "Revert "[TimeStats] Add callback for global stats""
This reverts commit de64c56f39b03f4908200d1ed2fdc9561f86fe2a.
Reason for revert: Rolling forward with fix
Test: adb shell cmd stats pull-source 10062
Change-Id: I981b12a1c5e69aad0c0d3a409d8b90b3399401f4
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 1796476..06a9b1f 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -494,6 +494,7 @@
ALOGI("Boot is finished (%ld ms)", long(ns2ms(duration)) );
mFrameTracer->initialize();
+ mTimeStats->onBootFinished();
// wait patiently for the window manager death
const String16 name("window");
diff --git a/services/surfaceflinger/TimeStats/Android.bp b/services/surfaceflinger/TimeStats/Android.bp
index 20c2218..7ff2594 100644
--- a/services/surfaceflinger/TimeStats/Android.bp
+++ b/services/surfaceflinger/TimeStats/Android.bp
@@ -8,12 +8,18 @@
"libcutils",
"liblog",
"libprotobuf-cpp-lite",
+ "libstatslog",
+ "libstatspull",
+ "libstatssocket",
"libtimestats_proto",
"libui",
"libutils",
],
export_include_dirs: ["."],
export_shared_lib_headers: [
+ "libstatslog",
+ "libstatspull",
+ "libstatssocket",
"libtimestats_proto",
],
cppflags: [
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 0939fa4..44a59fd 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -36,11 +36,50 @@
namespace impl {
-TimeStats::TimeStats() {
+status_pull_atom_return_t TimeStats::pullGlobalAtomCallback(int32_t atom_tag,
+ pulled_stats_event_list* data,
+ void* cookie) {
+ impl::TimeStats* timeStats = reinterpret_cast<impl::TimeStats*>(cookie);
+ if (atom_tag != android::util::SURFACEFLINGER_STATS_GLOBAL_INFO) {
+ return STATS_PULL_SKIP;
+ }
+
+ std::lock_guard<std::mutex> lock(timeStats->mMutex);
+
+ const auto& stats = timeStats->mTimeStats;
+ if (stats.statsStart == 0) {
+ return STATS_PULL_SKIP;
+ }
+ timeStats->flushPowerTimeLocked();
+
+ struct stats_event* event = timeStats->mStatsDelegate->addStatsEventToPullData(data);
+ timeStats->mStatsDelegate->statsEventSetAtomId(event,
+ android::util::SURFACEFLINGER_STATS_GLOBAL_INFO);
+ timeStats->mStatsDelegate->statsEventWriteInt64(event, stats.totalFrames);
+ timeStats->mStatsDelegate->statsEventWriteInt64(event, stats.missedFrames);
+ timeStats->mStatsDelegate->statsEventWriteInt64(event, stats.clientCompositionFrames);
+ timeStats->mStatsDelegate->statsEventWriteInt64(event, stats.displayOnTime);
+ timeStats->mStatsDelegate->statsEventWriteInt64(event, stats.presentToPresent.totalTime());
+ timeStats->mStatsDelegate->statsEventBuild(event);
+ timeStats->clearGlobalLocked();
+
+ return STATS_PULL_SUCCESS;
+}
+
+TimeStats::TimeStats() : TimeStats(nullptr) {}
+
+TimeStats::TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate) {
+ if (statsDelegate != nullptr) {
+ mStatsDelegate = std::move(statsDelegate);
+ }
+}
+
+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.
- // TODO: remove this.
+ // during testing without enabling by default.
+ // TODO: remove this, as we should only be paying this overhead on devices
+ // where statsd exists.
enable();
}
@@ -69,7 +108,7 @@
}
if (argsMap.count("-clear")) {
- clear();
+ clearAll();
}
if (argsMap.count("-enable")) {
@@ -594,6 +633,8 @@
mEnabled.store(true);
mTimeStats.statsStart = static_cast<int64_t>(std::time(0));
mPowerTime.prevTime = systemTime();
+ mStatsDelegate->registerStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO,
+ TimeStats::pullGlobalAtomCallback, nullptr, this);
ALOGD("Enabled");
}
@@ -606,15 +647,20 @@
flushPowerTimeLocked();
mEnabled.store(false);
mTimeStats.statsEnd = static_cast<int64_t>(std::time(0));
+ mStatsDelegate->unregisterStatsPullAtomCallback(
+ android::util::SURFACEFLINGER_STATS_GLOBAL_INFO);
ALOGD("Disabled");
}
-void TimeStats::clear() {
+void TimeStats::clearAll() {
+ std::lock_guard<std::mutex> lock(mMutex);
+ clearGlobalLocked();
+ clearLayersLocked();
+}
+
+void TimeStats::clearGlobalLocked() {
ATRACE_CALL();
- std::lock_guard<std::mutex> lock(mMutex);
- mTimeStatsTracker.clear();
- mTimeStats.stats.clear();
mTimeStats.statsStart = (mEnabled.load() ? static_cast<int64_t>(std::time(0)) : 0);
mTimeStats.statsEnd = 0;
mTimeStats.totalFrames = 0;
@@ -628,7 +674,15 @@
mPowerTime.prevTime = systemTime();
mGlobalRecord.prevPresentTime = 0;
mGlobalRecord.presentFences.clear();
- ALOGD("Cleared");
+ ALOGD("Cleared global stats");
+}
+
+void TimeStats::clearLayersLocked() {
+ ATRACE_CALL();
+
+ mTimeStatsTracker.clear();
+ mTimeStats.stats.clear();
+ ALOGD("Cleared layer stats");
}
bool TimeStats::isEnabled() {
diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index 65e5cf4..5cd421c 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -17,6 +17,9 @@
#pragma once
#include <hardware/hwcomposer_defs.h>
+#include <stats_event.h>
+#include <stats_pull_atom_callback.h>
+#include <statslog.h>
#include <timestatsproto/TimeStatsHelper.h>
#include <timestatsproto/TimeStatsProtoHeader.h>
#include <ui/FenceTime.h>
@@ -37,6 +40,10 @@
public:
virtual ~TimeStats() = default;
+ // Called once boot has been finished to perform additional capabilities,
+ // e.g. registration to statsd.
+ virtual void onBootFinished() = 0;
+
virtual void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) = 0;
virtual bool isEnabled() = 0;
virtual std::string miniDump() = 0;
@@ -131,6 +138,39 @@
public:
TimeStats();
+ // Delegate to the statsd service and associated APIs.
+ // Production code may use this class directly, whereas unit test may define
+ // a subclass for ease of testing.
+ class StatsEventDelegate {
+ public:
+ virtual ~StatsEventDelegate() = default;
+ virtual struct stats_event* addStatsEventToPullData(pulled_stats_event_list* data) {
+ return add_stats_event_to_pull_data(data);
+ }
+ virtual void registerStatsPullAtomCallback(int32_t atom_tag,
+ stats_pull_atom_callback_t callback,
+ pull_atom_metadata* metadata, void* cookie) {
+ return register_stats_pull_atom_callback(atom_tag, callback, metadata, cookie);
+ }
+
+ virtual void unregisterStatsPullAtomCallback(int32_t atom_tag) {
+ return unregister_stats_pull_atom_callback(atom_tag);
+ }
+
+ virtual void statsEventSetAtomId(struct stats_event* event, uint32_t atom_id) {
+ return stats_event_set_atom_id(event, atom_id);
+ }
+
+ virtual void statsEventWriteInt64(struct stats_event* event, int64_t field) {
+ return stats_event_write_int64(event, field);
+ }
+
+ virtual void statsEventBuild(struct stats_event* event) { return stats_event_build(event); }
+ };
+ // For testing only for injecting custom dependencies.
+ TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate);
+
+ void onBootFinished() override;
void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) override;
bool isEnabled() override;
std::string miniDump() override;
@@ -167,6 +207,9 @@
static const size_t MAX_NUM_TIME_RECORDS = 64;
private:
+ static status_pull_atom_return_t pullGlobalAtomCallback(int32_t atom_tag,
+ pulled_stats_event_list* data,
+ void* cookie);
bool recordReadyLocked(int32_t layerId, TimeRecord* timeRecord);
void flushAvailableRecordsToStatsLocked(int32_t layerId);
void flushPowerTimeLocked();
@@ -174,7 +217,9 @@
void enable();
void disable();
- void clear();
+ void clearAll();
+ void clearGlobalLocked();
+ void clearLayersLocked();
void dump(bool asProto, std::optional<uint32_t> maxLayers, std::string& result);
std::atomic<bool> mEnabled = false;
@@ -187,6 +232,7 @@
static const size_t MAX_NUM_LAYER_RECORDS = 200;
static const size_t MAX_NUM_LAYER_STATS = 200;
+ std::unique_ptr<StatsEventDelegate> mStatsDelegate = std::make_unique<StatsEventDelegate>();
};
} // namespace impl
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index becf484..e500672 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -83,6 +83,7 @@
"perfetto_trace_protos",
],
shared_libs: [
+ "libstatssocket",
"libsurfaceflinger",
"libtimestats",
"libtimestats_proto",
diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
index bcf3ba8..c667080 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -40,7 +40,9 @@
namespace android {
namespace {
+using testing::_;
using testing::Contains;
+using testing::InSequence;
using testing::SizeIs;
using testing::UnorderedElementsAre;
@@ -136,7 +138,40 @@
}
std::mt19937 mRandomEngine = std::mt19937(std::random_device()());
- std::unique_ptr<TimeStats> mTimeStats = std::make_unique<impl::TimeStats>();
+
+ class FakeStatsEventDelegate : public impl::TimeStats::StatsEventDelegate {
+ public:
+ FakeStatsEventDelegate() = default;
+ ~FakeStatsEventDelegate() override = default;
+
+ struct stats_event* addStatsEventToPullData(pulled_stats_event_list*) override {
+ return mEvent;
+ }
+ void registerStatsPullAtomCallback(int32_t atom_tag, stats_pull_atom_callback_t callback,
+ pull_atom_metadata*, void* cookie) override {
+ mAtomTag = atom_tag;
+ mCallback = callback;
+ mCookie = cookie;
+ }
+
+ status_pull_atom_return_t makePullAtomCallback(int32_t atom_tag, void* cookie) {
+ return (*mCallback)(atom_tag, nullptr, cookie);
+ }
+
+ MOCK_METHOD1(unregisterStatsPullAtomCallback, void(int32_t));
+ MOCK_METHOD2(statsEventSetAtomId, void(struct stats_event*, uint32_t));
+ MOCK_METHOD2(statsEventWriteInt64, void(struct stats_event*, int64_t));
+ MOCK_METHOD1(statsEventBuild, void(struct stats_event*));
+
+ struct stats_event* mEvent = stats_event_obtain();
+ int32_t mAtomTag = 0;
+ stats_pull_atom_callback_t mCallback = nullptr;
+ void* mCookie = nullptr;
+ };
+
+ FakeStatsEventDelegate* mDelegate = new FakeStatsEventDelegate;
+ std::unique_ptr<TimeStats> mTimeStats =
+ std::make_unique<impl::TimeStats>(std::unique_ptr<FakeStatsEventDelegate>(mDelegate));
};
std::string TimeStatsTest::inputCommand(InputCommand cmd, bool useProto) {
@@ -213,14 +248,22 @@
return distr(mRandomEngine);
}
-TEST_F(TimeStatsTest, enabledByDefault) {
+TEST_F(TimeStatsTest, disabledByDefault) {
+ ASSERT_FALSE(mTimeStats->isEnabled());
+}
+
+TEST_F(TimeStatsTest, enabledAfterBoot) {
+ mTimeStats->onBootFinished();
ASSERT_TRUE(mTimeStats->isEnabled());
}
TEST_F(TimeStatsTest, canEnableAndDisableTimeStats) {
EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty());
ASSERT_TRUE(mTimeStats->isEnabled());
+ EXPECT_EQ(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, mDelegate->mAtomTag);
+ EXPECT_CALL(*mDelegate,
+ unregisterStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO));
EXPECT_TRUE(inputCommand(InputCommand::DISABLE, FMT_STRING).empty());
ASSERT_FALSE(mTimeStats->isEnabled());
}
@@ -631,6 +674,56 @@
ASSERT_EQ(0, globalProto.stats_size());
}
+TEST_F(TimeStatsTest, globalStatsCallback) {
+ constexpr size_t TOTAL_FRAMES = 5;
+ constexpr size_t MISSED_FRAMES = 4;
+ constexpr size_t CLIENT_COMPOSITION_FRAMES = 3;
+
+ mTimeStats->onBootFinished();
+ EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty());
+
+ for (size_t i = 0; i < TOTAL_FRAMES; i++) {
+ mTimeStats->incrementTotalFrames();
+ }
+ for (size_t i = 0; i < MISSED_FRAMES; i++) {
+ mTimeStats->incrementMissedFrames();
+ }
+ for (size_t i = 0; i < CLIENT_COMPOSITION_FRAMES; i++) {
+ mTimeStats->incrementClientCompositionFrames();
+ }
+
+ mTimeStats->setPowerMode(HWC_POWER_MODE_NORMAL);
+ mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(3000000));
+ mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(5000000));
+
+ EXPECT_EQ(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, mDelegate->mAtomTag);
+ EXPECT_NE(nullptr, mDelegate->mCallback);
+ EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie);
+
+ {
+ InSequence seq;
+ EXPECT_CALL(*mDelegate,
+ statsEventSetAtomId(mDelegate->mEvent,
+ android::util::SURFACEFLINGER_STATS_GLOBAL_INFO));
+ EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, TOTAL_FRAMES));
+ EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, MISSED_FRAMES));
+ EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, CLIENT_COMPOSITION_FRAMES));
+ EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, _));
+ EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, 2));
+ EXPECT_CALL(*mDelegate, statsEventBuild(mDelegate->mEvent));
+ }
+ EXPECT_EQ(STATS_PULL_SUCCESS,
+ mDelegate->makePullAtomCallback(mDelegate->mAtomTag, mDelegate->mCookie));
+
+ SFTimeStatsGlobalProto globalProto;
+ ASSERT_TRUE(globalProto.ParseFromString(inputCommand(InputCommand::DUMP_ALL, FMT_PROTO)));
+
+ EXPECT_EQ(0, globalProto.total_frames());
+ EXPECT_EQ(0, globalProto.missed_frames());
+ EXPECT_EQ(0, globalProto.client_composition_frames());
+ EXPECT_EQ(0, globalProto.present_to_present_size());
+}
+
TEST_F(TimeStatsTest, canSurviveMonkey) {
if (g_noSlowTests) {
GTEST_SKIP();
diff --git a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
index ec74a42..9ada5ef 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
@@ -28,6 +28,7 @@
TimeStats();
~TimeStats() override;
+ MOCK_METHOD0(onBootFinished, void());
MOCK_METHOD3(parseArgs, void(bool, const Vector<String16>&, std::string&));
MOCK_METHOD0(isEnabled, bool());
MOCK_METHOD0(miniDump, std::string());