SF: Do not duplicate fences per layer per frame
Convert the unique_fd of RenderEngineResult (and futures thereof) into
sp<Fence> such that postFramebuffer does not duplicate release/present
fences.
Remove a few copies of shared futures/pointers with std::move.
Bug: 232436803
Test: simpleperf (-33% cycles in sys_dup)
Change-Id: Ia7c6c8333a712441f3612fb5c720ea2932799636
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/FenceResult.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/FenceResult.h
new file mode 100644
index 0000000..0ce263b
--- /dev/null
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/FenceResult.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <android-base/expected.h>
+#include <utils/Errors.h>
+#include <utils/StrongPointer.h>
+
+// TODO(b/232535621): Pull this file to <ui/FenceResult.h> so that RenderEngine::drawLayers returns
+// FenceResult rather than RenderEngineResult.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wconversion"
+#include <renderengine/RenderEngine.h>
+#pragma clang diagnostic pop
+
+namespace android {
+
+class Fence;
+
+using FenceResult = base::expected<sp<Fence>, status_t>;
+
+// TODO(b/232535621): Prevent base::unexpected(NO_ERROR) from being a valid FenceResult.
+inline status_t fenceStatus(const FenceResult& fenceResult) {
+ return fenceResult.ok() ? NO_ERROR : fenceResult.error();
+}
+
+inline FenceResult toFenceResult(renderengine::RenderEngineResult&& result) {
+ if (auto [status, fence] = std::move(result); fence.ok()) {
+ return sp<Fence>::make(std::move(fence));
+ } else {
+ return base::unexpected(status);
+ }
+}
+
+} // namespace android
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
index e77e155..b7fc62f 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
@@ -21,13 +21,14 @@
#include <ostream>
#include <unordered_set>
+#include <compositionengine/FenceResult.h>
+
// TODO(b/129481165): remove the #pragma below and fix conversion issues
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconversion"
#pragma clang diagnostic ignored "-Wextra"
#include <renderengine/LayerSettings.h>
-#include <renderengine/RenderEngine.h>
// TODO(b/129481165): remove the #pragma below and fix conversion issues
#pragma clang diagnostic pop // ignored "-Wconversion -Wextra"
@@ -156,7 +157,7 @@
ClientCompositionTargetSettings&) = 0;
// Called after the layer is displayed to update the presentation fence
- virtual void onLayerDisplayed(std::shared_future<renderengine::RenderEngineResult>) = 0;
+ virtual void onLayerDisplayed(std::shared_future<FenceResult>) = 0;
// Gets some kind of identifier for the layer for debug purposes.
virtual const char* getDebugName() const = 0;
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
index ee0c53d..871599d 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
@@ -49,7 +49,7 @@
std::vector<compositionengine::LayerFE::LayerSettings>(
compositionengine::LayerFE::ClientCompositionTargetSettings&));
- MOCK_METHOD1(onLayerDisplayed, void(std::shared_future<renderengine::RenderEngineResult>));
+ MOCK_METHOD1(onLayerDisplayed, void(std::shared_future<FenceResult>));
MOCK_CONST_METHOD0(getDebugName, const char*());
MOCK_CONST_METHOD0(getSequence, int32_t());
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp
index 4c30f99..742332c 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -1245,34 +1245,31 @@
// probably to encapsulate the output buffer into a structure that dispatches resource cleanup
// over to RenderEngine, in which case this flag can be removed from the drawLayers interface.
const bool useFramebufferCache = outputState.layerFilter.toInternalDisplay;
- auto [status, drawFence] =
- renderEngine
- .drawLayers(clientCompositionDisplay, clientRenderEngineLayers, tex,
- useFramebufferCache, std::move(fd))
- .get();
- if (status != NO_ERROR && mClientCompositionRequestCache) {
+ auto fenceResult =
+ toFenceResult(renderEngine
+ .drawLayers(clientCompositionDisplay, clientRenderEngineLayers,
+ tex, useFramebufferCache, std::move(fd))
+ .get());
+
+ if (mClientCompositionRequestCache && fenceStatus(fenceResult) != NO_ERROR) {
// If rendering was not successful, remove the request from the cache.
mClientCompositionRequestCache->remove(tex->getBuffer()->getId());
}
- auto& timeStats = getCompositionEngine().getTimeStats();
- if (drawFence.get() < 0) {
- timeStats.recordRenderEngineDuration(renderEngineStart, systemTime());
+ const auto fence = std::move(fenceResult).value_or(Fence::NO_FENCE);
+
+ if (auto& timeStats = getCompositionEngine().getTimeStats(); fence->isValid()) {
+ timeStats.recordRenderEngineDuration(renderEngineStart, std::make_shared<FenceTime>(fence));
} else {
- timeStats.recordRenderEngineDuration(renderEngineStart,
- std::make_shared<FenceTime>(
- new Fence(dup(drawFence.get()))));
+ timeStats.recordRenderEngineDuration(renderEngineStart, systemTime());
}
- if (clientCompositionLayersFE.size() > 0) {
- sp<Fence> clientCompFence = new Fence(dup(drawFence.get()));
- for (auto clientComposedLayer : clientCompositionLayersFE) {
- clientComposedLayer->setWasClientComposed(clientCompFence);
- }
+ for (auto* clientComposedLayer : clientCompositionLayersFE) {
+ clientComposedLayer->setWasClientComposed(fence);
}
- return std::move(drawFence);
+ return base::unique_fd(fence->dup());
}
std::vector<LayerFE::LayerSettings> Output::generateClientCompositionRequests(
@@ -1429,19 +1426,15 @@
Fence::merge("LayerRelease", releaseFence, frame.clientTargetAcquireFence);
}
layer->getLayerFE().onLayerDisplayed(
- ftl::yield<renderengine::RenderEngineResult>(
- {NO_ERROR, base::unique_fd(releaseFence->dup())})
- .share());
+ ftl::yield<FenceResult>(std::move(releaseFence)).share());
}
// We've got a list of layers needing fences, that are disjoint with
// OutputLayersOrderedByZ. The best we can do is to
// supply them with the present fence.
for (auto& weakLayer : mReleasedLayers) {
- if (auto layer = weakLayer.promote(); layer != nullptr) {
- layer->onLayerDisplayed(ftl::yield<renderengine::RenderEngineResult>(
- {NO_ERROR, base::unique_fd(frame.presentFence->dup())})
- .share());
+ if (const auto layer = weakLayer.promote()) {
+ layer->onLayerDisplayed(ftl::yield<FenceResult>(frame.presentFence).share());
}
}
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
index aaca4fe..d15b0a7 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp
@@ -271,13 +271,16 @@
bufferFence.reset(texture->getReadyFence()->dup());
}
- auto [status, drawFence] = renderEngine
- .drawLayers(displaySettings, layerSettings, texture->get(),
- false, std::move(bufferFence))
- .get();
+ constexpr bool kUseFramebufferCache = false;
- if (status == NO_ERROR) {
- mDrawFence = new Fence(drawFence.release());
+ auto fenceResult =
+ toFenceResult(renderEngine
+ .drawLayers(displaySettings, layerSettings, texture->get(),
+ kUseFramebufferCache, std::move(bufferFence))
+ .get());
+
+ if (fenceStatus(fenceResult) == NO_ERROR) {
+ mDrawFence = std::move(fenceResult).value_or(Fence::NO_FENCE);
mOutputSpace = outputState.framebufferSpace;
mTexture = texture;
mTexture->setReadyFence(mDrawFence);
diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
index 3a3c91e..784abea 100644
--- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp
@@ -3171,9 +3171,9 @@
mOutput.mState.isEnabled = true;
// Create three unique fence instances
- sp<Fence> layer1Fence = new Fence();
- sp<Fence> layer2Fence = new Fence();
- sp<Fence> layer3Fence = new Fence();
+ sp<Fence> layer1Fence = sp<Fence>::make();
+ sp<Fence> layer2Fence = sp<Fence>::make();
+ sp<Fence> layer3Fence = sp<Fence>::make();
Output::FrameFences frameFences;
frameFences.layerFences.emplace(&mLayer1.hwc2Layer, layer1Fence);
@@ -3188,23 +3188,17 @@
// are passed. This happens to work with the current implementation, but
// would not survive certain calls like Fence::merge() which would return a
// new instance.
- base::unique_fd layer1FD(layer1Fence->dup());
- base::unique_fd layer2FD(layer2Fence->dup());
- base::unique_fd layer3FD(layer3Fence->dup());
EXPECT_CALL(*mLayer1.layerFE, onLayerDisplayed(_))
- .WillOnce([&layer1FD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layer1FD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&layer1Fence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(layer1Fence), futureFenceResult.get());
});
EXPECT_CALL(*mLayer2.layerFE, onLayerDisplayed(_))
- .WillOnce([&layer2FD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layer2FD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&layer2Fence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(layer2Fence), futureFenceResult.get());
});
EXPECT_CALL(*mLayer3.layerFE, onLayerDisplayed(_))
- .WillOnce([&layer3FD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layer3FD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&layer3Fence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(layer3Fence), futureFenceResult.get());
});
mOutput.postFramebuffer();
@@ -3214,15 +3208,11 @@
mOutput.mState.isEnabled = true;
mOutput.mState.usesClientComposition = true;
- sp<Fence> clientTargetAcquireFence = new Fence();
- sp<Fence> layer1Fence = new Fence();
- sp<Fence> layer2Fence = new Fence();
- sp<Fence> layer3Fence = new Fence();
Output::FrameFences frameFences;
- frameFences.clientTargetAcquireFence = clientTargetAcquireFence;
- frameFences.layerFences.emplace(&mLayer1.hwc2Layer, layer1Fence);
- frameFences.layerFences.emplace(&mLayer2.hwc2Layer, layer2Fence);
- frameFences.layerFences.emplace(&mLayer3.hwc2Layer, layer3Fence);
+ frameFences.clientTargetAcquireFence = sp<Fence>::make();
+ frameFences.layerFences.emplace(&mLayer1.hwc2Layer, sp<Fence>::make());
+ frameFences.layerFences.emplace(&mLayer2.hwc2Layer, sp<Fence>::make());
+ frameFences.layerFences.emplace(&mLayer3.hwc2Layer, sp<Fence>::make());
EXPECT_CALL(*mRenderSurface, flip());
EXPECT_CALL(mOutput, presentAndGetFrameFences()).WillOnce(Return(frameFences));
@@ -3256,7 +3246,7 @@
mOutput.setReleasedLayers(std::move(layers));
// Set up a fake present fence
- sp<Fence> presentFence = new Fence();
+ sp<Fence> presentFence = sp<Fence>::make();
Output::FrameFences frameFences;
frameFences.presentFence = presentFence;
@@ -3265,21 +3255,17 @@
EXPECT_CALL(*mRenderSurface, onPresentDisplayCompleted());
// Each released layer should be given the presentFence.
- base::unique_fd layerFD(presentFence.get()->dup());
EXPECT_CALL(*releasedLayer1, onLayerDisplayed(_))
- .WillOnce([&layerFD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layerFD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&presentFence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get());
});
EXPECT_CALL(*releasedLayer2, onLayerDisplayed(_))
- .WillOnce([&layerFD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layerFD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&presentFence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get());
});
EXPECT_CALL(*releasedLayer3, onLayerDisplayed(_))
- .WillOnce([&layerFD](std::shared_future<renderengine::RenderEngineResult>
- futureRenderEngineResult) {
- EXPECT_EQ(layerFD, futureRenderEngineResult.get().drawFence);
+ .WillOnce([&presentFence](std::shared_future<FenceResult> futureFenceResult) {
+ EXPECT_EQ(FenceResult(presentFence), futureFenceResult.get());
});
mOutput.postFramebuffer();