SF: Remove ConnectionHandle concept in Scheduler
This abstraction never made sense, and only incurs pointless locking and
hash map lookups. This is a step toward replacing the two EventThreads
with a single EventDispatcher.
Start the Scheduler's timers after initialization. This solves the same
bug as Ib2617b914145bc4180cc7ca27203c59dbd625c94, which avoided crashes
without fixing the root cause of the race.
In a few cases, initScheduler indirectly calls Scheduler functions that
attempt EventThread dispatch before EventThread creation, so skip those
EventThread accesses using hasEventThreads for now. Further refactoring
will untangle circular dependencies and remove hasEventThreads.
Bug: 241285191
Bug: 241285945
Test: presubmit
Change-Id: I89b93b47540ab24bb5675edc79370f38bc48114f
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 9f29e9f..71df507 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -56,35 +56,6 @@
#include <FrontEnd/LayerHierarchy.h>
-namespace android::scheduler {
-
-// Opaque handle to scheduler connection.
-struct ConnectionHandle {
- using Id = std::uintptr_t;
- static constexpr Id INVALID_ID = static_cast<Id>(-1);
-
- Id id = INVALID_ID;
-
- explicit operator bool() const { return id != INVALID_ID; }
-};
-
-inline bool operator==(ConnectionHandle lhs, ConnectionHandle rhs) {
- return lhs.id == rhs.id;
-}
-
-} // namespace android::scheduler
-
-namespace std {
-
-template <>
-struct hash<android::scheduler::ConnectionHandle> {
- size_t operator()(android::scheduler::ConnectionHandle handle) const {
- return hash<android::scheduler::ConnectionHandle::Id>()(handle.id);
- }
-};
-
-} // namespace std
-
namespace android {
class FenceTime;
@@ -106,6 +77,11 @@
class VsyncConfiguration;
class VsyncSchedule;
+enum class Cycle {
+ Render, // Surface rendering.
+ LastComposite // Ahead of display compositing by one refresh period.
+};
+
class Scheduler : public IEventThreadCallback, android::impl::MessageQueue {
using Impl = android::impl::MessageQueue;
@@ -154,36 +130,32 @@
return std::move(future);
}
- enum class Cycle {
- Render, // Surface rendering.
- LastComposite // Ahead of display compositing by one refresh period.
- };
-
- ConnectionHandle createEventThread(Cycle, frametimeline::TokenManager*,
- std::chrono::nanoseconds workDuration,
- std::chrono::nanoseconds readyDuration);
+ void createEventThread(Cycle, frametimeline::TokenManager*,
+ std::chrono::nanoseconds workDuration,
+ std::chrono::nanoseconds readyDuration);
sp<IDisplayEventConnection> createDisplayEventConnection(
- ConnectionHandle, EventRegistrationFlags eventRegistration = {},
+ Cycle, EventRegistrationFlags eventRegistration = {},
const sp<IBinder>& layerHandle = nullptr) EXCLUDES(mChoreographerLock);
- sp<EventThreadConnection> getEventConnection(ConnectionHandle);
+ const sp<EventThreadConnection>& getEventConnection(Cycle cycle) const {
+ return cycle == Cycle::Render ? mRenderEventConnection : mLastCompositeEventConnection;
+ }
- void onHotplugReceived(ConnectionHandle, PhysicalDisplayId, bool connected);
- void onHotplugConnectionError(ConnectionHandle, int32_t errorCode);
+ void onHotplugReceived(Cycle, PhysicalDisplayId, bool connected);
+ void onHotplugConnectionError(Cycle, int32_t errorCode);
- void onPrimaryDisplayModeChanged(ConnectionHandle, const FrameRateMode&) EXCLUDES(mPolicyLock);
- void onNonPrimaryDisplayModeChanged(ConnectionHandle, const FrameRateMode&);
+ void onPrimaryDisplayModeChanged(Cycle, const FrameRateMode&) EXCLUDES(mPolicyLock);
+ void onNonPrimaryDisplayModeChanged(Cycle, const FrameRateMode&);
void enableSyntheticVsync(bool = true) REQUIRES(kMainThreadContext);
- void onFrameRateOverridesChanged(ConnectionHandle, PhysicalDisplayId)
- EXCLUDES(mConnectionsLock);
+ void onFrameRateOverridesChanged(Cycle, PhysicalDisplayId);
- void onHdcpLevelsChanged(ConnectionHandle, PhysicalDisplayId, int32_t, int32_t);
+ void onHdcpLevelsChanged(Cycle, PhysicalDisplayId, int32_t, int32_t);
// Modifies work duration in the event thread.
- void setDuration(ConnectionHandle, std::chrono::nanoseconds workDuration,
+ void setDuration(Cycle, std::chrono::nanoseconds workDuration,
std::chrono::nanoseconds readyDuration);
VsyncModulator& vsyncModulator() { return *mVsyncModulator; }
@@ -288,7 +260,7 @@
bool isVsyncInPhase(TimePoint expectedVsyncTime, Fps frameRate) const;
void dump(utils::Dumper&) const;
- void dump(ConnectionHandle, std::string&) const;
+ void dump(Cycle, std::string&) const;
void dumpVsync(std::string&) const EXCLUDES(mDisplayLock);
// Returns the preferred refresh rate and frame rate for the pacesetter display.
@@ -369,8 +341,15 @@
void onFrameSignal(ICompositor&, VsyncId, TimePoint expectedVsyncTime) override
REQUIRES(kMainThreadContext, mDisplayLock);
- // Create a connection on the given EventThread.
- ConnectionHandle createConnection(std::unique_ptr<EventThread>);
+ // Used to skip event dispatch before EventThread creation during boot.
+ // TODO: b/241285191 - Reorder Scheduler initialization to avoid this.
+ bool hasEventThreads() const {
+ return CC_LIKELY(mRenderEventThread && mLastCompositeEventThread);
+ }
+
+ EventThread& eventThreadFor(Cycle cycle) const {
+ return *(cycle == Cycle::Render ? mRenderEventThread : mLastCompositeEventThread);
+ }
// Update feature state machine to given state when corresponding timer resets or expires.
void kernelIdleTimerCallback(TimerState) EXCLUDES(mDisplayLock);
@@ -460,18 +439,11 @@
void resync() override EXCLUDES(mDisplayLock);
void onExpectedPresentTimePosted(TimePoint expectedPresentTime) override EXCLUDES(mDisplayLock);
- // Stores EventThread associated with a given VSyncSource, and an initial EventThreadConnection.
- struct Connection {
- sp<EventThreadConnection> connection;
- std::unique_ptr<EventThread> thread;
- };
+ std::unique_ptr<EventThread> mRenderEventThread;
+ sp<EventThreadConnection> mRenderEventConnection;
- ConnectionHandle::Id mNextConnectionHandleId = 0;
- mutable std::mutex mConnectionsLock;
- std::unordered_map<ConnectionHandle, Connection> mConnections GUARDED_BY(mConnectionsLock);
-
- ConnectionHandle mAppConnectionHandle;
- ConnectionHandle mSfConnectionHandle;
+ std::unique_ptr<EventThread> mLastCompositeEventThread;
+ sp<EventThreadConnection> mLastCompositeEventConnection;
std::atomic<nsecs_t> mLastResyncTime = 0;
@@ -585,7 +557,7 @@
ftl::Optional<FrameRateMode> modeOpt;
struct ModeChangedParams {
- ConnectionHandle handle;
+ Cycle cycle;
FrameRateMode mode;
};