Repository navigation
Conversation
This was referenced Oct 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RenderTexture::UpdateUniformBufferupdates the same 192-byte host-coherent uniform buffer every frame, after waiting for the previous frame fence.VulkanBuffer::MapDatamaps and unmaps its allocation on each callback. This repeats driver work even though the allocation and synchronization boundaries are unchanged. The existing example explicitly identifies persistent mapping as outstanding work.Scope is Vulkan only. Evaluated alternatives: partial/pitched texture uploads have real texture/cubemap consumers but still need a concrete incremental-update workload and format/pitch contract; exclusive ownership for query/image resources needs a deliberate compatibility migration and would overlap the ownership/API surface of open #128. Persistent uniform updates have an existing caller, a bounded lifetime, and directly measurable CPU overhead. This change does not touch #128's query pool/timestamp implementation or its callers.
Solution
Add opt-in
VulkanBuffer::MapDataPersistent(callback): lazily map host-visible, host-coherent memory once, check the Vulkan result, and reuse the mapping for subsequent callbacks. Move assignment transfers the mapping with its allocation, clears the source, and unmaps an overwritten destination. Destruction unmaps before releasing resources. Callback exceptions propagate; the persistent allocation remains usable and owned.Use the new entry point for initialization and per-frame updates of the existing DrawTextureDemo uniform buffer. No caller-side raw pointer or extra allocation is needed. For example:
MapData(callback)retains temporary mapping behavior and now releases a mapping through local RAII even when the callback throws. Calling it after persistent access also ends that cached mapping. Both entry points reject empty/moved-from buffers and buffers not created with both required host memory flags. A failed Vulkan map never invokes the callback.The first prototype automatically cached every
MapDataallocation. Measurements showed substantial cross-process noise for large macOS copies, and retaining every staging mapping would broaden address-space/lifetime impact. The final design makes persistence explicit and applies it only to the actual small uniform caller. No non-coherent flush/invalidate API, general allocator, extra dependency, or unrelated cleanup is introduced.Memory semantics follow the Vulkan mapping specification and host-coherent memory requirements. Persistent mapping does not provide synchronization: callers must wait for prior GPU use before CPU writes, submit after writes, use appropriate GPU/host memory dependencies, and wait before reading results. The example's existing fence remains the synchronization boundary.
Validation
All final source files were unchanged between the following full runs and the commit. Both runs used the supported entry point, all default enabled targets/examples, and the core
-O3 -DNDEBUGconfiguration with warnings treated as errors (the runner requests Debug but the core variant supplies these optimization flags)../scripts/run.sh -t macos -r tests./scripts/run.sh -t arm64-v8a -r testsVerified each expected local/
adb shellinvocation, GoogleTest start/completion/pass counts, individual RUN/OK counts, absence of failure/skip summaries, and theset -erunner's successful exit. Three new tests cover repeated host writes -> device-local copy -> persistent host readback over eight rounds; callback failures, mapped-to-mapped move assignment, destruction of the moved-from source, self-move and GPU readback; switching between temporary/persistent modes; empty, moved-from and non-host-requested buffers without invoking callbacks. Existing compute, transfer and image tests exercise temporary mappings. Unmodified-production baseline full runs also passed (51 macOS / 53 Android).macOS: Apple M6/MoltenVK 1.4.1, macOS 27.0.1/SDK 27.0, Apple Clang 21.0.0, CMake 4.4.4, Vulkan SDK 1.4.350.0. Khronos validation layer available/requested in tests; no validation errors observed. Android: sole authorized physical USB 2210132C/nuwa, serial
1dda2807, Android 13/API 33, qcom/Adreno 740, emulator property empty; explicitly fixedANDROID_SERIAL. Existing compatible NDK 28.2.13676358/r28c, Android Clang 19.0.1, target API 34, JDK 17.0.20.1. Explicit shell-local SDK setup and PATH resolved CMake/adb/glslc/slangc/xxd. Android layer availability was not independently confirmed. No install of APKs, device-setting changes, new license acceptance, sudo, shell profile edits or system dependency replacement.ADB 37.0.1 initially crashed in host libunwind; this failed Android attempt was retained and not counted as a pass. Downloaded official Google Platform Tools 35.0.1 into this worktree's ignored build directory, then reused that executable for successful baseline/final tests and benchmarks. A first ADB35 attempt during USB rediscovery found no device and failed; rechecked the sole authorized target before successful reruns. No test skip was used to bypass either failure.
Performance protocol and results
Before production changes, saved and executed original-source benchmark binaries on each platform; raw baseline/final/prototype logs remain in the worktree. The final reproducible
vulkan_mapping_benchmarkcompares temporary and persistent methods inside the same process, using separate identical buffers, alternating mode order, 5 warmups and 31 measured samples per mode/case. Every sample copies all updated buffers through Vulkan and compares every output byte outside timing. Inputs vary between samples. The CPU wall timer includes callback construction/invocation and identical memcpy work, excludes allocation/input generation/GPU copy/readback/destruction, and measures warmed updates (not first mapping). Validation is disabled for timing. No builds ran concurrently with the reported paired benchmarks.Local command:
./build/macos/tests/vulkan/vulkan_mapping_benchmark. Device: push that platform'sbuild/arm64-v8a/tests/vulkan/vulkan_mapping_benchmark, chmod, thenadb shell /data/local/tmp/core/mapping-paired. Each command exited 0 twice, with all four byte-verified cases.Median ns per update, temporary -> persistent (first paired run / independent repeat):
Representative p10/median/p90 ns: M6 single uniform temporary 19.32/20.84/21.12 vs persistent 4.74/5.14/5.46; Android 64 uniforms temporary 182.10/198.74/235.82 vs persistent 94.81/114.81/120.75 (repeat 168.94/209.94/236.88 vs 95.90/116.05/120.85). These support reduced CPU update overhead for the measured small-update workloads. For an uninterrupted sequence of N persistent callbacks on one allocation, source-level driver map/unmap pairs fall from N to 1; this is a code-path count, not an instrumented driver counter. Copied bytes and allocation count are unchanged.
Original-source single-path baseline medians for the four cases were M6 21.06/12.80/41.83/9528.66 ns and Android 44.01/197.67/265.16/61067.72 ns. Cross-process numbers, especially the abandoned automatic-cache prototype's large macOS copy results, varied substantially; final conclusions rely on paired control measurements, not selected cross-process ratios. CPU frequency/core placement, thermal state and scheduling were not controlled. Android single-uniform p10/p90 ranges overlap; 1 MiB results show no repeatable benefit and include a regression. Do not infer a general large-buffer, GPU-time, rendering FPS, cold-start or memory-footprint improvement. The actual graphical example was built, not visually exercised; GPU integration is covered by the tests and byte-checked benchmark.
Changes
vulkan/include/VulkanBuffer.hvulkan/src/VulkanBuffer.cppexamples/vulkan/DrawTextureDemo/RenderTexture.cpptests/vulkan/CMakeLists.txttests/vulkan/VulkanBufferMappingTest.cpptests/vulkan/benchmarks/BufferMappingBenchmark.cppImplementation/API/example +39/-9; tests/benchmark/build +204/-0; documentation +0/-0. Total 6 files, +243/-9 = 252 changed lines, relative to freshly fetched
origin/masteratf09815445638e27d285df999a418ad2d26b22b90. No project Markdown/README changes, generated binaries or unrelated backend implementation changes.Risk
Existing source calls and the original
MapDatasymbol remain compatible; class layout gains a pointer, so binary clients must rebuild. The new API requires the same host-visible/coherent requested flags as temporary access. Persistent host address ranges remain mapped until destruction, move-overwrite, or a temporaryMapDatacall; opt in only for workloads that justify this retained mapping. No guarantee that persistent mapping improves every driver or allocation type.The context must outlive the buffer; queue/buffer access and object movement/destruction remain externally synchronized. Do not retain callback pointers or access memory after unmapping/move/destruction. Concurrent and nested mapping callbacks on the same buffer are not supported. Allocation failure injection, additional GPU vendors, Android validation-layer availability, and graphical output inspection were not tested. No broad ownership/API migration is included.