Reverts previous SlabAllocator thread-safety changes for MutexProtected.
Bug: dawn:1662
Change-Id: I0485fda46e759cdf9ec5294307b92b2535198ee3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/145060
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/dawn/common/SlabAllocator.cpp b/src/dawn/common/SlabAllocator.cpp
index 507eaee..7aed329 100644
--- a/src/dawn/common/SlabAllocator.cpp
+++ b/src/dawn/common/SlabAllocator.cpp
@@ -170,8 +170,6 @@
}
void* SlabAllocatorImpl::Allocate() {
- std::lock_guard<std::mutex> lock(mMutex);
-
if (mAvailableSlabs.next == nullptr) {
GetNewSlab();
}
@@ -197,7 +195,6 @@
Slab* slab = reinterpret_cast<Slab*>(static_cast<char*>(firstAllocation) - mSlabBlocksOffset);
ASSERT(slab != nullptr);
- std::lock_guard<std::mutex> lock(mMutex);
bool slabWasFull = slab->blocksInUse == mBlocksPerSlab;
ASSERT(slab->blocksInUse != 0);
PushFront(slab, node);
diff --git a/src/dawn/common/SlabAllocator.h b/src/dawn/common/SlabAllocator.h
index 69710fc..7542b42 100644
--- a/src/dawn/common/SlabAllocator.h
+++ b/src/dawn/common/SlabAllocator.h
@@ -16,7 +16,6 @@
#define SRC_DAWN_COMMON_SLABALLOCATOR_H_
#include <cstdint>
-#include <mutex>
#include <type_traits>
#include <utility>
@@ -159,7 +158,6 @@
void Prepend(Slab* slab);
};
- std::mutex mMutex;
SentinelSlab mAvailableSlabs; // Available slabs to service allocations.
SentinelSlab mFullSlabs; // Full slabs. Stored here so we can skip checking them.
SentinelSlab mRecycledSlabs; // Recycled slabs. Not immediately added to |mAvailableSlabs| so
diff --git a/src/dawn/native/d3d11/BindGroupLayoutD3D11.cpp b/src/dawn/native/d3d11/BindGroupLayoutD3D11.cpp
index 0cfe3c9..a06df45 100644
--- a/src/dawn/native/d3d11/BindGroupLayoutD3D11.cpp
+++ b/src/dawn/native/d3d11/BindGroupLayoutD3D11.cpp
@@ -30,11 +30,11 @@
Ref<BindGroup> BindGroupLayout::AllocateBindGroup(Device* device,
const BindGroupDescriptor* descriptor) {
- return AcquireRef(mBindGroupAllocator.Allocate(device, descriptor));
+ return AcquireRef(mBindGroupAllocator->Allocate(device, descriptor));
}
void BindGroupLayout::DeallocateBindGroup(BindGroup* bindGroup) {
- mBindGroupAllocator.Deallocate(bindGroup);
+ mBindGroupAllocator->Deallocate(bindGroup);
}
} // namespace dawn::native::d3d11
diff --git a/src/dawn/native/d3d11/BindGroupLayoutD3D11.h b/src/dawn/native/d3d11/BindGroupLayoutD3D11.h
index 59d45d2..1205262 100644
--- a/src/dawn/native/d3d11/BindGroupLayoutD3D11.h
+++ b/src/dawn/native/d3d11/BindGroupLayoutD3D11.h
@@ -15,6 +15,7 @@
#ifndef SRC_DAWN_NATIVE_D3D11_BINDGROUPLAYOUTD3D11_H_
#define SRC_DAWN_NATIVE_D3D11_BINDGROUPLAYOUTD3D11_H_
+#include "dawn/common/MutexProtected.h"
#include "dawn/common/SlabAllocator.h"
#include "dawn/native/BindGroupLayoutInternal.h"
#include "dawn/native/d3d11/BindGroupD3D11.h"
@@ -35,7 +36,7 @@
BindGroupLayout(Device* device, const BindGroupLayoutDescriptor* descriptor);
~BindGroupLayout() override = default;
- SlabAllocator<BindGroup> mBindGroupAllocator;
+ MutexProtected<SlabAllocator<BindGroup>> mBindGroupAllocator;
};
} // namespace dawn::native::d3d11
diff --git a/src/dawn/native/d3d12/BindGroupLayoutD3D12.cpp b/src/dawn/native/d3d12/BindGroupLayoutD3D12.cpp
index 62c66b3..3c24d7b 100644
--- a/src/dawn/native/d3d12/BindGroupLayoutD3D12.cpp
+++ b/src/dawn/native/d3d12/BindGroupLayoutD3D12.cpp
@@ -164,7 +164,7 @@
}
Ref<BindGroup> bindGroup = AcquireRef<BindGroup>(
- mBindGroupAllocator.Allocate(device, descriptor, viewSizeIncrement, viewAllocation));
+ mBindGroupAllocator->Allocate(device, descriptor, viewSizeIncrement, viewAllocation));
if (GetSamplerDescriptorCount() > 0) {
Ref<SamplerHeapCacheEntry> samplerHeapCacheEntry;
@@ -182,7 +182,7 @@
mViewAllocator->Deallocate(viewAllocation);
}
- mBindGroupAllocator.Deallocate(bindGroup);
+ mBindGroupAllocator->Deallocate(bindGroup);
}
ityp::span<BindingIndex, const uint32_t> BindGroupLayout::GetDescriptorHeapOffsets() const {
diff --git a/src/dawn/native/d3d12/BindGroupLayoutD3D12.h b/src/dawn/native/d3d12/BindGroupLayoutD3D12.h
index d5df9ff..7357ddc 100644
--- a/src/dawn/native/d3d12/BindGroupLayoutD3D12.h
+++ b/src/dawn/native/d3d12/BindGroupLayoutD3D12.h
@@ -17,6 +17,7 @@
#include <vector>
+#include "dawn/common/MutexProtected.h"
#include "dawn/common/SlabAllocator.h"
#include "dawn/common/ityp_stack_vec.h"
#include "dawn/native/BindGroupLayoutInternal.h"
@@ -80,7 +81,7 @@
std::vector<D3D12_DESCRIPTOR_RANGE1> mCbvUavSrvDescriptorRanges;
std::vector<D3D12_DESCRIPTOR_RANGE1> mSamplerDescriptorRanges;
- SlabAllocator<BindGroup> mBindGroupAllocator;
+ MutexProtected<SlabAllocator<BindGroup>> mBindGroupAllocator;
StagingDescriptorAllocator* mSamplerAllocator = nullptr;
StagingDescriptorAllocator* mViewAllocator = nullptr;
diff --git a/src/dawn/native/metal/BindGroupLayoutMTL.h b/src/dawn/native/metal/BindGroupLayoutMTL.h
index 8c859a7..f16256c 100644
--- a/src/dawn/native/metal/BindGroupLayoutMTL.h
+++ b/src/dawn/native/metal/BindGroupLayoutMTL.h
@@ -15,6 +15,7 @@
#ifndef SRC_DAWN_NATIVE_METAL_BINDGROUPLAYOUTMTL_H_
#define SRC_DAWN_NATIVE_METAL_BINDGROUPLAYOUTMTL_H_
+#include "dawn/common/MutexProtected.h"
#include "dawn/common/SlabAllocator.h"
#include "dawn/native/BindGroupLayoutInternal.h"
@@ -35,7 +36,7 @@
BindGroupLayout(DeviceBase* device, const BindGroupLayoutDescriptor* descriptor);
~BindGroupLayout() override;
- SlabAllocator<BindGroup> mBindGroupAllocator;
+ MutexProtected<SlabAllocator<BindGroup>> mBindGroupAllocator;
};
} // namespace dawn::native::metal
diff --git a/src/dawn/native/metal/BindGroupLayoutMTL.mm b/src/dawn/native/metal/BindGroupLayoutMTL.mm
index a4f5de9..1c2fe64 100644
--- a/src/dawn/native/metal/BindGroupLayoutMTL.mm
+++ b/src/dawn/native/metal/BindGroupLayoutMTL.mm
@@ -32,11 +32,11 @@
Ref<BindGroup> BindGroupLayout::AllocateBindGroup(Device* device,
const BindGroupDescriptor* descriptor) {
- return AcquireRef(mBindGroupAllocator.Allocate(device, descriptor));
+ return AcquireRef(mBindGroupAllocator->Allocate(device, descriptor));
}
void BindGroupLayout::DeallocateBindGroup(BindGroup* bindGroup) {
- mBindGroupAllocator.Deallocate(bindGroup);
+ mBindGroupAllocator->Deallocate(bindGroup);
}
} // namespace dawn::native::metal
diff --git a/src/dawn/native/opengl/BindGroupLayoutGL.cpp b/src/dawn/native/opengl/BindGroupLayoutGL.cpp
index c0eb56d..cc77075 100644
--- a/src/dawn/native/opengl/BindGroupLayoutGL.cpp
+++ b/src/dawn/native/opengl/BindGroupLayoutGL.cpp
@@ -22,11 +22,11 @@
Ref<BindGroup> BindGroupLayout::AllocateBindGroup(Device* device,
const BindGroupDescriptor* descriptor) {
- return AcquireRef(mBindGroupAllocator.Allocate(device, descriptor));
+ return AcquireRef(mBindGroupAllocator->Allocate(device, descriptor));
}
void BindGroupLayout::DeallocateBindGroup(BindGroup* bindGroup) {
- mBindGroupAllocator.Deallocate(bindGroup);
+ mBindGroupAllocator->Deallocate(bindGroup);
}
} // namespace dawn::native::opengl
diff --git a/src/dawn/native/opengl/BindGroupLayoutGL.h b/src/dawn/native/opengl/BindGroupLayoutGL.h
index b77229a..996fb08 100644
--- a/src/dawn/native/opengl/BindGroupLayoutGL.h
+++ b/src/dawn/native/opengl/BindGroupLayoutGL.h
@@ -15,6 +15,7 @@
#ifndef SRC_DAWN_NATIVE_OPENGL_BINDGROUPLAYOUTGL_H_
#define SRC_DAWN_NATIVE_OPENGL_BINDGROUPLAYOUTGL_H_
+#include "dawn/common/MutexProtected.h"
#include "dawn/common/SlabAllocator.h"
#include "dawn/native/BindGroupLayoutInternal.h"
#include "dawn/native/opengl/BindGroupGL.h"
@@ -32,7 +33,7 @@
private:
~BindGroupLayout() override = default;
- SlabAllocator<BindGroup> mBindGroupAllocator;
+ MutexProtected<SlabAllocator<BindGroup>> mBindGroupAllocator;
};
} // namespace dawn::native::opengl
diff --git a/src/dawn/native/vulkan/BindGroupLayoutVk.cpp b/src/dawn/native/vulkan/BindGroupLayoutVk.cpp
index 03fdf56..4c45e68 100644
--- a/src/dawn/native/vulkan/BindGroupLayoutVk.cpp
+++ b/src/dawn/native/vulkan/BindGroupLayoutVk.cpp
@@ -176,13 +176,13 @@
DescriptorSetAllocation descriptorSetAllocation;
DAWN_TRY_ASSIGN(descriptorSetAllocation, mDescriptorSetAllocator->Allocate());
- return AcquireRef(mBindGroupAllocator.Allocate(device, descriptor, descriptorSetAllocation));
+ return AcquireRef(mBindGroupAllocator->Allocate(device, descriptor, descriptorSetAllocation));
}
void BindGroupLayout::DeallocateBindGroup(BindGroup* bindGroup,
DescriptorSetAllocation* descriptorSetAllocation) {
mDescriptorSetAllocator->Deallocate(descriptorSetAllocation);
- mBindGroupAllocator.Deallocate(bindGroup);
+ mBindGroupAllocator->Deallocate(bindGroup);
}
void BindGroupLayout::SetLabelImpl() {
diff --git a/src/dawn/native/vulkan/BindGroupLayoutVk.h b/src/dawn/native/vulkan/BindGroupLayoutVk.h
index 1aa6ff2..fead30d 100644
--- a/src/dawn/native/vulkan/BindGroupLayoutVk.h
+++ b/src/dawn/native/vulkan/BindGroupLayoutVk.h
@@ -17,6 +17,7 @@
#include <vector>
+#include "dawn/common/MutexProtected.h"
#include "dawn/common/SlabAllocator.h"
#include "dawn/common/vulkan_platform.h"
#include "dawn/native/BindGroupLayoutInternal.h"
@@ -70,7 +71,7 @@
VkDescriptorSetLayout mHandle = VK_NULL_HANDLE;
- SlabAllocator<BindGroup> mBindGroupAllocator;
+ MutexProtected<SlabAllocator<BindGroup>> mBindGroupAllocator;
Ref<DescriptorSetAllocator> mDescriptorSetAllocator;
};
diff --git a/src/dawn/tests/unittests/SlabAllocatorTests.cpp b/src/dawn/tests/unittests/SlabAllocatorTests.cpp
index 12c72d9..3e569f8 100644
--- a/src/dawn/tests/unittests/SlabAllocatorTests.cpp
+++ b/src/dawn/tests/unittests/SlabAllocatorTests.cpp
@@ -13,7 +13,6 @@
// limitations under the License.
#include <set>
-#include <thread>
#include <vector>
#include "dawn/common/Math.h"
@@ -180,51 +179,5 @@
}
}
-// Test many allocations and deallocations in a multithreaded environment.
-TEST(SlabAllocatorTests, AllocateDeallocateManyMultithread) {
- static constexpr uint32_t kNumObjectsPerThread = 100;
- static constexpr uint32_t kNumThreads = 10;
-
- SlabAllocator<Foo> allocator(17 * sizeof(Foo));
- auto f = [&] {
- std::set<Foo*> objects;
- std::set<Foo*> set2;
-
- // Allocate many objects.
- for (uint32_t i = 0; i < kNumObjectsPerThread; i++) {
- Foo* object = allocator.Allocate(i);
- EXPECT_TRUE(objects.insert(object).second);
- if (i % 2 == 0) {
- set2.insert(object);
- }
- }
-
- // Deallocate every other object.
- for (Foo* object : set2) {
- allocator.Deallocate(object);
- objects.erase(object);
- }
-
- // Allocate more objects.
- for (uint32_t i = 0; i < kNumObjectsPerThread / 2; i++) {
- Foo* object = allocator.Allocate(i);
- EXPECT_TRUE(objects.insert(object).second);
- }
-
- // Deallocate the rest of the objects
- for (Foo* object : objects) {
- allocator.Deallocate(object);
- }
- };
-
- std::vector<std::thread> threads;
- for (uint32_t i = 0; i < kNumThreads; i++) {
- threads.emplace_back(f);
- }
- for (uint32_t i = 0; i < kNumThreads; i++) {
- threads[i].join();
- }
-}
-
} // anonymous namespace
} // namespace dawn
