Adjust how present semaphore is handled
Avoids stashing a semaphore in a field
use unique_fd to ensure an unused present fence isn't leaked
Test: sample app ran in a loop for 3 hours, FD usage remained constant
Fixes: 295332012
Change-Id: Iee38e86587805ef3596e5f7efea3ca3b5c7758b3
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 7fac0c9..362bd9d 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -664,8 +664,8 @@
bool didDraw = false;
int error = OK;
- bool didSwap = mRenderPipeline->swapBuffers(frame, drawResult.success, windowDirty,
- mCurrentFrameInfo, &requireSwap);
+ bool didSwap = mRenderPipeline->swapBuffers(frame, drawResult, windowDirty, mCurrentFrameInfo,
+ &requireSwap);
mCurrentFrameInfo->set(FrameInfoIndex::CommandSubmissionCompleted) = std::max(
drawResult.commandSubmissionTime, mCurrentFrameInfo->get(FrameInfoIndex::SwapBuffers));
diff --git a/libs/hwui/renderthread/IRenderPipeline.h b/libs/hwui/renderthread/IRenderPipeline.h
index 6c2cb9d..9c879d5 100644
--- a/libs/hwui/renderthread/IRenderPipeline.h
+++ b/libs/hwui/renderthread/IRenderPipeline.h
@@ -61,6 +61,7 @@
// submission occurred. -1 if this time is unknown.
static constexpr nsecs_t kUnknownTime = -1;
nsecs_t commandSubmissionTime = kUnknownTime;
+ android::base::unique_fd presentFence;
};
virtual DrawResult draw(const Frame& frame, const SkRect& screenDirty, const SkRect& dirty,
const LightGeometry& lightGeometry, LayerUpdateQueue* layerUpdateQueue,
@@ -68,8 +69,9 @@
const std::vector<sp<RenderNode>>& renderNodes,
FrameInfoVisualizer* profiler,
const HardwareBufferRenderParams& bufferParams) = 0;
- virtual bool swapBuffers(const Frame& frame, bool drew, const SkRect& screenDirty,
- FrameInfo* currentFrameInfo, bool* requireSwap) = 0;
+ virtual bool swapBuffers(const Frame& frame, IRenderPipeline::DrawResult&,
+ const SkRect& screenDirty, FrameInfo* currentFrameInfo,
+ bool* requireSwap) = 0;
virtual DeferredLayerUpdater* createTextureLayer() = 0;
[[nodiscard]] virtual android::base::unique_fd flush() = 0;
virtual void setHardwareBuffer(AHardwareBuffer* hardwareBuffer) = 0;
diff --git a/libs/hwui/renderthread/VulkanManager.cpp b/libs/hwui/renderthread/VulkanManager.cpp
index 22c5862..e706eb0 100644
--- a/libs/hwui/renderthread/VulkanManager.cpp
+++ b/libs/hwui/renderthread/VulkanManager.cpp
@@ -531,32 +531,21 @@
PFN_vkDestroySemaphore mDestroyFunction;
VkDevice mDevice;
VkSemaphore mSemaphore;
- // We need to make sure we don't delete the VkSemaphore until it is done being used by both Skia
- // (including by the GPU) and inside the VulkanManager. So we always start with two refs, one
- // owned by Skia and one owned by the VulkanManager. The refs are decremented each time
- // destroy_semaphore is called with this object. Skia will call destroy_semaphore once it is
- // done with the semaphore and the GPU has finished work on the semaphore. The VulkanManager
- // calls destroy_semaphore after sending the semaphore to Skia and exporting it if need be.
- int mRefs = 2;
DestroySemaphoreInfo(PFN_vkDestroySemaphore destroyFunction, VkDevice device,
VkSemaphore semaphore)
: mDestroyFunction(destroyFunction), mDevice(device), mSemaphore(semaphore) {}
+
+ ~DestroySemaphoreInfo() { mDestroyFunction(mDevice, mSemaphore, nullptr); }
};
static void destroy_semaphore(void* context) {
DestroySemaphoreInfo* info = reinterpret_cast<DestroySemaphoreInfo*>(context);
- --info->mRefs;
- if (!info->mRefs) {
- info->mDestroyFunction(info->mDevice, info->mSemaphore, nullptr);
- delete info;
- }
+ delete info;
}
-nsecs_t VulkanManager::finishFrame(SkSurface* surface) {
+VulkanManager::VkDrawResult VulkanManager::finishFrame(SkSurface* surface) {
ATRACE_NAME("Vulkan finish frame");
- ALOGE_IF(mSwapSemaphore != VK_NULL_HANDLE || mDestroySemaphoreContext != nullptr,
- "finishFrame already has an outstanding semaphore");
VkExportSemaphoreCreateInfo exportInfo;
exportInfo.sType = VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO;
@@ -576,11 +565,11 @@
GrFlushInfo flushInfo;
if (err == VK_SUCCESS) {
- mDestroySemaphoreContext = new DestroySemaphoreInfo(mDestroySemaphore, mDevice, semaphore);
flushInfo.fNumSemaphores = 1;
flushInfo.fSignalSemaphores = &backendSemaphore;
flushInfo.fFinishedProc = destroy_semaphore;
- flushInfo.fFinishedContext = mDestroySemaphoreContext;
+ flushInfo.fFinishedContext =
+ new DestroySemaphoreInfo(mDestroySemaphore, mDevice, semaphore);
} else {
semaphore = VK_NULL_HANDLE;
}
@@ -589,10 +578,11 @@
GrSemaphoresSubmitted submitted = context->flush(
surface, SkSurfaces::BackendSurfaceAccess::kPresent, flushInfo);
context->submit();
- const nsecs_t submissionTime = systemTime();
+ VkDrawResult drawResult{
+ .submissionTime = systemTime(),
+ };
if (semaphore != VK_NULL_HANDLE) {
if (submitted == GrSemaphoresSubmitted::kYes) {
- mSwapSemaphore = semaphore;
if (mFrameBoundaryANDROID) {
// retrieve VkImage used as render target
VkImage image = VK_NULL_HANDLE;
@@ -611,45 +601,37 @@
}
// frameBoundaryANDROID needs to know about mSwapSemaphore, but
// it won't wait on it.
- mFrameBoundaryANDROID(mDevice, mSwapSemaphore, image);
+ mFrameBoundaryANDROID(mDevice, semaphore, image);
}
- } else {
- destroy_semaphore(mDestroySemaphoreContext);
- mDestroySemaphoreContext = nullptr;
}
+ VkSemaphoreGetFdInfoKHR getFdInfo;
+ getFdInfo.sType = VK_STRUCTURE_TYPE_SEMAPHORE_GET_FD_INFO_KHR;
+ getFdInfo.pNext = nullptr;
+ getFdInfo.semaphore = semaphore;
+ getFdInfo.handleType = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT;
+
+ int fenceFd = -1;
+ err = mGetSemaphoreFdKHR(mDevice, &getFdInfo, &fenceFd);
+ ALOGE_IF(VK_SUCCESS != err, "VulkanManager::swapBuffers(): Failed to get semaphore Fd");
+ drawResult.presentFence.reset(fenceFd);
+ } else {
+ ALOGE("VulkanManager::finishFrame(): Semaphore submission failed");
+ mQueueWaitIdle(mGraphicsQueue);
}
+
skiapipeline::ShaderCache::get().onVkFrameFlushed(context);
- return submissionTime;
+ return drawResult;
}
-void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect) {
+void VulkanManager::swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect,
+ android::base::unique_fd&& presentFence) {
if (CC_UNLIKELY(Properties::waitForGpuCompletion)) {
ATRACE_NAME("Finishing GPU work");
mDeviceWaitIdle(mDevice);
}
- int fenceFd = -1;
- if (mSwapSemaphore != VK_NULL_HANDLE) {
- VkSemaphoreGetFdInfoKHR getFdInfo;
- getFdInfo.sType = VK_STRUCTURE_TYPE_SEMAPHORE_GET_FD_INFO_KHR;
- getFdInfo.pNext = nullptr;
- getFdInfo.semaphore = mSwapSemaphore;
- getFdInfo.handleType = VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT;
-
- VkResult err = mGetSemaphoreFdKHR(mDevice, &getFdInfo, &fenceFd);
- ALOGE_IF(VK_SUCCESS != err, "VulkanManager::swapBuffers(): Failed to get semaphore Fd");
- } else {
- ALOGE("VulkanManager::swapBuffers(): Semaphore submission failed");
- mQueueWaitIdle(mGraphicsQueue);
- }
- if (mDestroySemaphoreContext) {
- destroy_semaphore(mDestroySemaphoreContext);
- }
-
- surface->presentCurrentBuffer(dirtyRect, fenceFd);
- mSwapSemaphore = VK_NULL_HANDLE;
- mDestroySemaphoreContext = nullptr;
+ surface->presentCurrentBuffer(dirtyRect, presentFence.release());
}
void VulkanManager::destroySurface(VulkanSurface* surface) {
@@ -753,22 +735,17 @@
GrBackendSemaphore backendSemaphore;
backendSemaphore.initVulkan(semaphore);
- DestroySemaphoreInfo* destroyInfo =
- new DestroySemaphoreInfo(mDestroySemaphore, mDevice, semaphore);
// Even if Skia fails to submit the semaphore, it will still call the destroy_semaphore callback
- // which will remove its ref to the semaphore. The VulkanManager must still release its ref,
- // when it is done with the semaphore.
GrFlushInfo flushInfo;
flushInfo.fNumSemaphores = 1;
flushInfo.fSignalSemaphores = &backendSemaphore;
flushInfo.fFinishedProc = destroy_semaphore;
- flushInfo.fFinishedContext = destroyInfo;
+ flushInfo.fFinishedContext = new DestroySemaphoreInfo(mDestroySemaphore, mDevice, semaphore);
GrSemaphoresSubmitted submitted = grContext->flush(flushInfo);
grContext->submit();
if (submitted == GrSemaphoresSubmitted::kNo) {
ALOGE("VulkanManager::createReleaseFence: Failed to submit semaphore");
- destroy_semaphore(destroyInfo);
return INVALID_OPERATION;
}
@@ -781,7 +758,6 @@
int fenceFd = 0;
err = mGetSemaphoreFdKHR(mDevice, &getFdInfo, &fenceFd);
- destroy_semaphore(destroyInfo);
if (VK_SUCCESS != err) {
ALOGE("VulkanManager::createReleaseFence: Failed to get semaphore Fd");
return INVALID_OPERATION;
diff --git a/libs/hwui/renderthread/VulkanManager.h b/libs/hwui/renderthread/VulkanManager.h
index dbef7fb..b92ebb3 100644
--- a/libs/hwui/renderthread/VulkanManager.h
+++ b/libs/hwui/renderthread/VulkanManager.h
@@ -22,6 +22,7 @@
#endif
#include <GrContextOptions.h>
#include <SkSurface.h>
+#include <android-base/unique_fd.h>
#include <utils/StrongPointer.h>
#include <vk/GrVkBackendContext.h>
#include <vk/GrVkExtensions.h>
@@ -82,10 +83,17 @@
void destroySurface(VulkanSurface* surface);
Frame dequeueNextBuffer(VulkanSurface* surface);
+
+ struct VkDrawResult {
+ // The estimated start time for intiating GPU work, -1 if unknown.
+ nsecs_t submissionTime;
+ android::base::unique_fd presentFence;
+ };
+
// Finishes the frame and submits work to the GPU
- // Returns the estimated start time for intiating GPU work, -1 otherwise.
- nsecs_t finishFrame(SkSurface* surface);
- void swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect);
+ VkDrawResult finishFrame(SkSurface* surface);
+ void swapBuffers(VulkanSurface* surface, const SkRect& dirtyRect,
+ android::base::unique_fd&& presentFence);
// Inserts a wait on fence command into the Vulkan command buffer.
status_t fenceWait(int fence, GrDirectContext* grContext);
@@ -201,9 +209,6 @@
GrVkExtensions mExtensions;
uint32_t mDriverVersion = 0;
- VkSemaphore mSwapSemaphore = VK_NULL_HANDLE;
- void* mDestroySemaphoreContext = nullptr;
-
std::once_flag mInitFlag;
std::atomic_bool mInitialized = false;
};