fix(vk): the readback line also counts the waits for a whole submission whose pages were written in place #700

Open

mooki wants to merge 1 commit from mooki/Longshot:fix/readback-whole-waits into main

pull from: mooki/Longshot:fix/readback-whole-waits

merge into: MagicBeanDev:main

MagicBeanDev:main

Conversation 0 Commits 1 Files changed 2 +13 -4

mooki commented 2026-11-15 05:23:45 +00:00

Member

• Copy link
• Copy Markdown

Type: fix · Risk: low · Tested on: macOS 26.7.1, Apple M4 Max (MoltenVK 1.4.2), arm64 and x86_64 under Rosetta (game-free tests only) · Needs: Windows and Linux, any GPU (build and start); macOS (a start, on the macOS stack)

Why

Every 3000 readbacks of GPU results the renderer logs how many waited and, since the part waits of issue 997, how many waited for
their submission's last part because guest memory is read in place after their part. A readback whose pages were written in place
(no mirror, so no part) waits for its whole submission too, but the line counted none of those. On a Mac, where every pool is written
in place, it read 0 although every readback there waits for its whole submission (reports/2026-11-14-mac-perf.md). This changes a
default log line on every platform, so it is its own PR, split from the macOS performance lines (#704). It is part of the
macOS port's work. See #420.

How to reproduce

Seen on: macOS 26.7.1, Apple M4 Max, MoltenVK 1.4.2 (any device whose pools have no mirror)
Settings and switches: defaults
Steps: play until the vk: readbacks of GPU results: line prints
Expected: the line says how many readbacks waited for their whole submission
Actual: it counts only the held-back waits; the whole-submission waits of pages written in place are not on it

What changed

The readback line counts one more kind of wait: a readback of pages written in place that waited for its whole submission. Nothing
else changes.

• gpu/vk/vk_replay.cpp invalidate(): a wait for a range whose pages were written in place (part 0, no mirror) adds one to
  in_place (a relaxed atomic); the line now ends …, K waited for their whole submission (their pages written in place, no mirror),
  read and reset with the others. Nothing waited for, recorded or drawn changes, on any driver.
• tests/test_readback_part.py: the model of invalidate() counts it (a range written in place counted once; a range with a
  mirror's page, and a range in a part of its own, not) and the line's source is checked.

Measurements

Verdict: nothing to measure: one counter on a log line; what is waited for is unchanged (the model's 40 checks pass).

Flags and settings

None.

Testing

Tested: on macOS 26.7.1, Apple M4 Max, at 203b8b7c, this PR's head before the rebases onto e090cc48 and aa95d35f
(main 6c9edffe plus this commit):

• tests/test_readback_part.py with /usr/bin/clang++ and clang++ -arch x86_64 (Rosetta): pass; with the test copied onto main it
  fails (the readback line no longer counts the waits for a whole submission whose pages were written in place)
  (reports/PubWave-files/PubLate4/proof-01/rb-readback-*, base-rb-readback-*).
• gpu/vk/vk_replay.cpp compiled whole for arm64 and x86_64 with main's renderer flags (#476's, #492's and #490's macOS compile
  pieces merged in a scratch tree only): 2 objects, exit 0 (reports/PubWave-files/PubLate4/compile/readback-whole-waits/).
• Off Apple: vk_replay.cpp differs (intended: the count is every platform's)
  (reports/PubWave-files/PubLate4/offapple-readback-whole-waits.txt).
• The wave's full suite, on a merge of this PR with the late macOS renderer PRs: see the run_all box.
• After the rebase onto main aa95d35f, at 4d243bc0 (203b8b7c before the rebases onto e090cc48 and aa95d35f):
  Sweep3's checks of this head (the C/C++ it changes compiled, and its own tests run, on arm64 and x86_64 where they
  apply) gave the same results as before; python -I -B tests/run_all.py on wave C6's merge a10d0c4b, which carries
  this head: 584/33/18 (passed/failed/skipped) against main aa95d35f's 543/33/17 on this Mac, one new failure,
  test_gpu_hang_watch, which passes run alone on the same merge on arm64 and x86_64 (its 2 s bound under the suite's
  load) (reports/2026-11-15-rebase-sweep-3.md).

Not tested: a game run (main does not start on macOS by itself); not built on Windows or Linux; its test request covers it.

Must be tested:

• Windows and Linux, any GPU: build this PR (head 4d243bc0, on main) and main aa95d35f the same way, play to the
  field, expecting the vk: readbacks of GPU results: line with the new waited for their whole submission count at
  the end and every other figure as main's; on a device whose pools are mirrored the new count is small or 0 (by hand:
  test request T-LF3Z).

Risks

What could break: a tool that parses the readback line by its end would see one more field.

Turning it off: revert the PR (no switch).

Compatibility: A log line gains a field on every platform. No file in the shader stamp changed; no MKWVK_API_VERSION change.

Reviewer notes

• One commit on main 6c9edffe; written for this PR from the macOS topic macos/perf-instr 2c20c030, whose in_place count and
  line it is, with a positive check added to the test (the topic's test only checked a range not counted).
• #704, the macOS performance lines, touches the same two places (its L5 wait kinds beside this line in invalidate();
  test_readback_part.py's stand-ins): whichever lands second keeps both lines (in the wave merge 39da60e1, this count first, then
  L5's).
• Conflicts with main: none.

Issues

See #420

Checklist

Everyone

• One topic.
• The title is a Conventional Commit (type(scope): description) of the status line's type.
• Why gives the reason plainly and links the issues that fit (the tracker searched).
• Measurements has a verdict, or one line saying why there is nothing to measure.
• Testing names what was tested, what wasn't, and what must still be tested, where and how. (Test request T-LF3Z.)
• Rebased on current main: on aa95d35f (#761, 2026-11-15), head 4d243bc0, without a conflict; this PR's own
  commits, old → new: 203b8b7c → 4d243bc0; the commit names elsewhere in this description are from before the
  rebase.
• python -I -B tests/run_all.py passes (any failure that also fails on main is named). (On the wave's merge 39da60e1 = the nine late renderer PRs' heads (fan-key 592cd8d3, fan-indirect fa6dd43a, provoking-first b8194380, perf-instr ea90d3ca, readback-whole-waits 203b8b7c, fix/indirect-quads c6e0c37f, indirect-quads 978079d4, fix/mesh-count 0a61a9e7, mesh-provoking 56c09f1b) with their sibling fixes: 570 passed, 31 failed, 18 skipped against main 6c9edffe's 531/32/17 on this Mac; the 31 failures are main's own and nothing that passes on main fails; test_collect_wide now passes; 38 new tests pass; reports/PubWave-files/PubLate4/wave4/full-39da60e1/. After the rebase onto main aa95d35f (head 4d243bc0): wave C6's merge a10d0c4b, 584/33/18 against main aa95d35f's 543/33/17, one new failure, test_gpu_hang_watch, which passes run alone on the same merge on arm64 and x86_64 (its 2 s bound under the suite's load).)
• No game files, dumps, build output or generated files.
• third_party/ untouched (a new folder with a PROVENANCE.md instead).
• The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

• AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
• A game-free test added for the fix, failing on main and passing here.
• A check under tests/ that reads changed source updated in this PR, not weakened.

**Type:** fix · **Risk:** low · **Tested on:** macOS 26.7.1, Apple M4 Max (MoltenVK 1.4.2), arm64 and x86_64 under Rosetta (game-free tests only) · **Needs:** Windows and Linux, any GPU (build and start); macOS (a start, on the macOS stack) ## Why Every 3000 readbacks of GPU results the renderer logs how many waited and, since the part waits of issue 997, how many waited for their submission's last part because guest memory is read in place after their part. A readback whose pages were written in place (no mirror, so no part) waits for its whole submission too, but the line counted none of those. On a Mac, where every pool is written in place, it read 0 although every readback there waits for its whole submission (`reports/2026-11-14-mac-perf.md`). This changes a default log line on every platform, so it is its own PR, split from the macOS performance lines (#704). It is part of the macOS port's work. See #420. ## How to reproduce **Seen on:** macOS 26.7.1, Apple M4 Max, MoltenVK 1.4.2 (any device whose pools have no mirror) **Settings and switches:** defaults **Steps:** play until the `vk: readbacks of GPU results:` line prints **Expected:** the line says how many readbacks waited for their whole submission **Actual:** it counts only the held-back waits; the whole-submission waits of pages written in place are not on it ## What changed The readback line counts one more kind of wait: a readback of pages written in place that waited for its whole submission. Nothing else changes. - `gpu/vk/vk_replay.cpp` `invalidate()`: a wait for a range whose pages were written in place (part 0, no mirror) adds one to `in_place` (a relaxed atomic); the line now ends `…, K waited for their whole submission (their pages written in place, no mirror)`, read and reset with the others. Nothing waited for, recorded or drawn changes, on any driver. - `tests/test_readback_part.py`: the model of `invalidate()` counts it (a range written in place counted once; a range with a mirror's page, and a range in a part of its own, not) and the line's source is checked. ## Measurements **Verdict:** nothing to measure: one counter on a log line; what is waited for is unchanged (the model's 40 checks pass). ### Flags and settings None. ## Testing **Tested:** on macOS 26.7.1, Apple M4 Max, at `203b8b7c`, this PR's head before the rebases onto e090cc48 and aa95d35f (main 6c9edffe plus this commit): - `tests/test_readback_part.py` with `/usr/bin/clang++` and `clang++ -arch x86_64` (Rosetta): pass; with the test copied onto main it fails (`the readback line no longer counts the waits for a whole submission whose pages were written in place`) (`reports/PubWave-files/PubLate4/proof-01/rb-readback-*`, `base-rb-readback-*`). - `gpu/vk/vk_replay.cpp` compiled whole for arm64 and x86_64 with main's renderer flags (#476's, #492's and #490's macOS compile pieces merged in a scratch tree only): 2 objects, exit 0 (`reports/PubWave-files/PubLate4/compile/readback-whole-waits/`). - Off Apple: `vk_replay.cpp` differs (intended: the count is every platform's) (`reports/PubWave-files/PubLate4/offapple-readback-whole-waits.txt`). - The wave's full suite, on a merge of this PR with the late macOS renderer PRs: see the run_all box. - **After the rebase onto main aa95d35f**, at `4d243bc0` (`203b8b7c` before the rebases onto e090cc48 and aa95d35f): Sweep3's checks of this head (the C/C++ it changes compiled, and its own tests run, on arm64 and x86_64 where they apply) gave the same results as before; `python -I -B tests/run_all.py` on wave C6's merge `a10d0c4b`, which carries this head: 584/33/18 (passed/failed/skipped) against main aa95d35f's 543/33/17 on this Mac, one new failure, `test_gpu_hang_watch`, which passes run alone on the same merge on arm64 and x86_64 (its 2 s bound under the suite's load) (`reports/2026-11-15-rebase-sweep-3.md`). **Not tested:** a game run (main does not start on macOS by itself); not built on Windows or Linux; its test request covers it. **Must be tested:** - Windows and Linux, any GPU: build this PR (head `4d243bc0`, on main) and main aa95d35f the same way, play to the field, expecting the `vk: readbacks of GPU results:` line with the new `waited for their whole submission` count at the end and every other figure as main's; on a device whose pools are mirrored the new count is small or 0 (by hand: test request T-LF3Z). ## Risks **What could break:** a tool that parses the readback line by its end would see one more field. **Turning it off:** revert the PR (no switch). **Compatibility:** A log line gains a field on every platform. No file in the shader stamp changed; no `MKWVK_API_VERSION` change. ## Reviewer notes - One commit on main 6c9edffe; written for this PR from the macOS topic `macos/perf-instr` `2c20c030`, whose `in_place` count and line it is, with a positive check added to the test (the topic's test only checked a range not counted). - #704, the macOS performance lines, touches the same two places (its L5 wait kinds beside this line in `invalidate()`; `test_readback_part.py`'s stand-ins): whichever lands second keeps both lines (in the wave merge `39da60e1`, this count first, then L5's). - Conflicts with main: none. ## Issues See #420 ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. (Test request T-LF3Z.) - [x] Rebased on current `main`: on aa95d35f (#761, 2026-11-15), head `4d243bc0`, without a conflict; this PR's own commits, old → new: `203b8b7c` → `4d243bc0`; the commit names elsewhere in this description are from before the rebase. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). (On the wave's merge `39da60e1` = the nine late renderer PRs' heads (fan-key 592cd8d3, fan-indirect fa6dd43a, provoking-first b8194380, perf-instr ea90d3ca, readback-whole-waits 203b8b7c, fix/indirect-quads c6e0c37f, indirect-quads 978079d4, fix/mesh-count 0a61a9e7, mesh-provoking 56c09f1b) with their sibling fixes: 570 passed, 31 failed, 18 skipped against main 6c9edffe's 531/32/17 on this Mac; the 31 failures are main's own and nothing that passes on main fails; `test_collect_wide` now passes; 38 new tests pass; `reports/PubWave-files/PubLate4/wave4/full-39da60e1/`. After the rebase onto main aa95d35f (head `4d243bc0`): wave C6's merge `a10d0c4b`, 584/33/18 against main aa95d35f's 543/33/17, one new failure, `test_gpu_hang_watch`, which passes run alone on the same merge on arm64 and x86_64 (its 2 s bound under the suite's load).) - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened.

mooki added 1 commit 2026-11-15 05:23:45 +00:00

fix(vk): the readback line also counts the waits for a whole submission whose pages were written in place ... 203b8b7c26

Every 3000 readbacks of GPU results, invalidate() logs how many waited and, since issue 997's part waits, how many
waited for their submission's last part because guest memory is read in place after their part (held back). A
readback whose pages were written in place (no mirror, so no part: part 0) waits for its whole submission too, but
the line counted none of those: on a Mac, where every pool is written in place, it read 0 while every readback
there waits for its whole submission (MacPerf, reports/2026-11-14-mac-perf.md on the Mac work area).

The line now ends "K waited for their whole submission (their pages written in place, no mirror)": one relaxed
atomic add per such wait, read and reset with the others. Nothing waited for, recorded or drawn changes, on any
driver.

Test: test_readback_part's model of invalidate() counts it (a range written in place counted once, a range with a
mirror's page and a range in a part of its own not) and checks the line's source.

(Split from the Mac's PerfInstr lines, 2c20c030, which joined this count to the line: it prints on every driver by
default, so it is its own PR on main.)

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 <[email protected]>

mooki referenced this pull request 2026-11-15 05:24:57 +00:00

WIP: feat(vk): log-only frame cost lines, a GPU timeline and the bench's A/B exactness arm, all off by default #704

mooki referenced this pull request 2026-11-15 06:11:07 +00:00

Idea: a native macOS build (Apple silicon, x86-64 under Rosetta) with Vulkan on Metal through MoltenVK #420

mooki force-pushed fix/readback-whole-waits from 203b8b7c26 to 65bad6b7f5 2026-11-15 07:14:34 +00:00 Compare

mooki changed title from WIP: fix(vk): the readback line also counts the waits for a whole submission whose pages were written in place to fix(vk): the readback line also counts the waits for a whole submission whose pages were written in place 2026-11-15 11:46:10 +00:00

mooki added the

area/renderer

kind

bug

platform/linux

platform/macos

platform/windows

priority

high

labels 2026-11-15 13:58:40 +00:00

mooki force-pushed fix/readback-whole-waits from 65bad6b7f5 to 4d243bc092 2026-11-15 15:05:05 +00:00 Compare

Some checks failed

Hide checks

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped

Details

Third-party parts / The same as upstream (pull_request) Has been skipped

Details

Website / Website on Cloudflare (pull_request) Has been skipped

Details

CI / Conventional Commit title (pull_request) Successful in 4s

Details

CI / Workflow lint (actionlint) (pull_request) Successful in 7s

Details

Guide screenshots / Guide images and local UI source review (pull_request) Failing after 5s

Details

Third-party parts / As their notes say (pull_request) Successful in 9s

Details

PR description / Description follows the template (pull_request_target) Successful in 5s

Details

This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u 'fix/readback-whole-waits:mooki-fix/readback-whole-waits'

git switch 'mooki-fix/readback-whole-waits'

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area/accessibility

Barriers for people with disabilities

  

area/audio

Sound and music

  

area/input

Controllers, keyboard, mouse

  

area/install

Installing, updating, building from your game

  

area/menu

In-game menu and settings

  

area/renderer

Vulkan renderer, shaders, effects

  

area/saves

Save files

  

area/upscaling

DLSS, FSR, frame generation

  

beanstalk: approved @ 1697603

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 2c5a62e

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 4fd82b7

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 64d6d09

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 6f91fae

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 70a78f9

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 8d361dc

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 9d8356e

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ abf2227

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ d82f202

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ e5f1e40

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ ee8ab5f

Set by Beanstalk for show; the farm's database is what counts

  

ci: approved

A maintainer approved CircleCI for this fork PR (sticky; removed after 14 quiet days)

  

good first issue

A good start for a new contributor

  

gpu/amd

AMD GPUs

  

gpu/igpu

Integrated GPUs

  

gpu/intel

Intel GPUs

  

gpu/nvidia

NVIDIA GPUs

  

help wanted

Extra hands or knowledge wanted

  

kind

bug

Something doesn't work as it should

  

kind

chore

Build, CI, dependencies, tidying

  

kind

docs

Documentation

  

kind

feature

A new feature or an improvement

  

kind

performance

Frame rate, frame time, loading, memory

  

kind

question

A question rather than a change

  

needs measurements

Waiting for before/after measurements

  

platform/linux

Affects Linux (a change for every system carries both Windows and Linux)

  

platform/macos

macOS (Apple silicon or Intel Macs)

  

platform/steam-deck

Steam Deck

  

platform/windows

Affects Windows (a change for every system carries both Windows and Linux)

  

preview

Maintainers: deploy a preview of this PR's website (read its changes first). Removed after the run.

  

priority

critical

Crashes, data loss, or blocks most players

  

priority

high

Important: next in line

  

priority

low

Nice to have

  

regression

Worked in an earlier build, broken in a newer one: highest priority, above other bugs

  

status

blocked

Waits on something else first

  

status

confirmed

Reproduced or agreed on

  

status

duplicate

Already reported or proposed

  

status

invalid

Not a problem in this project

  

status

needs-info

Waiting for more information from the reporter

  

status

wontfix

Will not be worked on

  

testing wanted

Wanted: a test on other hardware (posts a test request in #test-requests)

  

ci: approved

No labels

area/accessibility

area/audio

area/input

area/install

area/menu

area/renderer

area/saves

area/upscaling

beanstalk: approved @ 1697603

beanstalk: approved @ 2c5a62e

beanstalk: approved @ 4fd82b7

beanstalk: approved @ 64d6d09

beanstalk: approved @ 6f91fae

beanstalk: approved @ 70a78f9

beanstalk: approved @ 8d361dc

beanstalk: approved @ 9d8356e

beanstalk: approved @ abf2227

beanstalk: approved @ d82f202

beanstalk: approved @ e5f1e40

beanstalk: approved @ ee8ab5f

ci: approved

good first issue

gpu/amd

gpu/igpu

gpu/intel

gpu/nvidia

help wanted

kind

bug

kind

chore

kind

docs

kind

feature

kind

performance

kind

question

needs measurements

platform/linux

platform/macos

platform/steam-deck

platform/windows

preview

priority

critical

priority

high

priority

low

regression

status

blocked

status

confirmed

status

duplicate

status

invalid

status

needs-info

status

wontfix

testing wanted

ci: approved

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

MagicBeanDev/Longshot!700

No description provided.