Skip to content

Commit

Permalink
vk: add resource ref-counting alternative (#8220)
Browse files Browse the repository at this point in the history
We add a set of classes to the vk backend that will enable
cleaner ref-counting.

In particular, all allocation from the HandleAllocator will be
wrapped within a resource_ptr<> smart pointer. This struct will
maintain a count that will increment/decrement with respect to
references (similar to std::shared_ptr).

This commit only introduces the new classes/structs and does not
actually make any functional difference. A follow-up commit will
make the switch to use the smart pointer.

We also put VulkanFence and VulkanTimerQuery in a different header
because we will need to depend on them separately in the follow-up
(reason: they are accessed on the backend thread but allocated on
"sync"/filament thread).
  • Loading branch information
poweifeng authored Nov 1, 2024
1 parent 0c2d23e commit 3304a0c
Show file tree
Hide file tree
Showing 13 changed files with 872 additions and 103 deletions.
7 changes: 7 additions & 0 deletions filament/backend/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,18 @@ if (FILAMENT_SUPPORTS_VULKAN)
src/vulkan/caching/VulkanDescriptorSetManager.h
src/vulkan/caching/VulkanPipelineLayoutCache.cpp
src/vulkan/caching/VulkanPipelineLayoutCache.h
src/vulkan/memory/ResourceManager.cpp
src/vulkan/memory/ResourceManager.h
src/vulkan/memory/ResourcePointer.h
src/vulkan/memory/Resource.cpp
src/vulkan/memory/Resource.h
src/vulkan/platform/VulkanPlatform.cpp
src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp
src/vulkan/platform/VulkanPlatformSwapChainImpl.h
src/vulkan/spirv/VulkanSpirvUtils.cpp
src/vulkan/spirv/VulkanSpirvUtils.h
src/vulkan/VulkanAsyncHandles.cpp
src/vulkan/VulkanAsyncHandles.h
src/vulkan/VulkanBlitter.cpp
src/vulkan/VulkanBlitter.h
src/vulkan/VulkanBuffer.cpp
Expand Down
44 changes: 44 additions & 0 deletions filament/backend/src/vulkan/VulkanAsyncHandles.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright (C) 2024 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.
*/

#include "VulkanAsyncHandles.h"

namespace filament::backend {

VulkanTimerQuery::VulkanTimerQuery(std::tuple<uint32_t, uint32_t> indices)
: VulkanThreadSafeResource(VulkanResourceType::TIMER_QUERY),
mStartingQueryIndex(std::get<0>(indices)),
mStoppingQueryIndex(std::get<1>(indices)) {}

void VulkanTimerQuery::setFence(std::shared_ptr<VulkanCmdFence> fence) noexcept {
std::unique_lock<utils::Mutex> lock(mFenceMutex);
mFence = fence;
}

bool VulkanTimerQuery::isCompleted() noexcept {
std::unique_lock<utils::Mutex> lock(mFenceMutex);
// QueryValue is a synchronous call and might occur before beginTimerQuery has written anything
// into the command buffer, which is an error according to the validation layer that ships in
// the Android NDK. Even when AVAILABILITY_BIT is set, validation seems to require that the
// timestamp has at least been written into a processed command buffer.

// This fence indicates that the corresponding buffer has been completed.
return mFence && mFence->getStatus() == VK_SUCCESS;
}

VulkanTimerQuery::~VulkanTimerQuery() = default;

} // namespace filament::backend
109 changes: 109 additions & 0 deletions filament/backend/src/vulkan/VulkanAsyncHandles.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Copyright (C) 2024 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.
*/

#ifndef TNT_FILAMENT_BACKEND_VULKANASYNCHANDLES_H
#define TNT_FILAMENT_BACKEND_VULKANASYNCHANDLES_H

#include <bluevk/BlueVK.h>

#include "DriverBase.h"

#include "VulkanResources.h"

#include <utils/Mutex.h>
#include <utils/Condition.h>

namespace filament::backend {

// Wrapper to enable use of shared_ptr for implementing shared ownership of low-level Vulkan fences.
struct VulkanCmdFence {
struct SetValueScope {
public:
~SetValueScope() {
mHolder->mutex.unlock();
mHolder->condition.notify_all();
}

private:
SetValueScope(VulkanCmdFence* fenceHolder, VkResult result) :
mHolder(fenceHolder) {
mHolder->mutex.lock();
mHolder->status.store(result);
}
VulkanCmdFence* mHolder;
friend struct VulkanCmdFence;
};

VulkanCmdFence(VkFence ifence);
~VulkanCmdFence() = default;

SetValueScope setValue(VkResult value) {
return {this, value};
}

VkFence& getFence() {
return fence;
}

VkResult getStatus() {
std::unique_lock<utils::Mutex> lock(mutex);
return status.load(std::memory_order_acquire);
}

private:
VkFence fence;
utils::Condition condition;
utils::Mutex mutex;
std::atomic<VkResult> status;
};

struct VulkanFence : public HwFence, VulkanResource {
VulkanFence() : VulkanResource(VulkanResourceType::FENCE) {}

explicit VulkanFence(std::shared_ptr<VulkanCmdFence> fence)
: VulkanResource(VulkanResourceType::FENCE),
fence(fence) {}

std::shared_ptr<VulkanCmdFence> fence;
};

struct VulkanTimerQuery : public HwTimerQuery, VulkanThreadSafeResource {
explicit VulkanTimerQuery(std::tuple<uint32_t, uint32_t> indices);
~VulkanTimerQuery();

void setFence(std::shared_ptr<VulkanCmdFence> fence) noexcept;

bool isCompleted() noexcept;

uint32_t getStartingQueryIndex() const {
return mStartingQueryIndex;
}

uint32_t getStoppingQueryIndex() const {
return mStoppingQueryIndex;
}

private:
uint32_t mStartingQueryIndex;
uint32_t mStoppingQueryIndex;

std::shared_ptr<VulkanCmdFence> mFence;
utils::Mutex mFenceMutex;
};

} // namespace filament::backend

#endif // TNT_FILAMENT_BACKEND_VULKANHASYNCANDLES_H
43 changes: 1 addition & 42 deletions filament/backend/src/vulkan/VulkanCommands.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

#include "DriverBase.h"

#include "VulkanAsyncHandles.h"
#include "VulkanConstants.h"
#include "VulkanResources.h"

Expand Down Expand Up @@ -59,48 +60,6 @@ class VulkanGroupMarkers {

#endif // FVK_DEBUG_GROUP_MARKERS

// Wrapper to enable use of shared_ptr for implementing shared ownership of low-level Vulkan fences.
struct VulkanCmdFence {
struct SetValueScope {
public:
~SetValueScope() {
mHolder->mutex.unlock();
mHolder->condition.notify_all();
}

private:
SetValueScope(VulkanCmdFence* fenceHolder, VkResult result) :
mHolder(fenceHolder) {
mHolder->mutex.lock();
mHolder->status.store(result);
}
VulkanCmdFence* mHolder;
friend struct VulkanCmdFence;
};

VulkanCmdFence(VkFence ifence);
~VulkanCmdFence() = default;

SetValueScope setValue(VkResult value) {
return {this, value};
}

VkFence& getFence() {
return fence;
}

VkResult getStatus() {
std::unique_lock<utils::Mutex> lock(mutex);
return status.load(std::memory_order_acquire);
}

private:
VkFence fence;
utils::Condition condition;
utils::Mutex mutex;
std::atomic<VkResult> status;
};

// The submission fence has shared ownership semantics because it is potentially wrapped by a
// DriverApi fence object and should not be destroyed until both the DriverApi object is freed and
// we're done waiting on the most recent submission of the given command buffer.
Expand Down
4 changes: 2 additions & 2 deletions filament/backend/src/vulkan/VulkanContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ VulkanTimestamps::VulkanTimestamps(VkDevice device) : mDevice(device) {
VkQueryPoolCreateInfo tqpCreateInfo = {
.sType = VK_STRUCTURE_TYPE_QUERY_POOL_CREATE_INFO,
.queryType = VK_QUERY_TYPE_TIMESTAMP,
};
};
std::unique_lock<utils::Mutex> lock(mMutex);
tqpCreateInfo.queryCount = mUsed.size() * 2;
VkResult result = vkCreateQueryPool(mDevice, &tqpCreateInfo, VKALLOC, &mPool);
Expand All @@ -101,7 +101,7 @@ std::tuple<uint32_t, uint32_t> VulkanTimestamps::getNextQuery() {
for (size_t timerIndex = 0; timerIndex < maxTimers; ++timerIndex) {
if (!mUsed.test(timerIndex)) {
mUsed.set(timerIndex);
return std::make_tuple(timerIndex * 2, timerIndex * 2 + 1);
return std::make_tuple(timerIndex * 2, timerIndex * 2 + 1);
}
}
FVK_LOGE << "More than " << maxTimers << " timers are not supported." << utils::io::endl;
Expand Down
1 change: 1 addition & 0 deletions filament/backend/src/vulkan/VulkanDriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "CommandStreamDispatcher.h"
#include "DataReshaper.h"
#include "SystraceProfile.h"
#include "VulkanAsyncHandles.h"
#include "VulkanBuffer.h"
#include "VulkanCommands.h"
#include "VulkanDriverFactory.h"
Expand Down
23 changes: 0 additions & 23 deletions filament/backend/src/vulkan/VulkanHandles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,29 +564,6 @@ VulkanBufferObject::VulkanBufferObject(VmaAllocator allocator, VulkanStagePool&
buffer(allocator, stagePool, getBufferObjectUsage(bindingType), byteCount),
bindingType(bindingType) {}

VulkanTimerQuery::VulkanTimerQuery(std::tuple<uint32_t, uint32_t> indices)
: VulkanThreadSafeResource(VulkanResourceType::TIMER_QUERY),
mStartingQueryIndex(std::get<0>(indices)),
mStoppingQueryIndex(std::get<1>(indices)) {}

void VulkanTimerQuery::setFence(std::shared_ptr<VulkanCmdFence> fence) noexcept {
std::unique_lock<utils::Mutex> lock(mFenceMutex);
mFence = fence;
}

bool VulkanTimerQuery::isCompleted() noexcept {
std::unique_lock<utils::Mutex> lock(mFenceMutex);
// QueryValue is a synchronous call and might occur before beginTimerQuery has written anything
// into the command buffer, which is an error according to the validation layer that ships in
// the Android NDK. Even when AVAILABILITY_BIT is set, validation seems to require that the
// timestamp has at least been written into a processed command buffer.

// This fence indicates that the corresponding buffer has been completed.
return mFence && mFence->getStatus() == VK_SUCCESS;
}

VulkanTimerQuery::~VulkanTimerQuery() = default;

VulkanRenderPrimitive::VulkanRenderPrimitive(VulkanResourceAllocator* resourceAllocator,
PrimitiveType pt, Handle<HwVertexBuffer> vbh, Handle<HwIndexBuffer> ibh)
: VulkanResource(VulkanResourceType::RENDER_PRIMITIVE),
Expand Down
36 changes: 0 additions & 36 deletions filament/backend/src/vulkan/VulkanHandles.h
Original file line number Diff line number Diff line change
Expand Up @@ -450,42 +450,6 @@ struct VulkanRenderPrimitive : public HwRenderPrimitive, VulkanResource {
FixedSizeVulkanResourceManager<2> mResources;
};

struct VulkanFence : public HwFence, VulkanResource {
VulkanFence()
: VulkanResource(VulkanResourceType::FENCE) {}

explicit VulkanFence(std::shared_ptr<VulkanCmdFence> fence)
: VulkanResource(VulkanResourceType::FENCE),
fence(fence) {}

std::shared_ptr<VulkanCmdFence> fence;
};

struct VulkanTimerQuery : public HwTimerQuery, VulkanThreadSafeResource {
explicit VulkanTimerQuery(std::tuple<uint32_t, uint32_t> indices);
~VulkanTimerQuery();

void setFence(std::shared_ptr<VulkanCmdFence> fence) noexcept;

bool isCompleted() noexcept;

uint32_t getStartingQueryIndex() const {
return mStartingQueryIndex;
}

uint32_t getStoppingQueryIndex() const {
return mStoppingQueryIndex;
}

private:
uint32_t mStartingQueryIndex;
uint32_t mStoppingQueryIndex;

std::shared_ptr<VulkanCmdFence> mFence;
utils::Mutex mFenceMutex;
};


inline constexpr VkBufferUsageFlagBits getBufferObjectUsage(
BufferObjectBinding bindingType) noexcept {
switch(bindingType) {
Expand Down
Loading

0 comments on commit 3304a0c

Please sign in to comment.