vulkan: route download through staging when blob memory is not HOST_CACHED (13x, fixes #6857)

[giuliocorradi]

Fixes #6857. Opening as requested in #6857 (comment) — using the is_cached() on VkAllocator approach you suggested.

Problem

VkCompute::record_download() skips the staging buffer whenever the blob allocator is mappable:

Option opt_staging = opt;
if (!opt_staging.blob_vkallocator->mappable)
{
    opt_staging.blob_vkallocator = opt.staging_vkallocator;
}

On an integrated GPU the blob allocator prefers DEVICE_LOCAL | HOST_VISIBLE unified memory. That memory is mappable — but it is write-combined, not HOST_CACHED. So the result gets memcpy'd out of uncached memory.

Uploads are unaffected (writing to write-combined memory is fast). Only the device-to-host read collapses, which is why the same buffers show a large upload/download asymmetry.

The staging allocator is precisely the one that requests HOST_CACHED, and it was bypassed in exactly the case where it is needed.

Change

28 lines across 5 files:

• VulkanDevice::is_cached() — mirrors is_coherent()
• VkAllocator::cached — tracked alongside mappable / coherent, set at every site those two are set
• record_download() — requires mappable and cached to take the direct path

No behaviour change on discrete GPUs (blob memory is not mappable, so the staging path was already taken) or on hardware whose mappable blob memory is HOST_CACHED (condition still passes).

Measurements

Radeon 8060S (Strix Halo, RADV GFX1151, 256 GB/s unified LPDDR5X), 128 MB transfer.

A/B built from the same tree, the only difference being the record_download() condition — 3 runs each:

	before	after
download run 1	0.32 GB/s	4.19 GB/s
download run 2	0.32 GB/s	4.11 GB/s
download run 3	0.32 GB/s	4.23 GB/s

0.32 → 4.19 GB/s, 13.1x. For reference on the same machine: plain host memcpy 23.0 GB/s, ncnn upload 10.0 GB/s. The 31x upload/download asymmetry reported in the issue becomes ~2.4x.

Tests

ctest (178 tests, NCNN_VULKAN=ON), run on two clean worktrees — pristine a4d2ea1 and a4d2ea1 + this commit:

build	result
pristine a4d2ea1	177/178 passed
this PR	177/178 passed

Identical failure sets. The single failure is test_binaryop_3 → test_binaryop_atan2_special op_type=10, byte-identical output on both builds — a pre-existing atan2 special-value mismatch on RADV, unrelated to this change.

One question

VkCompute::record_clone(const VkMat& src, Mat& dst, const Option& opt) (command.cpp:649) has the same shape of test:

if (!src.allocator->mappable)
{
    // device to staging ...
}

and then defers a TYPE_post_download memcpy out of the mapped buffer — so on write-combined memory it should hit the identical uncached-read cost. I have not touched it here, since you asked specifically about the record_download() path and I have no benchmark isolating record_clone. Happy to add it to this PR or send it separately, whichever you prefer.

🤖 Generated with Claude Code

[tencent-adm]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98b0f74b0d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Track cached state separately for buffer memory

When a VkBlobAllocator serves both VkMat and VkImageMat and their selected memory types differ, fastMalloc(VkImageMemory) overwrites the same allocator-wide cached flag previously set by fastMalloc(VkBufferMemory). This condition therefore tests the most recently initialized memory kind rather than the buffer memory used for the download; for example, a cached image type can cause an uncached buffer download to bypass staging and retain the slow CPU read this change is intended to prevent. Base this decision on buffer_memory_type_index or maintain separate buffer/image cached flags.

Useful? React with 👍 / 👎.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.11%. Comparing base (dda2e28) to head (98b0f74).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
src/allocator.cpp	45.45%	6 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #6864    +/-   ##
========================================
  Coverage   95.10%   95.11%            
========================================
  Files         947      947            
  Lines      417208   417309   +101     
========================================
+ Hits       396796   396922   +126     
+ Misses      20412    20387    -25     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.