Don't expose the sampling routine cache's mutex
The getOrCreate() method takes care of the mutex locking and unlocking,
and takes the function for creating the routine as a callback argument.
Bug: b/131246679
Change-Id: I0873f6be94c92a11181ce575d1b44ed040177d43
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/42869
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/Pipeline/SpirvShaderSampling.cpp b/src/Pipeline/SpirvShaderSampling.cpp
index adc465b..dd84e70 100644
--- a/src/Pipeline/SpirvShaderSampling.cpp
+++ b/src/Pipeline/SpirvShaderSampling.cpp
@@ -45,58 +45,54 @@
return (ImageSampler *)(routine->getEntry());
}
- std::unique_lock<std::mutex> lock(imageDescriptor->device->getSamplingRoutineCacheMutex());
vk::Device::SamplingRoutineCache *cache = imageDescriptor->device->getSamplingRoutineCache();
- auto routine = cache->query(key);
- if(routine)
- {
- return (ImageSampler *)(routine->getEntry());
- }
+ auto createSamplingRoutine = [&](const vk::Device::SamplingRoutineCache::Key &key) {
+ auto type = imageDescriptor->type;
- auto type = imageDescriptor->type;
+ Sampler samplerState = {};
+ samplerState.textureType = type;
+ samplerState.textureFormat = imageDescriptor->format;
- Sampler samplerState = {};
- samplerState.textureType = type;
- samplerState.textureFormat = imageDescriptor->format;
-
- samplerState.addressingModeU = convertAddressingMode(0, sampler, type);
- samplerState.addressingModeV = convertAddressingMode(1, sampler, type);
- samplerState.addressingModeW = convertAddressingMode(2, sampler, type);
- samplerState.addressingModeY = convertAddressingMode(3, sampler, type);
-
- samplerState.mipmapFilter = convertMipmapMode(sampler);
- samplerState.swizzle = imageDescriptor->swizzle;
- samplerState.gatherComponent = instruction.gatherComponent;
- samplerState.highPrecisionFiltering = false;
- samplerState.largeTexture = (imageDescriptor->extent.width > SHRT_MAX) ||
- (imageDescriptor->extent.height > SHRT_MAX) ||
- (imageDescriptor->extent.depth > SHRT_MAX);
-
- if(sampler)
- {
- samplerState.textureFilter = (instruction.samplerMethod == Gather) ? FILTER_GATHER : convertFilterMode(sampler);
- samplerState.border = sampler->borderColor;
+ samplerState.addressingModeU = convertAddressingMode(0, sampler, type);
+ samplerState.addressingModeV = convertAddressingMode(1, sampler, type);
+ samplerState.addressingModeW = convertAddressingMode(2, sampler, type);
+ samplerState.addressingModeY = convertAddressingMode(3, sampler, type);
samplerState.mipmapFilter = convertMipmapMode(sampler);
+ samplerState.swizzle = imageDescriptor->swizzle;
+ samplerState.gatherComponent = instruction.gatherComponent;
+ samplerState.highPrecisionFiltering = false;
+ samplerState.largeTexture = (imageDescriptor->extent.width > SHRT_MAX) ||
+ (imageDescriptor->extent.height > SHRT_MAX) ||
+ (imageDescriptor->extent.depth > SHRT_MAX);
- samplerState.compareEnable = (sampler->compareEnable != VK_FALSE);
- samplerState.compareOp = sampler->compareOp;
- samplerState.unnormalizedCoordinates = (sampler->unnormalizedCoordinates != VK_FALSE);
+ if(sampler)
+ {
+ samplerState.textureFilter = (instruction.samplerMethod == Gather) ? FILTER_GATHER : convertFilterMode(sampler);
+ samplerState.border = sampler->borderColor;
- samplerState.ycbcrModel = sampler->ycbcrModel;
- samplerState.studioSwing = sampler->studioSwing;
- samplerState.swappedChroma = sampler->swappedChroma;
+ samplerState.mipmapFilter = convertMipmapMode(sampler);
- samplerState.mipLodBias = sampler->mipLodBias;
- samplerState.maxAnisotropy = sampler->maxAnisotropy;
- samplerState.minLod = sampler->minLod;
- samplerState.maxLod = sampler->maxLod;
- }
+ samplerState.compareEnable = (sampler->compareEnable != VK_FALSE);
+ samplerState.compareOp = sampler->compareOp;
+ samplerState.unnormalizedCoordinates = (sampler->unnormalizedCoordinates != VK_FALSE);
- routine = emitSamplerRoutine(instruction, samplerState);
+ samplerState.ycbcrModel = sampler->ycbcrModel;
+ samplerState.studioSwing = sampler->studioSwing;
+ samplerState.swappedChroma = sampler->swappedChroma;
- cache->add(key, routine);
+ samplerState.mipLodBias = sampler->mipLodBias;
+ samplerState.maxAnisotropy = sampler->maxAnisotropy;
+ samplerState.minLod = sampler->minLod;
+ samplerState.maxLod = sampler->maxLod;
+ }
+
+ return emitSamplerRoutine(instruction, samplerState);
+ };
+
+ auto routine = cache->getOrCreate(key, createSamplingRoutine);
+
return (ImageSampler *)(routine->getEntry());
}
diff --git a/src/System/Debug.cpp b/src/System/Debug.cpp
index 1b91e14..df7ab45 100644
--- a/src/System/Debug.cpp
+++ b/src/System/Debug.cpp
@@ -14,11 +14,6 @@
#include "Debug.hpp"
-#include <atomic>
-#include <cstdarg>
-#include <cstdio>
-#include <string>
-
#if __ANDROID__
# include <android/log.h>
#endif
@@ -34,6 +29,11 @@
# include <unistd.h>
#endif
+#include <atomic>
+#include <cstdarg>
+#include <cstdio>
+#include <string>
+
#ifdef ERROR
# undef ERROR // b/127920555
#endif
diff --git a/src/Vulkan/VkDevice.cpp b/src/Vulkan/VkDevice.cpp
index 492c676..982c10a 100644
--- a/src/Vulkan/VkDevice.cpp
+++ b/src/Vulkan/VkDevice.cpp
@@ -38,17 +38,6 @@
namespace vk {
-std::shared_ptr<rr::Routine> Device::SamplingRoutineCache::query(const vk::Device::SamplingRoutineCache::Key &key) const
-{
- return cache.query(key);
-}
-
-void Device::SamplingRoutineCache::add(const vk::Device::SamplingRoutineCache::Key &key, const std::shared_ptr<rr::Routine> &routine)
-{
- ASSERT(routine);
- cache.add(key, routine);
-}
-
rr::Routine *Device::SamplingRoutineCache::querySnapshot(const vk::Device::SamplingRoutineCache::Key &key) const
{
return cache.querySnapshot(key).get();
@@ -56,6 +45,8 @@
void Device::SamplingRoutineCache::updateSnapshot()
{
+ std::lock_guard<std::mutex> lock(mutex);
+
cache.updateSnapshot();
}
@@ -309,15 +300,9 @@
void Device::updateSamplingRoutineSnapshotCache()
{
- std::unique_lock<std::mutex> lock(samplingRoutineCacheMutex);
samplingRoutineCache->updateSnapshot();
}
-std::mutex &Device::getSamplingRoutineCacheMutex()
-{
- return samplingRoutineCacheMutex;
-}
-
uint32_t Device::indexSampler(const SamplerState &samplerState)
{
return samplerIndexer->index(samplerState);
diff --git a/src/Vulkan/VkDevice.hpp b/src/Vulkan/VkDevice.hpp
index 2a52473..3ba442d 100644
--- a/src/Vulkan/VkDevice.hpp
+++ b/src/Vulkan/VkDevice.hpp
@@ -86,18 +86,31 @@
};
};
- std::shared_ptr<rr::Routine> query(const Key &key) const;
- void add(const Key &key, const std::shared_ptr<rr::Routine> &routine);
+ template<typename Function>
+ std::shared_ptr<rr::Routine> getOrCreate(const Key &key, Function createRoutine)
+ {
+ std::lock_guard<std::mutex> lock(mutex);
+
+ if(auto existingRoutine = cache.query(key))
+ {
+ return existingRoutine;
+ }
+
+ std::shared_ptr<rr::Routine> newRoutine = createRoutine(key);
+ cache.add(key, newRoutine);
+
+ return newRoutine;
+ }
rr::Routine *querySnapshot(const Key &key) const;
void updateSnapshot();
private:
- sw::LRUSnapshotCache<Key, std::shared_ptr<rr::Routine>, Key::Hash> cache;
+ sw::LRUSnapshotCache<Key, std::shared_ptr<rr::Routine>, Key::Hash> cache; // guarded by mutex
+ std::mutex mutex;
};
SamplingRoutineCache *getSamplingRoutineCache() const;
- std::mutex &getSamplingRoutineCacheMutex();
rr::Routine *querySnapshotCache(const SamplingRoutineCache::Key &key) const;
void updateSamplingRoutineSnapshotCache();
@@ -139,14 +152,13 @@
Queue *const queues = nullptr;
uint32_t queueCount = 0;
std::unique_ptr<sw::Blitter> blitter;
- std::unique_ptr<SamplingRoutineCache> samplingRoutineCache;
- std::mutex samplingRoutineCacheMutex;
uint32_t enabledExtensionCount = 0;
typedef char ExtensionName[VK_MAX_EXTENSION_NAME_SIZE];
ExtensionName *extensions = nullptr;
const VkPhysicalDeviceFeatures enabledFeatures = {};
std::shared_ptr<marl::Scheduler> scheduler;
+ std::unique_ptr<SamplingRoutineCache> samplingRoutineCache;
std::unique_ptr<SamplerIndexer> samplerIndexer;
#ifdef ENABLE_VK_DEBUGGER
