diff --git a/CMakeLists.txt b/CMakeLists.txt index 29cd63b41..5642d1da8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -451,6 +451,11 @@ add_executable(lru_cache_tests EXCLUDE_FROM_ALL ) target_include_directories(lru_cache_tests PRIVATE ${inc_headers}) +add_executable(slot_vector_tests EXCLUDE_FROM_ALL + "${KYTY_TESTS_DIR}/SlotVectorTests.cpp" +) +target_include_directories(slot_vector_tests PRIVATE ${inc_headers}) + add_executable(ime_dialog_tests EXCLUDE_FROM_ALL "${KYTY_TESTS_DIR}/ImeDialogTests.cpp" "${KYTY_SOURCE_DIR}/libs/imeCommon.cpp" diff --git a/src/common/slotVector.h b/src/common/slotVector.h index b4c7d1049..aec1f3c33 100644 --- a/src/common/slotVector.h +++ b/src/common/slotVector.h @@ -1,15 +1,15 @@ #ifndef EMULATOR_SRC_COMMON_SLOTVECTOR_H_ #define EMULATOR_SRC_COMMON_SLOTVECTOR_H_ -#include "common/assert.h" -#include "common/abi.h" +#include "common/common.h" #include #include -#include #include #include -#include +#include +#include +#include #include #include @@ -19,105 +19,105 @@ struct SlotId { static constexpr uint32_t INVALID_INDEX = std::numeric_limits::max(); constexpr SlotId() noexcept = default; - constexpr SlotId(uint32_t value) noexcept: index(value), generation(1) {} - constexpr SlotId(uint32_t value, uint32_t slot_generation) noexcept - : index(value), generation(slot_generation) {} + constexpr SlotId(uint32_t index) noexcept: index(index) {} - [[nodiscard]] constexpr explicit operator bool() const noexcept { return index != INVALID_INDEX; } + [[nodiscard]] constexpr explicit operator bool() const noexcept { + return index != INVALID_INDEX; + } constexpr auto operator<=>(const SlotId&) const noexcept = default; uint32_t index = INVALID_INDEX; - uint32_t generation = 0; }; -// Stable-address slot storage for cache resources that are intentionally non-movable. +// Fixed-size chunks keep the stored, non-movable resources in place. Occupancy is a byte per +// slot, not a bitset: a deferred erase runs while fault threads read other slots. template class SlotVector { + static constexpr uint32_t ChunkSize = 256; + public: SlotVector() = default; KYTY_CLASS_NO_COPY(SlotVector); + ~SlotVector() noexcept { + ForEach([](SlotId, T& value) { value.~T(); }); + } + [[nodiscard]] T& operator[](SlotId id) noexcept { - EXIT_IF(!is_allocated(id)); - return *m_values[id.index].value; + return m_chunks[id.index / ChunkSize][id.index % ChunkSize].object; } [[nodiscard]] const T& operator[](SlotId id) const noexcept { - EXIT_IF(!is_allocated(id)); - return *m_values[id.index].value; + return m_chunks[id.index / ChunkSize][id.index % ChunkSize].object; } [[nodiscard]] T* try_get(SlotId id) noexcept { - return is_allocated(id) ? &*m_values[id.index].value : nullptr; + return is_allocated(id) ? &(*this)[id] : nullptr; } [[nodiscard]] const T* try_get(SlotId id) const noexcept { - return is_allocated(id) ? &*m_values[id.index].value : nullptr; + return is_allocated(id) ? &(*this)[id] : nullptr; } [[nodiscard]] bool is_allocated(SlotId id) const noexcept { - return id && id.index < m_values.size() && - m_values[id.index].generation == id.generation && m_values[id.index].value.has_value(); + return id.index < m_stored.size() && m_stored[id.index] != 0; } template [[nodiscard]] SlotId insert(Args&&... args) { - uint32_t index = 0; if (m_free_list.empty()) { - index = static_cast(m_values.size()); - m_values.emplace_back(); - m_values.back().value.emplace(std::forward(args)...); - } else { - index = m_free_list.back(); - m_free_list.pop_back(); - EXIT_IF(m_values[index].value.has_value()); - m_values[index].value.emplace(std::forward(args)...); + Reserve(); } - ++m_size; - return SlotId {index, m_values[index].generation}; + const uint32_t index = m_free_list.back(); + m_free_list.pop_back(); + new (&(*this)[index]) T(std::forward(args)...); + m_stored[index] = 1; + return SlotId {index}; } void erase(SlotId id) noexcept { - EXIT_IF(!is_allocated(id)); - auto& slot = m_values[id.index]; - slot.value.reset(); - if (++slot.generation == 0) { - slot.generation = 1; - } + (*this)[id].~T(); + m_stored[id.index] = 0; m_free_list.push_back(id.index); - --m_size; } - [[nodiscard]] size_t size() const noexcept { return m_size; } - [[nodiscard]] size_t capacity() const noexcept { return m_values.size(); } - template void ForEach(F&& fn) { - for (uint32_t index = 0; index < m_values.size(); ++index) { - if (m_values[index].value) { - fn(SlotId {index, m_values[index].generation}, *m_values[index].value); + for (uint32_t index = 0; index < m_stored.size(); ++index) { + if (m_stored[index] != 0) { + fn(SlotId {index}, (*this)[index]); } } } template void ForEach(F&& fn) const { - for (uint32_t index = 0; index < m_values.size(); ++index) { - if (m_values[index].value) { - fn(SlotId {index, m_values[index].generation}, *m_values[index].value); + for (uint32_t index = 0; index < m_stored.size(); ++index) { + if (m_stored[index] != 0) { + fn(SlotId {index}, (*this)[index]); } } } private: - struct Slot { - std::optional value; - uint32_t generation = 1; + union Slot { + Slot() noexcept {} + ~Slot() noexcept {} + + T object; }; - std::deque m_values; - std::vector m_free_list; - size_t m_size = 0; + void Reserve() { + const auto first = static_cast(m_stored.size()); + m_chunks.push_back(std::make_unique(ChunkSize)); + m_stored.resize(first + ChunkSize); + m_free_list.resize(ChunkSize); + std::iota(m_free_list.rbegin(), m_free_list.rend(), first); + } + + std::vector> m_chunks; + std::vector m_stored; + std::vector m_free_list; }; } // namespace Common @@ -125,7 +125,7 @@ private: template <> struct std::hash { [[nodiscard]] size_t operator()(Common::SlotId id) const noexcept { - return std::hash {}((static_cast(id.generation) << 32u) | id.index); + return std::hash {}(id.index); } }; diff --git a/src/graphics/host_gpu/renderer/image/image.h b/src/graphics/host_gpu/renderer/image/image.h index b3c5ae3d3..072255e0a 100644 --- a/src/graphics/host_gpu/renderer/image/image.h +++ b/src/graphics/host_gpu/renderer/image/image.h @@ -139,7 +139,10 @@ public: } static inline std::atomic heap_epoch {0}; + static inline std::atomic serials {0}; + // Unique per image, unlike ImageId: freed slots are reused. + const uint64_t serial = serials.fetch_add(1u, std::memory_order_relaxed) + 1u; ImageInfo info; VulkanImage backing; std::vector views; diff --git a/src/graphics/host_gpu/renderer/pipeline/imageHeap.cpp b/src/graphics/host_gpu/renderer/pipeline/imageHeap.cpp index ae743f537..7c1c50e8c 100644 --- a/src/graphics/host_gpu/renderer/pipeline/imageHeap.cpp +++ b/src/graphics/host_gpu/renderer/pipeline/imageHeap.cpp @@ -350,12 +350,17 @@ void ImageHeap::Release(View& view, uint32_t entry) { } void ImageHeap::Unreference(Element& element) { - if (auto* image = m_context.GetTextureCache().m_slot_images.try_get(element.binding.image_id)) { + if (auto* image = ImageOf(element)) { image->heap_refs.fetch_sub(1u, std::memory_order_relaxed); image->tick_accessed_last = m_context.GetCommandScheduler().CurrentTick(); } } +Image* ImageHeap::ImageOf(const Element& element) const { + auto* image = m_context.GetTextureCache().m_slot_images.try_get(element.binding.image_id); + return image != nullptr && image->serial == element.serial ? image : nullptr; +} + uint32_t ImageHeap::Allocate() { auto& scheduler = m_context.GetCommandScheduler(); while (!m_retired.empty() && scheduler.IsFree(m_retired.front().first)) { @@ -393,7 +398,7 @@ void ImageHeap::Rebind(RenderExecutor& executor) { for (const auto& [value, index]: view->elements) { const auto& binding = m_elements[index].binding; if (!binding.image_id) continue; - const auto* image = textures.m_slot_images.try_get(binding.image_id); + const auto* image = ImageOf(m_elements[index]); if (image == nullptr || !image->registered || image->depth_id || image->binding.needs_rebind) { mark(index); @@ -434,6 +439,7 @@ void ImageHeap::Rebind(RenderExecutor& executor) { if (binding.image_id) { auto& image = textures.GetImage(binding.image_id); image.heap_refs.fetch_add(1u, std::memory_order_relaxed); + m_elements[index].serial = image.serial; binding.layout = image.ReadLayout(); binding.image_view = textures.FindTexture(binding.image_id, binding.desc); if (binding.desc.info.metadata.kind == ImageMetadataKind::Dcc) { @@ -471,8 +477,7 @@ void ImageHeap::WriteDescriptors() { void ImageHeap::Commit(CommandBuffer& buffer, vk::PipelineBindPoint bind_point, vk::PipelineLayout layout) { KYTY_PROFILER_FUNCTION(); - auto& textures = m_context.GetTextureCache(); - const auto epoch = Image::heap_epoch.load(std::memory_order_relaxed); + const auto epoch = Image::heap_epoch.load(std::memory_order_relaxed); for (auto* view: m_active) { if (view->dirty_begin != view->dirty_end) { const auto slots = std::span(view->slots).subspan(view->dirty_begin, @@ -493,7 +498,7 @@ void ImageHeap::Commit(CommandBuffer& buffer, vk::PipelineBindPoint bind_point, // this draw's discovery replaced lives until the tick ends, and its slots still select it. std::erase_if(view->away, [&](uint32_t index) { const auto& element = m_elements[index]; - auto* image = textures.m_slot_images.try_get(element.binding.image_id); + auto* image = ImageOf(element); if (element.refs == 0u || image == nullptr) return true; if (image->binding.is_target || image->binding.shader_write || image->binding.force_general) { diff --git a/src/graphics/host_gpu/renderer/pipeline/imageHeap.h b/src/graphics/host_gpu/renderer/pipeline/imageHeap.h index 2c80fa1e6..41aef7333 100644 --- a/src/graphics/host_gpu/renderer/pipeline/imageHeap.h +++ b/src/graphics/host_gpu/renderer/pipeline/imageHeap.h @@ -57,9 +57,10 @@ private: struct Element { DescriptorValue value; TextureBinding binding; - uint32_t kind = 0; - uint32_t refs = 0; // Slots of the owning view that select it. - bool stale = false; + uint32_t kind = 0; + uint32_t refs = 0; // Slots of the owning view that select it. + bool stale = false; + uint64_t serial = 0; // Image::serial of binding.image_id. }; struct Memory; @@ -109,6 +110,7 @@ private: uint32_t Intern(View& view, uint32_t kind, const DescriptorValue& value); void Release(View& view, uint32_t entry); void Unreference(Element& element); + Image* ImageOf(const Element& element) const; uint32_t Allocate(); void Retire(View& view); void WriteDescriptors(); diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index ea02c9606..c821194af 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -331,22 +331,6 @@ struct TextureCacheTestAccess { return count; } - static void AddPageOwner(TextureCache &cache, uint64_t address, ImageId id) { - std::lock_guard lock(cache.m_lock); - cache - .m_image_page_table[static_cast( - address >> TextureCache::ImagePageTable::kPageBits)] - .push_back(id); - } - - static bool RemovePageOwner(TextureCache &cache, uint64_t address, - ImageId id) { - std::lock_guard lock(cache.m_lock); - auto *owners = cache.m_image_page_table.Find(static_cast( - address >> TextureCache::ImagePageTable::kPageBits)); - return owners != nullptr && owners->Erase(id); - } - static void SetQueryEpoch(TextureCache &cache, uint32_t epoch) { std::lock_guard lock(cache.m_lock); cache.m_image_query_epoch = epoch; @@ -4347,11 +4331,8 @@ public: constexpr uint64_t recycled_offset = index_offset + 4 * index_page; const auto recycled = cache.FindBuffer(base + recycled_offset + 0x100, sizeof(uint32_t)); - Require(name, "slot generation reuse", - (recycled.index == index_left.index && recycled != index_left) || - (recycled.index == index_right.index && - recycled != index_right), - "retired Buffer slot was not reused with a new generation"); + Require(name, "slot reuse", recycled == index_left || recycled == index_right, + "retired Buffer slot was not reused"); const auto [resolved_left, resolved_left_offset] = cache.ObtainBuffer( index_begin + 0x100, sizeof(uint32_t), true, false, index_left); const auto [resolved_right, resolved_right_offset] = @@ -5465,6 +5446,28 @@ public: Require(name, "unmapped descriptor", !textures.IsRegionRegistered(texture(23), 0x1000u), "a descriptor over unmapped memory was resolved"); + // An idle view keeps the element of a freed image. Once a new image takes its slot, + // reactivating the view must resolve the descriptor again. + write(30, descriptor(30, Format::k8_8_8_8UNorm)); + Require(name, "idle element", acquire(unfiltered), "the view was declined"); + draw(); + const auto erased = ImageHeapTestAccess::ImageOf(heap, idle.slots[240]); + textures.UnmapMemory(texture(30), 0x1000u); + Require(name, "replacement", acquire(sampled), "the view was declined"); + draw(); + write(31, descriptor(31, Format::k8_8_8_8UNorm)); + Require(name, "slot reuse", acquire(sampled), "the view was declined"); + draw(); + Require(name, "reused slot", + erased && ImageHeapTestAccess::ImageOf(heap, filtered.slots[248]) == erased, + "the next image did not take the freed image's slot"); + Require(name, "reactivated idle view", acquire(unfiltered), "the view was declined"); + draw(); + Require(name, "reused slot element", + ImageHeapTestAccess::ImageOf(heap, idle.slots[240]) == + ImageHeapTestAccess::ImageOf(heap, filtered.slots[240]), + "a reactivated view kept an element whose image slot holds a new image"); + heap.UnmapMemory(table, table_size); Require(name, "unmapped table", ImageHeapTestAccess::MemoryCount(heap) == 0u && @@ -6060,16 +6063,6 @@ public: large_owner) == 0, "large image unregister left coarse memberships behind"); - const ImageId stale{first.index, first.generation + 1}; - TextureCacheTestAccess::AddPageOwner(texture_cache, base, stale); - const auto stale_filtered = TextureCacheTestAccess::FindImages( - texture_cache, base, sizeof(initial), false); - Require(name, "stale page owner filtering", - IsOnlyImage(stale_filtered, first) && - TextureCacheTestAccess::RemovePageOwner(texture_cache, base, - stale), - "a stale generation escaped the direct page-owner lookup"); - TextureCacheTestAccess::SetQueryEpoch(texture_cache, UINT32_MAX); const auto wrap_results = TextureCacheTestAccess::FindImages( texture_cache, base, sizeof(initial), false); @@ -17009,9 +17002,7 @@ public: scheduler.EndRendering(); RenderExecutorTestAccess::ResetBindings(executor); Require(name, "draw-scoped target reset", - TextureCacheTestAccess::Owner(texture_cache, - target_subresource_id) == nullptr && - !texture_cache.GetImage(target_parent_id).binding.is_target && + !texture_cache.GetImage(target_parent_id).binding.is_target && !texture_cache.GetImage(depth_id).binding.is_target, "RenderExecutor retained target binding state after reset"); @@ -48130,37 +48121,6 @@ void CheckDepthTargetFootprints() { std::printf("[host] %-32s ok\n", "DepthTargetFootprints"); } -struct SlotLifetimeProbe { - explicit SlotLifetimeProbe(bool *destroyed) : destroyed(destroyed) { - if (destroyed == nullptr || *destroyed) { - EXIT("fence-lifetime probe has invalid construction state\n"); - } - } - ~SlotLifetimeProbe() { *destroyed = true; } - - bool *destroyed = nullptr; -}; - -void CheckSlotVectorLifetime() { - bool destroyed = false; - Common::SlotVector slots; - const auto first = slots.insert(&destroyed); - Require("SlotVectorLifetime", "inserted", - slots.is_allocated(first) && slots.try_get(first) != nullptr && - !destroyed, - "slot insertion did not expose its cache-owned object"); - slots.erase(first); - const bool first_destroyed = destroyed; - destroyed = false; - const auto second = slots.insert(&destroyed); - Require("SlotVectorLifetime", "generation", - first_destroyed && !destroyed && first.index == second.index && - first != second && !slots.is_allocated(first) && - slots.is_allocated(second), - "reused slot accepted a stale cache resource id"); - std::printf("[host] %-32s ok\n", "SlotVectorLifetime"); -} - void CheckEmbeddedFetchLaneSpill() { std::vector code; code.push_back(EncodeSMovB32(0, InlineU32(0))); @@ -51297,7 +51257,6 @@ int main(int argc, char **argv) { CheckDepthFeedbackAspects(); CheckDynamicRenderingState(); CheckDepthTargetFootprints(); - CheckSlotVectorLifetime(); #else if (argc != 1) { std::fprintf(stderr, "unknown test selector: %s\n", argv[1]); diff --git a/tests/SlotVectorTests.cpp b/tests/SlotVectorTests.cpp new file mode 100644 index 000000000..15ef95c39 --- /dev/null +++ b/tests/SlotVectorTests.cpp @@ -0,0 +1,103 @@ +#include "common/slotVector.h" + +#include +#include +#include +#include +#include + +namespace { + +void Check(bool value, const char *message) { + if (!value) { + std::fprintf(stderr, "SlotVectorTests: failed: %s\n", message); + std::abort(); + } +} + +// Non-movable like the cached Image/Buffer; counts live objects. +struct Probe { + Probe(int *live, uint32_t value) : live(live), value(value) { ++*live; } + ~Probe() { --*live; } + KYTY_CLASS_NO_COPY(Probe); + + int *live; + uint32_t value; +}; + +static_assert(sizeof(Common::SlotId) == sizeof(uint32_t)); + +void TestInsertEraseReuse() { + int live = 0; + Common::SlotVector slots; + Check(!slots.is_allocated(Common::SlotId {}) && slots.try_get(Common::SlotId {}) == nullptr && + !slots.is_allocated(Common::SlotId {0}), + "an empty vector reported an allocated slot"); + + const auto a = slots.insert(&live, 0u); + const auto b = slots.insert(&live, 1u); + const auto c = slots.insert(&live, 2u); + Check(a.index == 0 && b.index == 1 && c.index == 2 && live == 3, + "fresh slots were not handed out in order"); + Check(slots.is_allocated(b) && slots.try_get(b) == &slots[b] && slots[b].value == 1, + "an inserted slot did not expose its object"); + Check(!slots.is_allocated(Common::SlotId {3}), "an unused slot reported an object"); + + slots.erase(b); + slots.erase(a); + Check(live == 1 && !slots.is_allocated(a) && !slots.is_allocated(b) && + slots.try_get(b) == nullptr && slots.is_allocated(c), + "erase did not destroy and free exactly its slot"); + + // Freed indices are reused most recently freed first, with the same plain id. + const auto reused_a = slots.insert(&live, 0u); + const auto reused_b = slots.insert(&live, 1u); + const auto d = slots.insert(&live, 3u); + Check(reused_a == a && reused_b == b && d.index == 3 && live == 4, + "freed slots were not reused in LIFO order"); + Check(slots[reused_a].value == 0 && slots[reused_b].value == 1, + "a reused slot did not expose its new object"); +} + +void TestForEachAndStability() { + int live = 0; + { + Common::SlotVector slots; + const auto first = slots.insert(&live, 0u); + Probe *const first_address = &slots[first]; + + // Cross several chunks; earlier objects must keep their address. + std::vector ids {first}; + for (uint32_t i = 1; i < 1000; ++i) { + ids.push_back(slots.insert(&live, i)); + } + Check(&slots[first] == first_address && live == 1000, + "growing the vector moved a stored object"); + + std::vector expected; + for (uint32_t i = 0; i < 1000; ++i) { + if (i % 3 == 0) { + slots.erase(ids[i]); + } else { + expected.push_back(i); + } + } + std::vector visited; + std::as_const(slots).ForEach([&](Common::SlotId id, const Probe &probe) { + Check(probe.value == id.index, "ForEach passed an id that does not match its object"); + visited.push_back(id.index); + }); + Check(visited == expected && live == static_cast(expected.size()), + "ForEach did not visit exactly the live slots in index order"); + } + Check(live == 0, "destroying the vector did not destroy every live object exactly once"); +} + +} // namespace + +int main() { + TestInsertEraseReuse(); + TestForEachAndStability(); + std::puts("SlotVectorTests: all cases passed"); + return 0; +}