[perf] SaveFrame readback uses HOST_COHERENT (write-combined) — should be HOST_CACHED #74

New issue

Closed

opened 2026-06-16 17:10:54 +02:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-06-16 17:10:54 +02:00

Owner

Subsystem: Window / frame capture
Location: implementations/Crafter.Graphics-Window.cpp:1129
Impact: Low · Effort: Small · Not hot path (screenshot/debug)
Related: #60 (coherency / invalidate gating), #89 (upload-strategy helper — explicitly NOT for this path, see caveat 4)

Problem

SaveFrame allocates its readback staging buffer HOST_VISIBLE | HOST_COHERENT, copies the frame image into it (TRANSFER_DST), then the CPU reads the whole buffer to write to disk. On most discrete GPUs the first HOST_VISIBLE|HOST_COHERENT memory type is write-combined (uncached) — and CPU reads from write-combined memory are pathologically slow (every read is an uncached memory transaction). This is the textbook wrong memory type for a readback path: coherency here buys nothing, and the WC penalty applies to the entire frame.

Proposed fix

Allocate readback buffers HOST_VISIBLE | HOST_CACHED (prefer also HOST_COHERENT if a cached-coherent type exists). If the chosen type is not coherent, call vkInvalidateMappedMemoryRanges before the CPU read (respecting nonCoherentAtomSize). Cached reads are dramatically faster for the full-frame copy.

Correctness caveats

HOST_CACHED is not spec-guaranteed but is near-universal; fall back to HOST_VISIBLE if absent (the #59 mechanism).
The invalidate is mandatory on a non-coherent cached type — it is the read-path counterpart of the flush discussed in #60. Gate it on the chosen type's coherency, not the requested flags.
This is a one-shot debug/capture path, so impact is low — filing for correctness of the type choice, not frame cost.
Do NOT route this through #89. That helper is upload-only (CPU-write -> GPU-read) and steers toward DEVICE_LOCAL|HOST_VISIBLE, which on this hardware is coherent-not-cached (write-combined) — the worst type for readback. This path wants the opposite (HOST_CACHED). The two are separate primitives by design.

**Subsystem:** Window / frame capture **Location:** `implementations/Crafter.Graphics-Window.cpp:1129` **Impact:** Low · **Effort:** Small · Not hot path (screenshot/debug) **Related:** #60 (coherency / invalidate gating), #89 (upload-strategy helper — **explicitly NOT for this path**, see caveat 4) ### Problem `SaveFrame` allocates its readback staging buffer `HOST_VISIBLE | HOST_COHERENT`, copies the frame image into it (`TRANSFER_DST`), then the CPU reads the whole buffer to write to disk. On most discrete GPUs the first `HOST_VISIBLE|HOST_COHERENT` memory type is **write-combined (uncached)** — and CPU **reads** from write-combined memory are pathologically slow (every read is an uncached memory transaction). This is the textbook wrong memory type for a readback path: coherency here buys nothing, and the WC penalty applies to the entire frame. ### Proposed fix Allocate readback buffers `HOST_VISIBLE | HOST_CACHED` (prefer also `HOST_COHERENT` if a cached-coherent type exists). If the chosen type is not coherent, call `vkInvalidateMappedMemoryRanges` before the CPU read (respecting `nonCoherentAtomSize`). Cached reads are dramatically faster for the full-frame copy. ### Correctness caveats 1. `HOST_CACHED` is not spec-guaranteed but is near-universal; fall back to `HOST_VISIBLE` if absent (the #59 mechanism). 2. The invalidate is mandatory on a non-coherent cached type — it is the read-path counterpart of the flush discussed in #60. Gate it on the chosen type's coherency, not the requested flags. 3. This is a one-shot debug/capture path, so impact is low — filing for correctness of the type choice, not frame cost. 4. **Do NOT route this through #89.** That helper is upload-only (CPU-write -> GPU-read) and steers toward `DEVICE_LOCAL|HOST_VISIBLE`, which on this hardware is coherent-not-cached (write-combined) — the *worst* type for readback. This path wants the opposite (`HOST_CACHED`). The two are separate primitives by design.

jorijnvdgraaf referenced this issue

2026-06-16 18:55:02 +02:00

[perf] Runtime upload-strategy helper: direct HOST_VISIBLE|DEVICE_LOCAL vs staging (ReBAR-aware) #89