From e46890adb49ad9d3c240866d942ebe0e1a517b3a Mon Sep 17 00:00:00 2001 From: ekisu Date: Mon, 19 Oct 2026 19:19:55 -0300 Subject: [PATCH 1/2] perf(indium): cache buffer address keys in CPU memory --- patches/metal--deps--indium.patch | 49 ++++++- tests/fixtures/indium-address-shadow.cpp | 175 +++++++++++++++++++++++ tests/test_indium_address_shadow.py | 50 +++++++ 3 files changed, 267 insertions(+), 7 deletions(-) create mode 100644 tests/fixtures/indium-address-shadow.cpp create mode 100644 tests/test_indium_address_shadow.py diff --git a/patches/metal--deps--indium.patch b/patches/metal--deps--indium.patch index f3c149f..f6c73e5 100644 --- a/patches/metal--deps--indium.patch +++ b/patches/metal--deps--indium.patch @@ -289,7 +289,7 @@ index b7ffcce..5b1e84c 100644 }; }; diff --git a/private-include/indium/command-encoder.private.hpp b/private-include/indium/command-encoder.private.hpp -index c3b1b93..3dab2ce 100644 +index c3b1b93..6b8783d 100644 --- a/private-include/indium/command-encoder.private.hpp +++ b/private-include/indium/command-encoder.private.hpp @@ -1,6 +1,7 @@ @@ -300,7 +300,7 @@ index c3b1b93..3dab2ce 100644 #include #include #include -@@ -17,117 +18,295 @@ +@@ -17,117 +18,330 @@ #include @@ -529,6 +529,40 @@ index c3b1b93..3dab2ce 100644 + struct AddressPool { + std::shared_ptr chunk; + size_t used = 0; ++ ++ // Two shader stages are live in the render encoder. Cache their small ++ // immutable tables in CPU memory, not in the GPU-mapped scratch chunk. ++ // This is only a read cache: misses retain the original byte comparison. ++ struct Shadow { ++ bool valid = false; ++ uint32_t offset; ++ VkDeviceSize range; ++ size_t count; ++ std::array addresses; ++ }; ++ std::array shadows; ++ size_t nextShadow = 0; ++ ++ bool sameAddresses(uint32_t offset, VkDeviceSize range, const uint64_t* addresses, size_t count) const { ++ for (const auto& shadow : shadows) { ++ // A table's last eight bytes can be padding. If a new layout ++ // reads that padding, only the mapped bytes define the old key. ++ if (shadow.valid && shadow.offset == offset && shadow.range == range && count <= shadow.count) ++ return !memcmp(shadow.addresses.data(), addresses, count * sizeof(uint64_t)); ++ } ++ return !memcmp(static_cast(chunk->contents()) + offset, addresses, count * sizeof(uint64_t)); ++ } ++ ++ void rememberAddresses(uint32_t offset, VkDeviceSize range, const uint64_t* addresses, size_t count) { ++ if (count > 64) return; ++ auto& shadow = shadows[nextShadow]; ++ nextShadow = (nextShadow + 1) % shadows.size(); ++ shadow.offset = offset; ++ shadow.range = range; ++ shadow.count = count; ++ memcpy(shadow.addresses.data(), addresses, count * sizeof(uint64_t)); ++ shadow.valid = true; ++ } + }; - if (functionResources.buffers.size() > 0) { @@ -583,17 +617,18 @@ index c3b1b93..3dab2ce 100644 + if (bindingReuseEnabled() && previous && addressPool.chunk && previous->range == tableSize && + previous->offset <= chunkSize && tableSize <= chunkSize - previous->offset && + previous->buffer == static_cast(addressPool.chunk.get())->buffer() && -+ !memcmp(static_cast(addressPool.chunk->contents()) + previous->offset, -+ addresses.data(), addresses.size() * sizeof(uint64_t))) ++ addressPool.sameAddresses(previous->offset, previous->range, addresses.data(), addresses.size())) + return *previous; + size_t tableOffset = (addressPool.used + alignment - 1) & ~(alignment - 1); + if (!addressPool.chunk || tableOffset + tableSize > chunkSize) { + addressPool.chunk = privateDevice->acquireScratchChunk(); ++ for (auto& shadow : addressPool.shadows) shadow.valid = false; + // we need to keep the chunk alive until the operation is completed + keepAliveBuffers.push_back(addressPool.chunk); + tableOffset = 0; + } + memcpy(static_cast(addressPool.chunk->contents()) + tableOffset, addresses.data(), addresses.size() * sizeof(uint64_t)); ++ if (previous) addressPool.rememberAddresses(static_cast(tableOffset), tableSize, addresses.data(), addresses.size()); + addressPool.used = tableOffset + tableSize; + // plain pointer: a shared_ptr cast here was an atomic reference count round trip per stage per draw + auto* privateAddressBuffer = static_cast(addressPool.chunk.get()); @@ -654,7 +689,7 @@ index c3b1b93..3dab2ce 100644 descSet.descriptorCount = 1; descSet.pBufferInfo = &info; } -@@ -136,17 +315,27 @@ namespace Indium { +@@ -136,17 +350,27 @@ namespace Indium { auto& bindingInfo = funcInfo.bindings[j]; if (bindingInfo.type == Iridium::BindingType::Texture) { @@ -691,7 +726,7 @@ index c3b1b93..3dab2ce 100644 auto& descSet = writeDescSet.emplace_back(); descSet.sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; descSet.dstSet = descriptorSets[i]; -@@ -156,19 +345,21 @@ namespace Indium { +@@ -156,19 +380,21 @@ namespace Indium { descSet.descriptorCount = 1; descSet.pImageInfo = &info; } else if (bindingInfo.type == Iridium::BindingType::Sampler) { @@ -724,7 +759,7 @@ index c3b1b93..3dab2ce 100644 info.sampler = privateSampler->sampler(); auto& descSet = writeDescSet.emplace_back(); -@@ -188,10 +379,4 @@ namespace Indium { +@@ -188,10 +414,4 @@ namespace Indium { return descriptorSets; }; diff --git a/tests/fixtures/indium-address-shadow.cpp b/tests/fixtures/indium-address-shadow.cpp new file mode 100644 index 0000000..4d10ebd --- /dev/null +++ b/tests/fixtures/indium-address-shadow.cpp @@ -0,0 +1,175 @@ +// CPU-only buffer/Vulkan shims; the scratch, pool and table helper are extracted +// from the authoritative component patch, not copied into this fixture. +#include +#include +#include +#include +#include +#include +#include +#include +#include +static size_t allocationCalls = 0; +void* operator new(size_t size) { + ++allocationCalls; + if (auto* p = std::malloc(size ? size : 1)) return p; + throw std::bad_alloc(); +} +void operator delete(void* p) noexcept { std::free(p); } +void operator delete(void* p, size_t) noexcept { std::free(p); } +using VkBuffer = uint64_t; +using VkDeviceSize = uint64_t; +constexpr VkBuffer VK_NULL_HANDLE = 0; +namespace Iridium { enum class BindingType { Buffer, Texture, Sampler }; } +static bool enabled = true; +bool bindingReuseEnabled() { return enabled; } +void logUnboundResource(const char*, size_t, size_t, size_t) {} +struct Buffer { virtual ~Buffer() = default; virtual void* contents() = 0; }; +struct PrivateBuffer : Buffer { + uint64_t id, address; + size_t reads = 0; + std::vector bytes; + PrivateBuffer(uint64_t id, uint64_t address) : id(id), address(address), bytes(256*1024, 0xa5) {} + void* contents() override { ++reads; return bytes.data(); } + VkBuffer buffer() const { return id; } + uint64_t gpuAddress() const { return address; } +}; +struct PrivateDevice { + size_t chunks = 0; + uint64_t fallback = 0xf00d; + uint64_t fallbackBufferAddress() { return fallback; } + std::shared_ptr acquireScratchChunk() { ++chunks; return std::make_shared(chunks, chunks << 20); } +}; +struct FunctionResources { std::vector, size_t>> buffers; }; +struct Resolved { PrivateBuffer* value = nullptr; }; +struct ResolvedFunctionResources { std::vector buffers; }; +struct Binding { Iridium::BindingType type; size_t index; }; +struct FunctionInfo { std::vector bindings; }; + +/* ACTUAL_PATCHED_HELPER */ + +struct Check { + AddressPool pool; + std::shared_ptr device = std::make_shared(); + std::vector> keep; + std::array tables{}; + Check() { keep.reserve(8); } + PrivateBuffer* mapped() { return static_cast(pool.chunk.get()); } + void run(size_t stage, const FunctionResources& resources, const FunctionInfo& info, + bool reuse, const ResolvedFunctionResources* resolved = nullptr, bool previous = true) { + auto old = tables[stage]; + auto used = pool.used, chunks = device->chunks, calls = allocationCalls; + auto table = makeBufferAddressTable(resources, info, resolved, device, keep, pool, + previous ? &tables[stage] : nullptr); + // Only the existing DescriptorScratch overflow and new scratch chunk + // may allocate (make_shared control/object plus the chunk byte vector). + assert(allocationCalls - calls == (info.bindings.size() > 64 ? 1u : 0u) + + (device->chunks - chunks) * 2); + assert((table.buffer == old.buffer && table.offset == old.offset && pool.used == used) == reuse); + assert(table.range == ((std::count_if(info.bindings.begin(), info.bindings.end(), + [](const Binding& b) { return b.type == Iridium::BindingType::Buffer; }) * 8 + 15) & ~size_t(15))); + assert(keep.size() == device->chunks); + size_t index = 0; + for (const auto& binding : info.bindings) { + if (binding.type != Iridium::BindingType::Buffer) continue; + PrivateBuffer* buffer = nullptr; + if (resolved && binding.index < resolved->buffers.size()) buffer = resolved->buffers[binding.index].value; + else if (binding.index < resources.buffers.size()) buffer = dynamic_cast(resources.buffers[binding.index].first.get()); + uint64_t expected = buffer ? buffer->gpuAddress() + resources.buffers[binding.index].second : device->fallback; + uint64_t actual; + memcpy(&actual, mapped()->bytes.data() + table.offset + index++ * 8, 8); + assert(actual == expected); + } + tables[stage] = table; + } +}; +FunctionInfo info(size_t count, size_t start = 0) { + FunctionInfo result; + for (size_t i = 0; i < count; ++i) result.bindings.push_back({Iridium::BindingType::Buffer, start+i}); + return result; +} +int main() { + // Two 64-key entries: +1080 bytes per pool on the tested 64-bit ABI. + static_assert(sizeof(AddressPool) == sizeof(std::shared_ptr) + sizeof(size_t) + 1080); + FunctionResources r; + auto resource = std::make_shared(100, 0x1000); + for (size_t i = 0; i < 150; ++i) r.buffers.push_back({resource, i*16}); + Check p; + auto f = info(3); + p.run(0, r, f, false); + auto accesses = p.mapped()->reads; + p.run(0, r, f, true); assert(p.mapped()->reads == accesses); + r.buffers[100].second++; p.run(0, r, f, true); + auto equivalent = std::make_shared(101, 0x0ff0); + r.buffers[0] = {equivalent, 16}; p.run(0, r, f, true); + r.buffers[1].second++; p.run(0, r, f, false); + auto mixed = f; + mixed.bindings.push_back({Iridium::BindingType::Texture, 999}); + mixed.bindings.push_back({Iridium::BindingType::Sampler, 999}); + p.run(0, r, mixed, true); + p.run(1, r, info(4, 20), false); + accesses = p.mapped()->reads; + p.run(0, r, f, true); p.run(1, r, info(4, 20), true); + assert(p.mapped()->reads == accesses); // both interleaved stages hit CPU keys + for (size_t i = 0; i < 5; ++i) { + r.buffers[20].second++; p.run(1, r, info(4, 20), false); + } + accesses = p.mapped()->reads; + p.run(0, r, f, true); + assert(p.mapped()->reads == accesses + 1); // eviction retains mapped fallback + + Check padding; + padding.run(0, r, info(1), false); + r.buffers[1] = {resource, uint64_t(0xa5a5a5a5a5a5a5a5ULL - resource->gpuAddress())}; + accesses = padding.mapped()->reads; + padding.run(0, r, info(2), true); // odd -> even reads real padding + assert(padding.mapped()->reads == accesses + 1); + r.buffers[1].second++; padding.run(0, r, info(2), false); + accesses = padding.mapped()->reads; + padding.run(0, r, info(1), true); // even -> odd needs only cached prefix + assert(padding.mapped()->reads == accesses); + padding.run(0, r, info(3), false); + + Check large; + for (size_t count : {size_t(64), size_t(65), size_t(150), size_t(32767), size_t(32768)}) { + large.run(0, r, info(count), false); + large.run(0, r, info(count), true); + } + r.buffers[64].second++; large.run(0, r, info(65), false); + large.run(1, r, info(8), false, nullptr, false); + large.run(1, r, info(8), true); + + Check missing; + FunctionResources empty; + missing.run(0, empty, info(2, 999), false); + missing.run(0, empty, info(2, 999), true); + missing.device->fallback++; missing.run(0, empty, info(2, 999), false); + ResolvedFunctionResources resolved; + resolved.buffers.resize(r.buffers.size()); resolved.buffers[0].value = resource.get(); + missing.run(1, r, info(1), false, &resolved); + missing.run(1, r, info(1), true, &resolved); + resolved.buffers[0].value = nullptr; + missing.run(1, r, info(1), false, &resolved); + + Check roll; + roll.run(0, r, info(2), false); roll.run(1, r, info(2, 3), false); + auto oldChunk = roll.pool.chunk; + roll.pool.used = 256*1024; + r.buffers[0].second++; roll.run(0, r, info(2), false); + assert(roll.pool.chunk != oldChunk && roll.keep.size() == 2); + // Old offset 256 must not supply a stale key for untouched new-chunk bytes. + roll.tables[1] = {roll.tables[0].buffer, 256, 16}; + roll.run(1, r, info(2, 3), false); roll.run(1, r, info(2, 3), true); + enabled = false; roll.run(1, r, info(2, 3), false); enabled = true; + + Check guard; + guard.run(0, r, info(0), false); guard.run(0, r, info(0), true); + auto id = guard.tables[0].buffer; + guard.tables[0] = {id, 256*1024, 0}; guard.run(0, r, info(0), true); + guard.tables[0] = {id, 256*1024+1, 0}; guard.run(0, r, info(0), false); + accesses = guard.mapped()->reads; + guard.tables[0] = {id, 256*1024-8, 16}; guard.run(0, r, info(1), false); + assert(guard.mapped()->reads == accesses + 1); // write only; no invalid read + guard.tables[0].buffer += 100; guard.run(0, r, info(1), false); + guard.tables[0].range = 32; guard.run(0, r, info(1), false); +} diff --git a/tests/test_indium_address_shadow.py b/tests/test_indium_address_shadow.py new file mode 100644 index 0000000..1a8e886 --- /dev/null +++ b/tests/test_indium_address_shadow.py @@ -0,0 +1,50 @@ +"""Compile the shipped address-table helper against an inert CPU-only fixture.""" +from pathlib import Path +import re +import shutil +import subprocess +import tempfile +import unittest + +ROOT = Path(__file__).resolve().parents[1] +HEADER = 'private-include/indium/command-encoder.private.hpp' + + +def address_helper(): + patch = (ROOT / 'patches/metal--deps--indium.patch').read_text() + marker = f'diff --git a/{HEADER} b/{HEADER}\n' + section = patch.split(marker, 1)[1].split('\ndiff --git ', 1)[0] + # Extract each postimage hunk separately: never silently join a source gap. + hunks = re.split(r'(?m)^@@ [^\n]*\n', section)[1:] + postimages = [''.join(line[1:] for line in hunk.splitlines(keepends=True) + if line.startswith((' ', '+'))) for hunk in hunks] + + def region(start, end): + matches = [text[text.index(start):text.index(end, text.index(start))] + for text in postimages if start in text and end in text] + if len(matches) != 1: + raise AssertionError('expected one complete patched region: ' + start) + return matches[0] + + return (region(' template class DescriptorScratch', + ' // Per-encoder binding tables') + + region('\tstruct AddressPool {', '\n\ttemplate')) + + +@unittest.skipUnless(shutil.which('c++'), 'C++ compiler required') +class AddressShadowChecks(unittest.TestCase): + def test_interleaving_fallback_padding_rollover_large_keys_and_allocations(self): + fixture = (ROOT / 'tests/fixtures/indium-address-shadow.cpp').read_text() + with tempfile.TemporaryDirectory() as folder: + path = Path(folder) + source = path / 'check.cpp' + source.write_text(fixture.replace('/* ACTUAL_PATCHED_HELPER */', address_helper())) + build = subprocess.run(['c++', '-std=c++17', '-O1', str(source), '-o', str(path/'check')], + capture_output=True, text=True) + self.assertEqual(build.returncode, 0, build.stderr) + result = subprocess.run([str(path/'check')], capture_output=True, text=True, timeout=30) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + +if __name__ == '__main__': + unittest.main() From fb520d4d71a7997189fd8a011f27ad32c50062db Mon Sep 17 00:00:00 2001 From: ekisu Date: Tue, 20 Oct 2026 00:34:25 -0300 Subject: [PATCH 2/2] test(indium): run accepted address shadow regression in CI --- .github/workflows/source-check.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/source-check.yml b/.github/workflows/source-check.yml index 3e91493..d8c96b5 100644 --- a/.github/workflows/source-check.yml +++ b/.github/workflows/source-check.yml @@ -16,6 +16,10 @@ jobs: - uses: actions/setup-python@v5 with: python-version: '3.12' + - name: Require C++ regression compiler + run: | + command -v c++ + c++ --version - run: python scripts/validate.py - run: python tools/riot-input-offline-check.py - run: python tools/riot-launcher-offline-check.py @@ -28,5 +32,6 @@ jobs: - run: python -m unittest discover -s tests -p 'test_urlsession_runner.py' - run: python -m unittest discover -s tests -p 'test_system_profiler.py' - run: python -m unittest discover -s tests -p 'test_metal_device_info.py' + - run: python -m unittest discover -s tests -p 'test_indium_address_shadow.py' -v - run: python -m unittest discover -s tests -p 'test_exec_guest.py' - run: python -m unittest discover -s tests -p 'test_close_league_client_in_game.py'