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.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index 6979f03..e6c58a3 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -60,14 +60,6 @@
#include "VsyncController.h"
#include "VsyncSchedule.h"
-#define RETURN_IF_INVALID_HANDLE(handle, ...) \
- do { \
- if (mConnections.count(handle) == 0) { \
- ALOGE("Invalid connection handle %" PRIuPTR, handle.id); \
- return __VA_ARGS__; \
- } \
- } while (false)
-
namespace android::scheduler {
Scheduler::Scheduler(ICompositor& compositor, ISchedulerCallback& callback, FeatureFlags features,
@@ -344,40 +336,26 @@
}
}
-ConnectionHandle Scheduler::createEventThread(Cycle cycle,
- frametimeline::TokenManager* tokenManager,
- std::chrono::nanoseconds workDuration,
- std::chrono::nanoseconds readyDuration) {
+void Scheduler::createEventThread(Cycle cycle, frametimeline::TokenManager* tokenManager,
+ std::chrono::nanoseconds workDuration,
+ std::chrono::nanoseconds readyDuration) {
auto eventThread =
std::make_unique<android::impl::EventThread>(cycle == Cycle::Render ? "app" : "appSf",
getVsyncSchedule(), tokenManager, *this,
workDuration, readyDuration);
- auto& handle = cycle == Cycle::Render ? mAppConnectionHandle : mSfConnectionHandle;
- handle = createConnection(std::move(eventThread));
- return handle;
-}
-
-ConnectionHandle Scheduler::createConnection(std::unique_ptr<EventThread> eventThread) {
- const ConnectionHandle handle = ConnectionHandle{mNextConnectionHandleId++};
- ALOGV("Creating a connection handle with ID %" PRIuPTR, handle.id);
-
- auto connection = eventThread->createEventConnection();
-
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- mConnections.emplace(handle, Connection{connection, std::move(eventThread)});
- return handle;
+ if (cycle == Cycle::Render) {
+ mRenderEventThread = std::move(eventThread);
+ mRenderEventConnection = mRenderEventThread->createEventConnection();
+ } else {
+ mLastCompositeEventThread = std::move(eventThread);
+ mLastCompositeEventConnection = mLastCompositeEventThread->createEventConnection();
+ }
}
sp<IDisplayEventConnection> Scheduler::createDisplayEventConnection(
- ConnectionHandle handle, EventRegistrationFlags eventRegistration,
- const sp<IBinder>& layerHandle) {
- const auto connection = [&]() -> sp<EventThreadConnection> {
- std::scoped_lock lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle, nullptr);
-
- return mConnections[handle].thread->createEventConnection(eventRegistration);
- }();
+ Cycle cycle, EventRegistrationFlags eventRegistration, const sp<IBinder>& layerHandle) {
+ const auto connection = eventThreadFor(cycle).createEventConnection(eventRegistration);
const auto layerId = static_cast<int32_t>(LayerHandle::getLayerId(layerHandle));
if (layerId != static_cast<int32_t>(UNASSIGNED_LAYER_ID)) {
@@ -397,85 +375,48 @@
return connection;
}
-sp<EventThreadConnection> Scheduler::getEventConnection(ConnectionHandle handle) {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle, nullptr);
- return mConnections[handle].connection;
+void Scheduler::onHotplugReceived(Cycle cycle, PhysicalDisplayId displayId, bool connected) {
+ if (hasEventThreads()) {
+ eventThreadFor(cycle).onHotplugReceived(displayId, connected);
+ }
}
-void Scheduler::onHotplugReceived(ConnectionHandle handle, PhysicalDisplayId displayId,
- bool connected) {
- android::EventThread* thread;
- {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle);
- thread = mConnections[handle].thread.get();
+void Scheduler::onHotplugConnectionError(Cycle cycle, int32_t errorCode) {
+ if (hasEventThreads()) {
+ eventThreadFor(cycle).onHotplugConnectionError(errorCode);
}
-
- thread->onHotplugReceived(displayId, connected);
-}
-
-void Scheduler::onHotplugConnectionError(ConnectionHandle handle, int32_t errorCode) {
- android::EventThread* thread;
- {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle);
- thread = mConnections[handle].thread.get();
- }
-
- thread->onHotplugConnectionError(errorCode);
}
void Scheduler::enableSyntheticVsync(bool enable) {
- // TODO(b/241285945): Remove connection handles.
- const ConnectionHandle handle = mAppConnectionHandle;
- android::EventThread* thread;
- {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle);
- thread = mConnections[handle].thread.get();
- }
- thread->enableSyntheticVsync(enable);
+ eventThreadFor(Cycle::Render).enableSyntheticVsync(enable);
}
-void Scheduler::onFrameRateOverridesChanged(ConnectionHandle handle, PhysicalDisplayId displayId) {
+void Scheduler::onFrameRateOverridesChanged(Cycle cycle, PhysicalDisplayId displayId) {
const bool supportsFrameRateOverrideByContent =
pacesetterSelectorPtr()->supportsAppFrameRateOverrideByContent();
std::vector<FrameRateOverride> overrides =
mFrameRateOverrideMappings.getAllFrameRateOverrides(supportsFrameRateOverrideByContent);
- android::EventThread* thread;
- {
- std::lock_guard lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle);
- thread = mConnections[handle].thread.get();
- }
- thread->onFrameRateOverridesChanged(displayId, std::move(overrides));
+ eventThreadFor(cycle).onFrameRateOverridesChanged(displayId, std::move(overrides));
}
-void Scheduler::onHdcpLevelsChanged(ConnectionHandle handle, PhysicalDisplayId displayId,
+void Scheduler::onHdcpLevelsChanged(Cycle cycle, PhysicalDisplayId displayId,
int32_t connectedLevel, int32_t maxLevel) {
- android::EventThread* thread;
- {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle);
- thread = mConnections[handle].thread.get();
- }
- thread->onHdcpLevelsChanged(displayId, connectedLevel, maxLevel);
+ eventThreadFor(cycle).onHdcpLevelsChanged(displayId, connectedLevel, maxLevel);
}
-void Scheduler::onPrimaryDisplayModeChanged(ConnectionHandle handle, const FrameRateMode& mode) {
+void Scheduler::onPrimaryDisplayModeChanged(Cycle cycle, const FrameRateMode& mode) {
{
std::lock_guard<std::mutex> lock(mPolicyLock);
// Cache the last reported modes for primary display.
- mPolicy.cachedModeChangedParams = {handle, mode};
+ mPolicy.cachedModeChangedParams = {cycle, mode};
// Invalidate content based refresh rate selection so it could be calculated
// again for the new refresh rate.
mPolicy.contentRequirements.clear();
}
- onNonPrimaryDisplayModeChanged(handle, mode);
+ onNonPrimaryDisplayModeChanged(cycle, mode);
}
void Scheduler::dispatchCachedReportedMode() {
@@ -502,39 +443,25 @@
}
mPolicy.cachedModeChangedParams->mode = *mPolicy.modeOpt;
- onNonPrimaryDisplayModeChanged(mPolicy.cachedModeChangedParams->handle,
+ onNonPrimaryDisplayModeChanged(mPolicy.cachedModeChangedParams->cycle,
mPolicy.cachedModeChangedParams->mode);
}
-void Scheduler::onNonPrimaryDisplayModeChanged(ConnectionHandle handle, const FrameRateMode& mode) {
- android::EventThread* thread;
- {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle);
- thread = mConnections[handle].thread.get();
+void Scheduler::onNonPrimaryDisplayModeChanged(Cycle cycle, const FrameRateMode& mode) {
+ if (hasEventThreads()) {
+ eventThreadFor(cycle).onModeChanged(mode);
}
- thread->onModeChanged(mode);
}
-void Scheduler::dump(ConnectionHandle handle, std::string& result) const {
- android::EventThread* thread;
- {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle);
- thread = mConnections.at(handle).thread.get();
- }
- thread->dump(result);
+void Scheduler::dump(Cycle cycle, std::string& result) const {
+ eventThreadFor(cycle).dump(result);
}
-void Scheduler::setDuration(ConnectionHandle handle, std::chrono::nanoseconds workDuration,
+void Scheduler::setDuration(Cycle cycle, std::chrono::nanoseconds workDuration,
std::chrono::nanoseconds readyDuration) {
- android::EventThread* thread;
- {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- RETURN_IF_INVALID_HANDLE(handle);
- thread = mConnections[handle].thread.get();
+ if (hasEventThreads()) {
+ eventThreadFor(cycle).setDuration(workDuration, readyDuration);
}
- thread->setDuration(workDuration, readyDuration);
}
void Scheduler::updatePhaseConfiguration(Fps refreshRate) {
@@ -557,10 +484,10 @@
}
void Scheduler::setVsyncConfig(const VsyncConfig& config, Period vsyncPeriod) {
- setDuration(mAppConnectionHandle,
+ setDuration(Cycle::Render,
/* workDuration */ config.appWorkDuration,
/* readyDuration */ config.sfWorkDuration);
- setDuration(mSfConnectionHandle,
+ setDuration(Cycle::LastComposite,
/* workDuration */ vsyncPeriod,
/* readyDuration */ config.sfWorkDuration);
setDuration(config.sfWorkDuration);
@@ -1027,16 +954,10 @@
void Scheduler::applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule> vsyncSchedule) {
onNewVsyncSchedule(vsyncSchedule->getDispatch());
- std::vector<android::EventThread*> threads;
- {
- std::lock_guard<std::mutex> lock(mConnectionsLock);
- threads.reserve(mConnections.size());
- for (auto& [_, connection] : mConnections) {
- threads.push_back(connection.thread.get());
- }
- }
- for (auto* thread : threads) {
- thread->onNewVsyncSchedule(vsyncSchedule);
+
+ if (hasEventThreads()) {
+ eventThreadFor(Cycle::Render).onNewVsyncSchedule(vsyncSchedule);
+ eventThreadFor(Cycle::LastComposite).onNewVsyncSchedule(vsyncSchedule);
}
}
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;
};