Remove event thread connection metric.
It's not an actionable metric and is causing performance problems due to
lock contention. So, remove the code.
Bug: 170661858
Test: builds
Change-Id: I70fc2772d2cfa1c8364775427b1e8f08ce4ca0ff
diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp
index 281b0ae..c70ed2c 100644
--- a/services/surfaceflinger/Scheduler/EventThread.cpp
+++ b/services/surfaceflinger/Scheduler/EventThread.cpp
@@ -427,11 +427,6 @@
mCondition.notify_all();
}
-size_t EventThread::getEventThreadConnectionCount() {
- std::lock_guard<std::mutex> lock(mMutex);
- return mDisplayEventConnections.size();
-}
-
void EventThread::threadMain(std::unique_lock<std::mutex>& lock) {
DisplayEventConsumers consumers;
diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h
index 684745b..7023445 100644
--- a/services/surfaceflinger/Scheduler/EventThread.h
+++ b/services/surfaceflinger/Scheduler/EventThread.h
@@ -131,9 +131,6 @@
virtual VsyncEventData getLatestVsyncEventData(
const sp<EventThreadConnection>& connection) const = 0;
- // Retrieves the number of event connections tracked by this EventThread.
- virtual size_t getEventThreadConnectionCount() = 0;
-
virtual void onNewVsyncSchedule(std::shared_ptr<scheduler::VsyncSchedule>) = 0;
};
@@ -172,8 +169,6 @@
void setDuration(std::chrono::nanoseconds workDuration,
std::chrono::nanoseconds readyDuration) override;
- size_t getEventThreadConnectionCount() override;
-
void onNewVsyncSchedule(std::shared_ptr<scheduler::VsyncSchedule>) override EXCLUDES(mMutex);
private:
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 0a1bd4c..08667bf 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -393,12 +393,6 @@
thread->onModeChanged(mode);
}
-size_t Scheduler::getEventThreadConnectionCount(ConnectionHandle handle) {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle, 0);
- return mConnections[handle].thread->getEventThreadConnectionCount();
-}
-
void Scheduler::dump(ConnectionHandle handle, std::string& result) const {
android::EventThread* thread;
{
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index c44c447..0ffa2d2 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -278,8 +278,6 @@
// Notifies the scheduler when the display size has changed. Called from SF's main thread
void onActiveDisplayAreaChanged(uint32_t displayArea);
- size_t getEventThreadConnectionCount(ConnectionHandle handle);
-
// Stores the preferred refresh rate that an app should run at.
// FrameRateOverride.refreshRateHz == 0 means no preference.
void setPreferredRefreshRateForUid(FrameRateOverride);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2782ce4..8c53c42 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3002,10 +3002,6 @@
}
}
- const size_t sfConnections = mScheduler->getEventThreadConnectionCount(mSfConnectionHandle);
- const size_t appConnections = mScheduler->getEventThreadConnectionCount(mAppConnectionHandle);
- mTimeStats->recordDisplayEventConnectionCount(sfConnections + appConnections);
-
if (isDisplayConnected && !defaultDisplay->isPoweredOn()) {
getRenderEngine().cleanupPostRender();
return;
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 630cef1..368cb41 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -106,7 +106,8 @@
atom->set_client_composition_frames(mTimeStats.clientCompositionFramesLegacy);
atom->set_display_on_millis(mTimeStats.displayOnTimeLegacy);
atom->set_animation_millis(mTimeStats.presentToPresentLegacy.totalTime());
- atom->set_event_connection_count(mTimeStats.displayEventConnectionsCountLegacy);
+ // Deprecated
+ atom->set_event_connection_count(0);
*atom->mutable_frame_duration() =
histogramToProto(mTimeStats.frameDurationLegacy.hist, mMaxPulledHistogramBuckets);
*atom->mutable_render_engine_timing() =
@@ -356,16 +357,6 @@
mTimeStats.refreshRateSwitchesLegacy++;
}
-void TimeStats::recordDisplayEventConnectionCount(int32_t count) {
- if (!mEnabled.load()) return;
-
- ATRACE_CALL();
-
- std::lock_guard<std::mutex> lock(mMutex);
- mTimeStats.displayEventConnectionsCountLegacy =
- std::max(mTimeStats.displayEventConnectionsCountLegacy, count);
-}
-
static int32_t toMs(nsecs_t nanos) {
int64_t millis =
std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::nanoseconds(nanos))
@@ -1072,7 +1063,6 @@
mTimeStats.compositionStrategyPredictedLegacy = 0;
mTimeStats.compositionStrategyPredictionSucceededLegacy = 0;
mTimeStats.refreshRateSwitchesLegacy = 0;
- mTimeStats.displayEventConnectionsCountLegacy = 0;
mTimeStats.displayOnTimeLegacy = 0;
mTimeStats.presentToPresentLegacy.hist.clear();
mTimeStats.frameDurationLegacy.hist.clear();
diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index 5f58657..0c227d4 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -57,9 +57,6 @@
virtual void incrementMissedFrames() = 0;
// Increments the number of times the display refresh rate changed.
virtual void incrementRefreshRateSwitches() = 0;
- // Records the most up-to-date count of display event connections.
- // The stored count will be the maximum ever recoded.
- virtual void recordDisplayEventConnectionCount(int32_t count) = 0;
// Records the start and end times for a frame.
// The start time is the same as the beginning of a SurfaceFlinger
@@ -253,7 +250,6 @@
void incrementTotalFrames() override;
void incrementMissedFrames() override;
void incrementRefreshRateSwitches() override;
- void recordDisplayEventConnectionCount(int32_t count) override;
void recordFrameDuration(nsecs_t startTime, nsecs_t endTime) override;
void recordRenderEngineDuration(nsecs_t startTime, nsecs_t endTime) override;
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h b/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
index 60aa810..9e97f0d 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
+++ b/services/surfaceflinger/TimeStats/timestatsproto/include/timestatsproto/TimeStatsHelper.h
@@ -175,7 +175,6 @@
int32_t clientCompositionReusedFramesLegacy = 0;
int32_t refreshRateSwitchesLegacy = 0;
int32_t compositionStrategyChangesLegacy = 0;
- int32_t displayEventConnectionsCountLegacy = 0;
int64_t displayOnTimeLegacy = 0;
Histogram presentToPresentLegacy;
Histogram frameDurationLegacy;
diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
index 5fed9b4..91c6239 100644
--- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
+++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
@@ -634,31 +634,6 @@
expectVSyncCallbackScheduleReceived(false);
}
-TEST_F(EventThreadTest, tracksEventConnections) {
- setupEventThread(VSYNC_PERIOD);
-
- EXPECT_EQ(2, mThread->getEventThreadConnectionCount());
- ConnectionEventRecorder errorConnectionEventRecorder{NO_MEMORY};
- sp<MockEventThreadConnection> errorConnection = createConnection(errorConnectionEventRecorder);
- mThread->setVsyncRate(1, errorConnection);
- EXPECT_EQ(3, mThread->getEventThreadConnectionCount());
- ConnectionEventRecorder secondConnectionEventRecorder{0};
- sp<MockEventThreadConnection> secondConnection =
- createConnection(secondConnectionEventRecorder);
- mThread->setVsyncRate(1, secondConnection);
- EXPECT_EQ(4, mThread->getEventThreadConnectionCount());
-
- // EventThread should enable vsync callbacks.
- expectVSyncCallbackScheduleReceived(true);
-
- // The first event will be seen by the connection, which then returns an error.
- onVSyncEvent(123, 456, 789);
- expectVsyncEventReceivedByConnection("errorConnection", errorConnectionEventRecorder, 123, 1u);
- expectVsyncEventReceivedByConnection("successConnection", secondConnectionEventRecorder, 123,
- 1u);
- EXPECT_EQ(3, mThread->getEventThreadConnectionCount());
-}
-
TEST_F(EventThreadTest, eventsDroppedIfNonfatalEventDeliveryError) {
setupEventThread(VSYNC_PERIOD);
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 405f79d..3200003 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -158,10 +158,6 @@
EXPECT_CALL(*mEventThread, setDuration(10ns, 20ns)).Times(1);
mScheduler->setDuration(mConnectionHandle, 10ns, 20ns);
-
- static constexpr size_t kEventConnections = 5;
- EXPECT_CALL(*mEventThread, getEventThreadConnectionCount()).WillOnce(Return(kEventConnections));
- EXPECT_EQ(kEventConnections, mScheduler->getEventThreadConnectionCount(mConnectionHandle));
}
TEST_F(SchedulerTest, registerDisplay) FTL_FAKE_GUARD(kMainThreadContext) {
diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
index a9ae1d3..86ed233 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -1079,7 +1079,6 @@
constexpr size_t TOTAL_FRAMES = 5;
constexpr size_t MISSED_FRAMES = 4;
constexpr size_t CLIENT_COMPOSITION_FRAMES = 3;
- constexpr size_t DISPLAY_EVENT_CONNECTIONS = 14;
constexpr nsecs_t DISPLAY_DEADLINE_DELTA = 1'000'000;
constexpr nsecs_t DISPLAY_PRESENT_JITTER = 2'000'000;
constexpr nsecs_t APP_DEADLINE_DELTA = 3'000'000;
@@ -1100,7 +1099,6 @@
insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000);
- mTimeStats->recordDisplayEventConnectionCount(DISPLAY_EVENT_CONNECTIONS);
mTimeStats->setPowerMode(PowerMode::ON);
mTimeStats->recordFrameDuration(1000000, 3000000);
mTimeStats->recordRenderEngineDuration(2000000, 4000000);
@@ -1157,7 +1155,6 @@
EXPECT_EQ(atom.client_composition_frames(), CLIENT_COMPOSITION_FRAMES);
// Display on millis is not checked.
EXPECT_EQ(atom.animation_millis(), 2);
- EXPECT_EQ(atom.event_connection_count(), DISPLAY_EVENT_CONNECTIONS);
EXPECT_THAT(atom.frame_duration(), HistogramEq(buildExpectedHistogram({2}, {1})));
EXPECT_THAT(atom.render_engine_timing(), HistogramEq(buildExpectedHistogram({1, 2}, {1, 1})));
EXPECT_EQ(atom.total_timeline_frames(), 9);
diff --git a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
index 8d57049..9a1a16d 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h
@@ -49,7 +49,6 @@
(const sp<android::EventThreadConnection>&), (const, override));
MOCK_METHOD(void, requestLatestConfig, (const sp<android::EventThreadConnection>&));
MOCK_METHOD(void, pauseVsyncCallback, (bool));
- MOCK_METHOD(size_t, getEventThreadConnectionCount, (), (override));
MOCK_METHOD(void, onNewVsyncSchedule, (std::shared_ptr<scheduler::VsyncSchedule>), (override));
};
diff --git a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
index 86fbadc..c82e45b 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h
@@ -34,7 +34,6 @@
MOCK_METHOD0(incrementTotalFrames, void());
MOCK_METHOD0(incrementMissedFrames, void());
MOCK_METHOD0(incrementRefreshRateSwitches, void());
- MOCK_METHOD1(recordDisplayEventConnectionCount, void(int32_t));
MOCK_METHOD2(recordFrameDuration, void(nsecs_t, nsecs_t));
MOCK_METHOD2(recordRenderEngineDuration, void(nsecs_t, nsecs_t));
MOCK_METHOD2(recordRenderEngineDuration, void(nsecs_t, const std::shared_ptr<FenceTime>&));