Track TextureViews in Textures so that they can be chain-destroyed.

- Adds Prepend function to LinkedList to avoid directly using the
  insert functions on the LinkNodes. (And tests for this as well.)
- Adds ApiObjectList class for tracking lists of objects for
  destruction.
- Renames and virtualizes some tracking interfaces so that they can be
  overriden for the TextureView/Texture cases.
- Removes explicit destroying of TextureViews from Device since
  destroying Textures will destroy TextureViews now.

Fixed: dawn:1355
Change-Id: I3522383ea7724d6e41ac0c805793a6c34d9bec27
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/101762
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Loko Kung 2022-09-15 21:06:51 +00:00 committed by Dawn LUCI CQ
parent 9c07d3c52c
commit 18dfc4797c
25 changed files with 158 additions and 68 deletions

View File

@ -185,6 +185,9 @@ class LinkedList {
// Appends |e| to the end of the linked list.
void Append(LinkNode<T>* e) { e->InsertBefore(&root_); }
// Prepends |e| to the front og the linked list.
void Prepend(LinkNode<T>* e) { e->InsertAfter(&root_); }
// Moves all elements (in order) of the list and appends them into |l| leaving the list empty.
void MoveInto(LinkedList<T>* l) {
if (empty()) {

View File

@ -450,11 +450,11 @@ BindGroupBase::BindGroupBase(DeviceBase* device,
}
}
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
BindGroupBase::BindGroupBase(DeviceBase* device) : ApiObjectBase(device, kLabelNotImplemented) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
BindGroupBase::~BindGroupBase() = default;

View File

@ -481,7 +481,7 @@ BindGroupLayoutBase::BindGroupLayoutBase(DeviceBase* device,
const BindGroupLayoutDescriptor* descriptor,
PipelineCompatibilityToken pipelineCompatibilityToken)
: BindGroupLayoutBase(device, descriptor, pipelineCompatibilityToken, kUntrackedByDevice) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
BindGroupLayoutBase::BindGroupLayoutBase(DeviceBase* device, ObjectBase::ErrorTag tag)
@ -489,7 +489,7 @@ BindGroupLayoutBase::BindGroupLayoutBase(DeviceBase* device, ObjectBase::ErrorTa
BindGroupLayoutBase::BindGroupLayoutBase(DeviceBase* device)
: ApiObjectBase(device, kLabelNotImplemented) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
BindGroupLayoutBase::~BindGroupLayoutBase() = default;

View File

@ -68,7 +68,7 @@ class ErrorBuffer final : public BufferBase {
}
// Since error buffers in this case may allocate memory, we need to track them
// for destruction on the device.
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
}
@ -152,7 +152,7 @@ BufferBase::BufferBase(DeviceBase* device, const BufferDescriptor* descriptor)
mUsage |= kInternalStorageBuffer;
}
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
BufferBase::BufferBase(DeviceBase* device,
@ -171,7 +171,7 @@ BufferBase::BufferBase(DeviceBase* device,
BufferBase::BufferBase(DeviceBase* device, BufferState state)
: ApiObjectBase(device, kLabelNotImplemented), mState(state) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
BufferBase::~BufferBase() {

View File

@ -30,12 +30,12 @@ CommandBufferBase::CommandBufferBase(CommandEncoder* encoder,
: ApiObjectBase(encoder->GetDevice(), descriptor->label),
mCommands(encoder->AcquireCommands()),
mResourceUsages(encoder->AcquireResourceUsages()) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
CommandBufferBase::CommandBufferBase(DeviceBase* device)
: ApiObjectBase(device, kLabelNotImplemented) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
CommandBufferBase::CommandBufferBase(DeviceBase* device, ObjectBase::ErrorTag tag)

View File

@ -718,7 +718,7 @@ CommandEncoder* CommandEncoder::MakeError(DeviceBase* device) {
CommandEncoder::CommandEncoder(DeviceBase* device, const CommandEncoderDescriptor* descriptor)
: ApiObjectBase(device, descriptor->label), mEncodingContext(device, this) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
const DawnEncoderInternalUsageDescriptor* internalUsageDesc = nullptr;
FindInChain(descriptor->nextInChain, &internalUsageDesc);

View File

@ -114,7 +114,7 @@ ComputePassEncoder::ComputePassEncoder(DeviceBase* device,
EncodingContext* encodingContext)
: ProgrammableEncoder(device, descriptor->label, encodingContext),
mCommandEncoder(commandEncoder) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
// static

View File

@ -47,14 +47,14 @@ ComputePipelineBase::ComputePipelineBase(DeviceBase* device,
{{SingleShaderStage::Compute, descriptor->compute.module, descriptor->compute.entryPoint,
descriptor->compute.constantCount, descriptor->compute.constants}}) {
SetContentHash(ComputeContentHash());
TrackInDevice();
GetObjectTrackingList()->Track(this);
// Initialize the cache key to include the cache type and device information.
StreamIn(&mCacheKey, CacheKey::Type::ComputePipeline, device->GetCacheKey());
}
ComputePipelineBase::ComputePipelineBase(DeviceBase* device) : PipelineBase(device) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
ComputePipelineBase::ComputePipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag)

View File

@ -338,7 +338,7 @@ void DeviceBase::DestroyObjects() {
// can destroy the frontend cache.
// clang-format off
static constexpr std::array<ObjectType, 19> kObjectTypeDependencyOrder = {
static constexpr std::array<ObjectType, 18> kObjectTypeDependencyOrder = {
ObjectType::ComputePassEncoder,
ObjectType::RenderPassEncoder,
ObjectType::RenderBundleEncoder,
@ -353,26 +353,15 @@ void DeviceBase::DestroyObjects() {
ObjectType::BindGroupLayout,
ObjectType::ShaderModule,
ObjectType::ExternalTexture,
ObjectType::TextureView,
ObjectType::Texture,
ObjectType::Texture, // Note that Textures own the TextureViews.
ObjectType::QuerySet,
ObjectType::Sampler,
ObjectType::Buffer,
};
// clang-format on
// We first move all objects out from the tracking list into a separate list so that we can
// avoid locking the same mutex twice. We can then iterate across the separate list to call
// the actual destroy function.
LinkedList<ApiObjectBase> objects;
for (ObjectType type : kObjectTypeDependencyOrder) {
ApiObjectList& objList = mObjectLists[type];
const std::lock_guard<std::mutex> lock(objList.mutex);
objList.objects.MoveInto(&objects);
}
while (!objects.empty()) {
// The destroy call should also remove the object from the list.
objects.head()->value()->Destroy();
mObjectLists[type].Destroy();
}
}
@ -685,14 +674,8 @@ bool DeviceBase::IsLost() const {
return mState != State::Alive;
}
void DeviceBase::TrackObject(ApiObjectBase* object) {
ApiObjectList& objectList = mObjectLists[object->GetType()];
std::lock_guard<std::mutex> lock(objectList.mutex);
object->InsertBefore(objectList.objects.head());
}
std::mutex* DeviceBase::GetObjectListMutex(ObjectType type) {
return &mObjectLists[type].mutex;
ApiObjectList* DeviceBase::GetObjectTrackingList(ObjectType type) {
return &mObjectLists[type];
}
AdapterBase* DeviceBase::GetAdapter() const {

View File

@ -333,8 +333,7 @@ class DeviceBase : public RefCountedWithExternalCount {
};
State GetState() const;
bool IsLost() const;
void TrackObject(ApiObjectBase* object);
std::mutex* GetObjectListMutex(ObjectType type);
ApiObjectList* GetObjectTrackingList(ObjectType type);
std::vector<const char*> GetTogglesUsed() const;
WGSLExtensionSet GetWGSLExtensionAllowList() const;
@ -548,12 +547,6 @@ class DeviceBase : public RefCountedWithExternalCount {
State mState = State::BeingCreated;
// Encompasses the mutex and the actual list that contains all live objects "owned" by the
// device.
struct ApiObjectList {
std::mutex mutex;
LinkedList<ApiObjectBase> objects;
};
PerObjectType<ApiObjectList> mObjectLists;
FormatTable mFormatTable;

View File

@ -115,12 +115,12 @@ ResultOrError<Ref<ExternalTextureBase>> ExternalTextureBase::Create(
ExternalTextureBase::ExternalTextureBase(DeviceBase* device,
const ExternalTextureDescriptor* descriptor)
: ApiObjectBase(device, descriptor->label), mState(ExternalTextureState::Alive) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
ExternalTextureBase::ExternalTextureBase(DeviceBase* device)
: ApiObjectBase(device, kLabelNotImplemented), mState(ExternalTextureState::Alive) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
// Error external texture cannot be used in bind group.

View File

@ -37,6 +37,31 @@ DeviceBase* ObjectBase::GetDevice() const {
return mDevice.Get();
}
void ApiObjectList::Track(ApiObjectBase* object) {
if (mMarkedDestroyed) {
object->DestroyImpl();
return;
}
std::lock_guard<std::mutex> lock(mMutex);
mObjects.Prepend(object);
}
bool ApiObjectList::Untrack(ApiObjectBase* object) {
std::lock_guard<std::mutex> lock(mMutex);
return object->RemoveFromList();
}
void ApiObjectList::Destroy() {
std::lock_guard<std::mutex> lock(mMutex);
mMarkedDestroyed = true;
while (!mObjects.empty()) {
auto* head = mObjects.head();
bool removed = head->RemoveFromList();
ASSERT(removed);
head->value()->DestroyImpl();
}
}
ApiObjectBase::ApiObjectBase(DeviceBase* device, const char* label) : ObjectBase(device) {
if (label) {
mLabel = label;
@ -71,14 +96,18 @@ void ApiObjectBase::DeleteThis() {
RefCounted::DeleteThis();
}
void ApiObjectBase::TrackInDevice() {
ApiObjectList* ApiObjectBase::GetObjectTrackingList() {
ASSERT(GetDevice() != nullptr);
GetDevice()->TrackObject(this);
return GetDevice()->GetObjectTrackingList(GetType());
}
void ApiObjectBase::Destroy() {
const std::lock_guard<std::mutex> lock(*GetDevice()->GetObjectListMutex(GetType()));
if (RemoveFromList()) {
if (!IsAlive()) {
return;
}
ApiObjectList* list = GetObjectTrackingList();
ASSERT(list != nullptr);
if (list->Untrack(this)) {
DestroyImpl();
}
}

View File

@ -15,6 +15,7 @@
#ifndef SRC_DAWN_NATIVE_OBJECTBASE_H_
#define SRC_DAWN_NATIVE_OBJECTBASE_H_
#include <mutex>
#include <string>
#include "dawn/common/LinkedList.h"
@ -23,6 +24,7 @@
namespace dawn::native {
class ApiObjectBase;
class DeviceBase;
class ErrorMonad : public RefCounted {
@ -48,6 +50,26 @@ class ObjectBase : public ErrorMonad {
Ref<DeviceBase> mDevice;
};
// Generic object list with a mutex for tracking for destruction.
class ApiObjectList {
public:
// Tracks an object if the list is not destroyed. If the list is destroyed, destroys the object.
void Track(ApiObjectBase* object);
// Returns true iff the object was removed from the list.
bool Untrack(ApiObjectBase* object);
// Destroys and removes all the objects tracked in the list.
void Destroy();
private:
// Boolean used to mark the list so that on subsequent calls to Untrack, we don't need to
// reaquire the lock, and Track on new objects immediately destroys them.
bool mMarkedDestroyed = false;
std::mutex mMutex;
LinkedList<ApiObjectBase> mObjects;
};
class ApiObjectBase : public ObjectBase, public LinkNode<ApiObjectBase> {
public:
struct LabelNotImplementedTag {};
@ -85,7 +107,11 @@ class ApiObjectBase : public ObjectBase, public LinkNode<ApiObjectBase> {
// and they should ensure that their overriding versions call this underlying version
// somewhere.
void DeleteThis() override;
void TrackInDevice();
// Returns the list where this object may be tracked for future destruction. This can be
// overrided to create hierarchical object tracking ownership:
// i.e. Device -[tracks]-> Texture -[tracks]-> TextureView.
virtual ApiObjectList* GetObjectTrackingList();
// Sub-classes may override this function multiple times. Whenever overriding this function,
// however, users should be sure to call their parent's version in the new override to make
@ -94,6 +120,8 @@ class ApiObjectBase : public ObjectBase, public LinkNode<ApiObjectBase> {
virtual void DestroyImpl() = 0;
private:
friend class ApiObjectList;
virtual void SetLabelImpl();
std::string mLabel;

View File

@ -71,12 +71,12 @@ PipelineLayoutBase::PipelineLayoutBase(DeviceBase* device,
PipelineLayoutBase::PipelineLayoutBase(DeviceBase* device,
const PipelineLayoutDescriptor* descriptor)
: PipelineLayoutBase(device, descriptor, kUntrackedByDevice) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
PipelineLayoutBase::PipelineLayoutBase(DeviceBase* device)
: ApiObjectBase(device, kLabelNotImplemented) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
PipelineLayoutBase::PipelineLayoutBase(DeviceBase* device, ObjectBase::ErrorTag tag)

View File

@ -107,11 +107,11 @@ QuerySetBase::QuerySetBase(DeviceBase* device, const QuerySetDescriptor* descrip
}
mQueryAvailability.resize(descriptor->count);
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
QuerySetBase::QuerySetBase(DeviceBase* device) : ApiObjectBase(device, kLabelNotImplemented) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
QuerySetBase::QuerySetBase(DeviceBase* device,

View File

@ -39,7 +39,7 @@ RenderBundleBase::RenderBundleBase(RenderBundleEncoder* encoder,
mStencilReadOnly(stencilReadOnly),
mDrawCount(encoder->GetDrawCount()),
mResourceUsage(std::move(resourceUsage)) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
void RenderBundleBase::DestroyImpl() {

View File

@ -100,7 +100,7 @@ RenderBundleEncoder::RenderBundleEncoder(DeviceBase* device,
descriptor->depthReadOnly,
descriptor->stencilReadOnly),
mBundleEncodingContext(device, this) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
RenderBundleEncoder::RenderBundleEncoder(DeviceBase* device, ErrorTag errorTag)

View File

@ -76,7 +76,7 @@ RenderPassEncoder::RenderPassEncoder(DeviceBase* device,
if (maxDrawCountInfo) {
mMaxDrawCount = maxDrawCountInfo->maxDrawCount;
}
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
// static

View File

@ -616,14 +616,14 @@ RenderPipelineBase::RenderPipelineBase(DeviceBase* device,
}
SetContentHash(ComputeContentHash());
TrackInDevice();
GetObjectTrackingList()->Track(this);
// Initialize the cache key to include the cache type and device information.
StreamIn(&mCacheKey, CacheKey::Type::RenderPipeline, device->GetCacheKey());
}
RenderPipelineBase::RenderPipelineBase(DeviceBase* device) : PipelineBase(device) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
RenderPipelineBase::RenderPipelineBase(DeviceBase* device, ObjectBase::ErrorTag tag)

View File

@ -86,11 +86,11 @@ SamplerBase::SamplerBase(DeviceBase* device,
SamplerBase::SamplerBase(DeviceBase* device, const SamplerDescriptor* descriptor)
: SamplerBase(device, descriptor, kUntrackedByDevice) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
SamplerBase::SamplerBase(DeviceBase* device) : ApiObjectBase(device, kLabelNotImplemented) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
SamplerBase::SamplerBase(DeviceBase* device, ObjectBase::ErrorTag tag)

View File

@ -1089,12 +1089,12 @@ ShaderModuleBase::ShaderModuleBase(DeviceBase* device,
ShaderModuleBase::ShaderModuleBase(DeviceBase* device, const ShaderModuleDescriptor* descriptor)
: ShaderModuleBase(device, descriptor, kUntrackedByDevice) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
ShaderModuleBase::ShaderModuleBase(DeviceBase* device)
: ApiObjectBase(device, kLabelNotImplemented) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
ShaderModuleBase::ShaderModuleBase(DeviceBase* device, ObjectBase::ErrorTag tag)

View File

@ -117,7 +117,7 @@ TextureDescriptor GetSwapChainBaseTextureDescriptor(NewSwapChainBase* swapChain)
// SwapChainBase
SwapChainBase::SwapChainBase(DeviceBase* device) : ApiObjectBase(device, kLabelNotImplemented) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
SwapChainBase::SwapChainBase(DeviceBase* device, ObjectBase::ErrorTag tag)

View File

@ -552,7 +552,7 @@ TextureBase::TextureBase(DeviceBase* device,
if (internalUsageDesc != nullptr) {
mInternalUsage |= internalUsageDesc->internalUsage;
}
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
TextureBase::~TextureBase() = default;
@ -561,7 +561,7 @@ static constexpr Format kUnusedFormat;
TextureBase::TextureBase(DeviceBase* device, TextureState state)
: ApiObjectBase(device, kLabelNotImplemented), mFormat(kUnusedFormat), mState(state) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
TextureBase::TextureBase(DeviceBase* device,
@ -578,6 +578,9 @@ TextureBase::TextureBase(DeviceBase* device,
void TextureBase::DestroyImpl() {
mState = TextureState::Destroyed;
// Destroy all of the views associated with the texture as well.
mTextureViews.Destroy();
}
// static
@ -766,6 +769,10 @@ ResultOrError<Ref<TextureViewBase>> TextureBase::CreateView(
return GetDevice()->CreateTextureView(this, descriptor);
}
ApiObjectList* TextureBase::GetViewTrackingList() {
return &mTextureViews;
}
TextureViewBase* TextureBase::APICreateView(const TextureViewDescriptor* descriptor) {
DeviceBase* device = GetDevice();
@ -831,14 +838,14 @@ TextureViewBase::TextureViewBase(TextureBase* texture, const TextureViewDescript
mRange({ConvertViewAspect(mFormat, descriptor->aspect),
{descriptor->baseArrayLayer, descriptor->arrayLayerCount},
{descriptor->baseMipLevel, descriptor->mipLevelCount}}) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
TextureViewBase::TextureViewBase(TextureBase* texture)
: ApiObjectBase(texture->GetDevice(), kLabelNotImplemented),
mTexture(texture),
mFormat(kUnusedFormat) {
TrackInDevice();
GetObjectTrackingList()->Track(this);
}
TextureViewBase::TextureViewBase(DeviceBase* device, ObjectBase::ErrorTag tag)
@ -907,4 +914,9 @@ const SubresourceRange& TextureViewBase::GetSubresourceRange() const {
return mRange;
}
ApiObjectList* TextureViewBase::GetObjectTrackingList() {
ASSERT(!IsError());
return mTexture->GetViewTrackingList();
}
} // namespace dawn::native

View File

@ -92,6 +92,7 @@ class TextureBase : public ApiObjectBase {
ResultOrError<Ref<TextureViewBase>> CreateView(
const TextureViewDescriptor* descriptor = nullptr);
ApiObjectList* GetViewTrackingList();
// Dawn API
TextureViewBase* APICreateView(const TextureViewDescriptor* descriptor = nullptr);
@ -129,6 +130,10 @@ class TextureBase : public ApiObjectBase {
TextureState mState;
wgpu::TextureFormat mFormatEnumForReflection;
// Textures track texture views created from them so that they can be destroyed when the texture
// is destroyed.
ApiObjectList mTextureViews;
// TODO(crbug.com/dawn/845): Use a more optimized data structure to save space
std::vector<bool> mIsSubresourceContentInitializedAtIndex;
};
@ -162,6 +167,8 @@ class TextureViewBase : public ApiObjectBase {
private:
TextureViewBase(DeviceBase* device, ObjectBase::ErrorTag tag);
ApiObjectList* GetObjectTrackingList() override;
Ref<TextureBase> mTexture;
const Format& mFormat;

View File

@ -117,6 +117,41 @@ TEST(LinkedList, Append) {
}
}
TEST(LinkedList, Prepend) {
LinkedList<Node> list;
ExpectListContents(list, 0, nullptr);
Node n1(1);
list.Prepend(&n1);
EXPECT_EQ(&n1, list.head());
EXPECT_EQ(&n1, list.tail());
{
const int expected[] = {1};
ExpectListContents(list, 1, expected);
}
Node n2(2);
list.Prepend(&n2);
EXPECT_EQ(&n2, list.head());
EXPECT_EQ(&n1, list.tail());
{
const int expected[] = {2, 1};
ExpectListContents(list, 2, expected);
}
Node n3(3);
list.Prepend(&n3);
EXPECT_EQ(&n3, list.head());
EXPECT_EQ(&n1, list.tail());
{
const int expected[] = {3, 2, 1};
ExpectListContents(list, 3, expected);
}
}
TEST(LinkedList, RemoveFromList) {
LinkedList<Node> list;