Skip to content

perf(shader): keep 64 Mi words of image table resolutions in the reuse memo - #8566

Closed
tv7 wants to merge 1 commit into
boykopovar:mainfrom
tv7:perf/table-resolution-memo-budget
Closed

tv7 wants to merge 1 commit into
boykopovar:mainfrom
tv7:perf/table-resolution-memo-budget

Conversation

@tv7

@tv7 tv7 commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

What

ResolveImageTablesShared (077bc20) hands out an earlier resolution of a variant's image tables when the columns match. It keeps at most 4 per variant and 256 in all within ResolutionMemoWords = 8 Mi words (32 MiB), counting the columns the entries keep alive.

A title with large bindless image tables has more live resolutions than fit in 32 MiB, so they push each other out: the next result memo miss of a variant finds no entry for it and resolves its tables again, although its columns did not change.

This raises ResolutionMemoWords to 64 Mi words (256 MiB). The 256-entry and 4-per-variant bounds stay, so the memo still holds at most 256 resolutions. docs/dev/ARCHITECTURE.md gets the new number.

Tested

Checklist

  • Based on current main (d0852de); no open PR changes the resolution memo's bounds or eviction
  • One topic per PR
  • No comments in code except technical debt
  • Unimplemented paths throw (no new paths)
  • No notes, investigation .md files or images added
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 3070
  • AI-assisted: yes

🤖 Generated with Claude Code

…e memo

ResolveImageTablesShared keeps resolutions within 8 Mi words (32 MiB),
counting the columns they keep alive. A title with large bindless image
tables holds more than that in live resolutions, so they push each other
out and every result memo miss resolves its tables again: in Marvel's
Wolverine (PPSA03671) about 135 resolutions per 10 s found no entry for
their variant, ~12 ms each. Its live set settles at ~240 resolutions of
0.6-0.9 MiB, so the budget becomes 64 Mi words (256 MiB); the 256-entry
and 4-per-variant bounds stay.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added perf Performance change perf numbers missing perf pull request without a before and after summary line labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown

This pull request is labeled perf, so it needs the summary line of a capture on main and one with the change, from the same title, scene and machine, as the Performance report form asks. Please paste both aps5-summary.jsonl lines, attach the two traces and add the output of tools/trace_digest.py.

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 907 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 5371 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 5925 docs/dev/ARCHITECTURE.md
PR 6044 docs/dev/ARCHITECTURE.md
PR 6280 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 6412 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
docs/dev/ARCHITECTURE.md
PR 6488 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 7550 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 7642 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
docs/dev/ARCHITECTURE.md
PR 7684 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
docs/dev/ARCHITECTURE.md
PR 7689 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 7803 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 8077 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 8527 docs/dev/ARCHITECTURE.md
PR 8625 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 8672 docs/dev/ARCHITECTURE.md
PR 8998 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 9020 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 9335 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp
PR 9748 core/shader/recompiler/Optimization/src/ResourceMaterializer.cpp

@github-actions github-actions Bot added shader core/shader docs labels Oct 20, 2026
@adriadam10 adriadam10 self-assigned this Oct 20, 2026

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change itself is small and the ARCHITECTURE.md update matches it. Two things are missing before it can merge:

  • Peak memory. The memo is a single global (resolutionMemo() in ResourceMaterializer.cpp) and this raises what it can keep alive from 32 MiB to 256 MiB of columns for every title and every host. docs/dev/CONVENTIONS and the memory rules ask for the bound (given) plus peak RSS before/after on a large workload. Please add the process RSS peak on main and with the change in the scene you measured, and say what frees the entries on a low-memory host.
  • Performance report. The PR is labeled perf: please paste the aps5-summary.jsonl line and the tools/trace_digest.py output of a capture on main and one with the change, same title, scene and machine, as the bot asked. The local counters are useful context but are not that report.

Once both are in, this looks mergeable.

@tv7

tv7 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Closing this one instead of adding the report.

  • Memory: nothing frees the memo early on a low-memory host. The cap is a fixed bound, so every title could keep up to 256 MiB of columns, as you said.
  • Gain: in the Wolverine scene I measured, the bigger memo cut image table resolution from about 1.65 s to 0.15-0.6 s per 10 s, but the frame rate did not change. The saved time turned into GPU mutex wait, so a main vs PR capture would show the extra memory and no frame time gain. Also, that scene is only reachable with other unmerged changes, not on plain main.

The cost isn't worth it for every title, so I'm dropping it.

@tv7 tv7 closed this Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs perf numbers missing perf pull request without a before and after summary line perf Performance change shader core/shader

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants