Reland "Add a DisplayCapability for multi-threaded present"
Originally landed as I5dbb01fc23abd5e0108c565f96d25e62c77fc16d and
previously reverted in I6265f8de3db31f07506906cee82a91fe3baac0bc due to
timeline constraints.
Add a test presenting from multiple threads. Split up execute() so that
it can be called for a single display. Update MultiThreadedPresent
(test) to offload presentDisplay to a separate thread, similar to how
Ib9d074671e32c95875ef7e0791dd95d6e595e47a does it, as described in
go/multi-threaded-present.
Bug: 259132483
Bug: 284156408
Fixes: 251842321
Fixes: 295841597
Test: VtsHalGraphicsComposer3_TargetTest
Change-Id: If975ee9bb0b9c6f64ef50401e2aee32f934e3f08
diff --git a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp
index 1e6f34b..67c7b5a 100644
--- a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp
+++ b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_TargetTest.cpp
@@ -34,6 +34,7 @@
#include <ui/PixelFormat.h>
#include <algorithm>
#include <iterator>
+#include <mutex>
#include <numeric>
#include <string>
#include <thread>
@@ -1381,21 +1382,17 @@
void execute() {
std::vector<CommandResultPayload> payloads;
for (auto& [_, writer] : mWriters) {
- auto commands = writer.takePendingCommands();
- if (commands.empty()) {
- continue;
- }
-
- auto [status, results] = mComposerClient->executeCommands(commands);
- ASSERT_TRUE(status.isOk()) << "executeCommands failed " << status.getDescription();
-
- payloads.reserve(payloads.size() + results.size());
- payloads.insert(payloads.end(), std::make_move_iterator(results.begin()),
- std::make_move_iterator(results.end()));
+ executeInternal(writer, payloads);
}
mReader.parse(std::move(payloads));
}
+ void execute(ComposerClientWriter& writer, ComposerClientReader& reader) {
+ std::vector<CommandResultPayload> payloads;
+ executeInternal(writer, payloads);
+ reader.parse(std::move(payloads));
+ }
+
static inline auto toTimePoint(nsecs_t time) {
return std::chrono::time_point<std::chrono::steady_clock>(std::chrono::nanoseconds(time));
}
@@ -1720,6 +1717,7 @@
// clang-format on
ComposerClientWriter& getWriter(int64_t display) {
+ std::lock_guard guard{mWritersMutex};
auto [it, _] = mWriters.try_emplace(display, display);
return it->second;
}
@@ -1727,7 +1725,27 @@
ComposerClientReader mReader;
private:
- std::unordered_map<int64_t, ComposerClientWriter> mWriters;
+ void executeInternal(ComposerClientWriter& writer,
+ std::vector<CommandResultPayload>& payloads) {
+ auto commands = writer.takePendingCommands();
+ if (commands.empty()) {
+ return;
+ }
+
+ auto [status, results] = mComposerClient->executeCommands(commands);
+ ASSERT_TRUE(status.isOk()) << "executeCommands failed " << status.getDescription();
+
+ payloads.reserve(payloads.size() + results.size());
+ payloads.insert(payloads.end(), std::make_move_iterator(results.begin()),
+ std::make_move_iterator(results.end()));
+ }
+
+ // Guards access to the map itself. Callers must ensure not to attempt to
+ // - modify the same writer from multiple threads
+ // - insert a new writer into the map during concurrent access, which would invalidate
+ // references from other threads
+ std::mutex mWritersMutex;
+ std::unordered_map<int64_t, ComposerClientWriter> mWriters GUARDED_BY(mWritersMutex);
};
TEST_P(GraphicsComposerAidlCommandTest, SetColorTransform) {
@@ -2785,6 +2803,106 @@
}
}
+TEST_P(GraphicsComposerAidlCommandTest, MultiThreadedPresent) {
+ std::vector<VtsDisplay*> displays;
+ for (auto& display : mDisplays) {
+ if (hasDisplayCapability(display.getDisplayId(),
+ DisplayCapability::MULTI_THREADED_PRESENT)) {
+ displays.push_back(&display);
+ }
+ }
+
+ const size_t numDisplays = displays.size();
+ if (numDisplays <= 1u) {
+ GTEST_SKIP();
+ }
+
+ // When multi-threaded, use a reader per display. As with mWriters, this mutex
+ // guards access to the map.
+ std::mutex readersMutex;
+ std::unordered_map<int64_t, ComposerClientReader> readers;
+ std::vector<std::thread> threads;
+ threads.reserve(numDisplays);
+
+ // Each display will have a layer to present. This maps from the display to
+ // the layer, so we can properly destroy each layer at the end.
+ std::unordered_map<int64_t, int64_t> layers;
+
+ for (auto* const display : displays) {
+ const int64_t displayId = display->getDisplayId();
+
+ // Ensure that all writers and readers have been added to their respective
+ // maps initially, so that the following loop never modifies the maps. The
+ // maps are accessed from different threads, and if the maps were modified,
+ // this would invalidate their iterators, and therefore references to the
+ // writers and readers.
+ auto& writer = getWriter(displayId);
+ {
+ std::lock_guard guard{readersMutex};
+ readers.try_emplace(displayId, displayId);
+ }
+
+ EXPECT_TRUE(mComposerClient->setPowerMode(displayId, PowerMode::ON).isOk());
+
+ const auto& [status, layer] = mComposerClient->createLayer(displayId, kBufferSlotCount);
+ const auto buffer = allocate(::android::PIXEL_FORMAT_RGBA_8888);
+ ASSERT_NE(nullptr, buffer);
+ ASSERT_EQ(::android::OK, buffer->initCheck());
+ ASSERT_NE(nullptr, buffer->handle);
+
+ configureLayer(*display, layer, Composition::DEVICE, display->getFrameRect(),
+ display->getCrop());
+ writer.setLayerBuffer(displayId, layer, /*slot*/ 0, buffer->handle,
+ /*acquireFence*/ -1);
+ writer.setLayerDataspace(displayId, layer, common::Dataspace::UNKNOWN);
+ layers.try_emplace(displayId, layer);
+ }
+
+ for (auto* const display : displays) {
+ const int64_t displayId = display->getDisplayId();
+ auto& writer = getWriter(displayId);
+ std::unique_lock lock{readersMutex};
+ auto& reader = readers.at(displayId);
+ lock.unlock();
+
+ writer.validateDisplay(displayId, ComposerClientWriter::kNoTimestamp);
+ execute(writer, reader);
+
+ threads.emplace_back([this, displayId, &readers, &readersMutex]() {
+ auto& writer = getWriter(displayId);
+ std::unique_lock lock{readersMutex};
+ ComposerClientReader& reader = readers.at(displayId);
+ lock.unlock();
+
+ writer.presentDisplay(displayId);
+ execute(writer, reader);
+ ASSERT_TRUE(reader.takeErrors().empty());
+
+ auto presentFence = reader.takePresentFence(displayId);
+ // take ownership
+ const int fenceOwner = presentFence.get();
+ *presentFence.getR() = -1;
+ EXPECT_NE(-1, fenceOwner);
+ const auto presentFence2 = sp<::android::Fence>::make(fenceOwner);
+ presentFence2->waitForever(LOG_TAG);
+ });
+ }
+
+ for (auto& thread : threads) {
+ thread.join();
+ }
+
+ for (auto& [displayId, layer] : layers) {
+ EXPECT_TRUE(mComposerClient->destroyLayer(displayId, layer).isOk());
+ }
+
+ std::lock_guard guard{readersMutex};
+ for (auto& [displayId, reader] : readers) {
+ ASSERT_TRUE(reader.takeErrors().empty());
+ ASSERT_TRUE(reader.takeChangedCompositionTypes(displayId).empty());
+ }
+}
+
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(GraphicsComposerAidlCommandTest);
INSTANTIATE_TEST_SUITE_P(
PerInstance, GraphicsComposerAidlCommandTest,