feat(tools): add the APS5_* and ANYPS5_* environment reference and check it

[011010]

What

Add a reference for the APS5_* and ANYPS5_* environment variables and a check that keeps it current, as proposed in discussion #1284.

• docs/dev/ENVIRONMENT.md: every name the code reads through getenv, IsEnvironmentSet, kibSetting, MaskFromEnvironment, EnvInt, configured/PipelineSwitch, environmentCount, GetEnvironmentVariableA, GetEnvironmentVariableW and addWideString (436 today), one table per library, with its kind, the value used when it is unset, the source file and a one-line description.
• tools/env_reference.py: lists the names the code reads with their kind and default, emits the table rows (--table) and compares them with the document (--check).
• tools/check_conventions.py: a new env-reference rule (hard failure, as discussed). It reads the pull request's own diff, so it never executes pull request code and it only checks the names the pull request adds or stops reading: a name the diff adds with no row is reported at the line that reads it, a row whose name the diff stops reading is reported on the document, and a row the diff removes while the head still reads its name is reported on the document.
• tools/tests/test_env_reference.py and tools/tests/test_check_conventions_env_reference.py: unit tests; registered in CMakeLists.txt.

Tested

Linux and macOS. python3 tools/env_reference.py --check reports 436 names, all documented; python3 tools/check_conventions.py --base origin/main --head HEAD passes; the tool's tests pass. Rebased onto current main and rechecked on Windows 11 (Python 3.14): python tools/env_reference.py --check reports 436 names, all documented, python tools/check_conventions.py --base upstream/main passes, and the 41 scripts under tools/tests pass. The descriptions were written from the call sites, and the rows the code does not settle are the ones to check again at review (the APS5_HOST_IMPORT_MIB default and the APS5_DUMP_TARGETS / APS5_NO_RESIDENT_TARGETS interaction among them).

Checklist

• Based on current main; no other open PR implements the same functions
• One topic per PR; follow-ups go in a new PR
• No comments in code except technical debt
• Unimplemented paths throw (NotImplemented_nid_no_patch); no silent stubs added
• No notes, investigation .md files or images added to the repository
• New third-party code is a submodule built from source (none added)
• Generic behaviour, not specific to one title
• Depends on: none
• Unblocks: none
• AI-assisted: yes

[fisherjoey]

Thanks for this. I checked the rows we use most against the code at the PR's base (f785b88). Most of them match. A few need fixing:

1. APS5_DUMP_TARGETS says it "saves the first n renders of every color target". It only dumps targets that are not resident. The copy into binding.dump is on the non-resident write-back path, after the binding.resident != nullptr continue, and only when binding.gpuTiling is set (Draw.cpp:2333-2339). Multisampled and proxied targets are skipped as well. GPU-tiled targets are normally resident, so in practice the dump needs APS5_NO_RESIDENT_TARGETS=1. Any non-zero value also turns off recorded draws, so every draw runs synchronously (Draw.cpp:1995, 2088). Files go to the working directory as target_<address>_<n>.raw (Draw.cpp:2385).

2. The scanner misses three names that BUILD.md:176 already documents: APS5_THREAD_NICE (default -10), APS5_THREAD_RT (default 10) and APS5_THREAD_RTTIME_US (default 200000). ThreadPriority.cpp reads them through its local EnvInt helper (lines 228, 256, 264), so neither env_reference.py nor the env-reference rule sees them. APS5_CRASH_INDIRECT_MEMORY (CrashReport.cpp:335, GetEnvironmentVariableA) is missed for the same reason.

3. main has moved since the base. Seven names read on current main (eea428c) have no row: APS5_NO_DRAW_DATA_HITS, APS5_NO_DRAW_STAGE_DATA_HITS, APS5_SNAPSHOT_CHECK_FATAL, APS5_TRACE_DRAW_CACHE, APS5_VERIFY_BDA_TABLES, APS5_VERIFY_DRAW_DATA_HITS and APS5_VERIFY_DRAW_SNAPSHOTS. The env-reference rule will fail after a rebase until they are added.

Smaller notes:

• APS5_HOST_IMPORT_MIB: your row is closer than what I wrote in the discussion. 6144 is the floor, not a flat default. The cap is the largest host heap minus 2 GiB rather than the host heap itself (GuestBufferMemory.cpp:849-851), and an unset or 0 value falls through to that computed budget.
• APS5_TRACE_GPU is read in six files (DisplayBuffer.cpp:246, PacketExecution.cpp:81 and 259, QueueWorker.cpp:108, Submission.cpp:303, WaitMemory.cpp:116), but the row lists only DisplayBuffer.cpp. It also prints execute lines and waits longer than 200 ms.
• The new env_reference test in CMakeLists.txt has no TIMEOUT, unlike the tests next to it.

These rows match the code: APS5_TEXTURE_CACHE_MIB, APS5_NO_RESIDENT_TARGETS, APS5_THREAD_PRIORITY, APS5_NO_GPU_INDIRECT, APS5_NO_GPU_INDIRECT_DRAW, APS5_COARSE_TIMED_WAITS, APS5_PIPELINE_STATS, ANYPS5_GPU and ANYPS5_SHADER_CACHE_DIR. python3 tools/env_reference.py --check prints "383 name(s), all documented", and tools/check_conventions.py --base origin/main passes on the PR head.

AI-assisted: yes

[011010]

Thanks, all of it is taken:

1. APS5_DUMP_TARGETS now says it dumps the first n renders of each non-resident color target, as target_<address>_<n>.raw in the working directory, that a non-zero value also runs every draw synchronously, and that in practice it needs APS5_NO_RESIDENT_TARGETS=1.
2. The scanner now recognises EnvInt and GetEnvironmentVariableA, so APS5_THREAD_NICE, APS5_THREAD_RT and APS5_THREAD_RTTIME_US (with their defaults) and APS5_CRASH_INDIRECT_MEMORY have rows, and the tool's tests cover the two new readers.
3. Rebased on current main; the seven names it added have rows.
4. APS5_HOST_IMPORT_MIB now reads 6144 as the floor, with an unset or 0 value using the largest host heap minus 2 GiB.
5. APS5_TRACE_GPU names the other five readers in its description.
6. The new test carries TIMEOUT 20.

python3 tools/env_reference.py --check reports 394 names, all documented, and tools/check_conventions.py --base origin/main passes on the new head.

[github-actions]

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request	Conflicting files (lines on main)	Same exports	Same new files
PR 6268	tools/check_conventions.py L15
PR 6757	tools/check_conventions.py L31

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 674	CMakeLists.txt
PR 1835	CMakeLists.txt
PR 3998	CMakeLists.txt
PR 5021	CMakeLists.txt
PR 5710	CMakeLists.txt
PR 6032	CMakeLists.txt
PR 6044	CMakeLists.txt
PR 6447	tools/check_conventions.py
PR 6879	CMakeLists.txt
PR 6946	CMakeLists.txt
PR 6947	CMakeLists.txt
PR 7201	CMakeLists.txt
PR 7332	CMakeLists.txt

[011010]

Rebased onto current main (66702f1cf) and added the missing rows.

The list is 24 names rather than the 21 in your review: main moved again since and added APS5_NO_RECT_INDIRECT, APS5_NO_RESIDENT_LINEAR and APS5_PLAYGO_UNLISTED_CHUNKS, the last one adding the new ### libScePlayGo section.

python3 tools/env_reference.py --check now reports 418 name(s), all documented with 0 stale, python3 tools/check_conventions.py --base origin/main passes, and the 40 scripts under tools/tests pass, test_env_reference.py among them. I also updated the count in the body from 383 to 418.

Two notes on how the rows are written, since they are the part worth re-checking:

• They are not --table output. The generator reports the reader form it matched, not what the value means, so for APS5_PIPELINED_DRAWS it says presence (it is an integer depth, 8 when unset) and for APS5_SKIP_TIMING_TESTS it says string, taken from the GetEnvironmentVariableA site, even though the Linux site tests it as presence. Both rows carry what the code gives, integer with default 8 and presence respectively; every other row follows the same rule as the existing 394.
• APS5_PIPELINE_DRAIN_FILLS, APS5_PIPELINE_DRAIN_LABELS and APS5_PIPELINE_DRAIN_SUBMISSIONS are gated by getenv(...) == nullptr, so any value, 0 included, turns the ordering off. APS5_EXACT_DRAW_WRITES and APS5_LOG_STATS use the other form, strcmp(text, "0") != 0, and their descriptions carry the 0 escape. The rows say which form each name uses.

On the overlap bot: #6757 adds its debt-length rule at the same line of the RULES table in tools/check_conventions.py, so whichever of the two lands second needs a one-line rebase. There is no dependency in either direction, so Depends on stays none.

[oneandonlydean]

Picking this up while @adriadam10 is away. The 21 names from his review are all documented now, thanks. Main has added 7 more since your rebase (tools/env_reference.py --check on the merge with current main): ANYPS5_REQUIRE_ROBUST2, APS5_DISPMOVE_MAX_KIB, APS5_HANDOFF_SPIN_US, APS5_NO_ADDRESS_READ_PASSES, APS5_NO_DCC_PRESYNC, APS5_NO_DISPATCH_MOVE, APS5_TRACE_UDS. Please rebase and add their rows, written from the reader at each site as you did for the others.

[011010]

@oneandonlydean thanks for the pass. Rebased onto current main (ccb0c0926) and added the rows.

The list is 10 names rather than the 7 in your message: main moved again since, and tools/env_reference.py --check on the merge with current main also reports APS5_NO_DEFERRED_SETTLES (GuestBufferMemory.cpp:809, Windows only), APS5_NO_SRT_TAPE and APS5_VERIFY_SRT_TAPE (SrtWalker/SrtTape.cpp:287 and :292). All ten are documented, written from the reader at each site: ANYPS5_REQUIRE_ROBUST2, APS5_DISPMOVE_MAX_KIB, APS5_HANDOFF_SPIN_US, APS5_NO_ADDRESS_READ_PASSES, APS5_NO_DCC_PRESYNC, APS5_NO_DEFERRED_SETTLES, APS5_NO_DISPATCH_MOVE, APS5_NO_SRT_TAPE, APS5_TRACE_UDS, APS5_VERIFY_SRT_TAPE.

Two rows are the reader form the scanner matches, not what the value means:

• APS5_DISPMOVE_MAX_KIB is an integer with a default of 1024 (text != nullptr ? std::strtoull(text, nullptr, 10) : 1024ull, times 1024), but the scanner reports presence/1024 because the text != nullptr test matches its null test before the integer test. The row carries what the code gives, as with APS5_PIPELINED_DRAWS last round.
• APS5_HANDOFF_SPIN_US is an integer clamped to 0..10000, and an unset value picks HandoffSpinWindowFor's automatic window: 150 us when the process has more than 8 usable processors, otherwise none. The scanner reports string, and the row's description names the automatic rule instead of a default.

Two notes on placement:

• APS5_TRACE_UDS is read in libSceNpUniversalDataSystem/src/Event.cpp as well as libSceNpTrophy2/src/TitleTrophies.cpp, so the description names both and the row sits under the new ### libSceNpTrophy2 section (the first site by path).
• APS5_NO_ADDRESS_READ_PASSES is also read by tests/execution/AddressPasses.cpp:274, which is the pass-split case of the address-passes test; the row describes the driver effect.

I also fixed the document's opening line, which still named only getenv, IsEnvironmentSet, kibSetting and MaskFromEnvironment after the last push taught the scanner EnvInt and GetEnvironmentVariableA.

Verified at this head: python tools/env_reference.py --check reports 428 names, all documented, 0 stale; python tools/check_conventions.py --base upstream/main reports All conventions checks passed; the 40 scripts under tools/tests pass (Windows 11, Python 3.14).

AI-assisted: yes

[oneandonlydean]

Thanks for adding the rows, the check now passes on current main (428 names). One gap before this goes in:

The scanner doesn't see names read through helper functions, and the stale rule then rejects their rows. On the merged tree these are read but have no row:

• APS5_PIPELINE_JOURNAL_ENTRIES, APS5_PIPELINE_JOURNAL_MAX_MIB (through configured(name, fallback) in PipelineJournal.cpp)
• APS5_PIPELINE_STORE_ENTRIES (through PipelineSwitch)
• APS5_PM4_RING, APS5_STALL_WATCHDOG_SECS (through environmentCount in SubmissionRing.cpp)
• ANYPS5_LOADEXEC_PARENT (a u"..." string the Windows entry stub reads with GetEnvironmentVariableW)

Adding a row for any of them fails --check with "has a row but the code no longer reads it", so once the env-reference rule is live nobody can document them. Could you teach both scanners those three helpers and the entry stub's wide-string form (addWideString(u"...")), add the six rows, and add a fixture line for each new form in test_env_reference.py?

Optional, but it would stop the two scanners drifting apart: have check_conventions.py import the reader pattern from env_reference.py instead of keeping its own copy.

[oneandonlydean]

One more suggestion: instead of skipping heads without ENVIRONMENT.md, could the conventions rule check only the env names a PR's own diff adds? Then PRs branched before this one can't land undocumented reads, and a later PR doesn't fail for names it didn't touch.

[011010]

@oneandonlydean thanks, I applied both points.

Helpers and wide strings (bb109df67, 47710d801):

• env_reference.py and check_conventions.py now share one table of reader forms. check_conventions imports env_reference.GREP, line_reads and documented_pattern instead of keeping its own copy.
• New forms: configured(...) and PipelineSwitch(...), environmentCount(...), addWideString(u"...") and GetEnvironmentVariableW(L"...").
• That makes 8 rows, not 6. Besides your list, APS5_PIPELINE_JOURNAL_THREADS (PipelineJournal.cpp:1136) uses the same configured form, and ANYPS5_LOADEXEC_ARGV_SKIP is read with GetEnvironmentVariableW(L"...") in libkernel/System/src/Process.cpp:90. Both have rows now.
• Checked: the scanner output before and after adds only those 8 entries. No existing name changed kind or default.
• test_env_reference.py has one fixture line per new form, plus a negative case (reconfigured(...) must not match). env_reference.py --check reports 436 names, all documented.

Diff-based rule (1dcd1ee4a):

• env-reference now reads git diff base head -- core. A name the pull request adds to the code without a row in ENVIRONMENT.md is reported at the file and line where it is read. If the head has no ENVIRONMENT.md, the rule reports it instead of skipping.
• A row is reported only when the pull request stops reading its name and nothing else in the head reads it.
• Names the pull request does not touch are not reported, so earlier branches and unrelated names do not fail.
• New tests in tools/tests/test_check_conventions_env_reference.py cover each case. All 41 scripts under tools/tests pass, and check_conventions.py --base upstream/main passes.

One trade-off: with this model, CI no longer catches an undocumented name that reached main through a path without a row. I suggest running the full env_reference.py --check on a schedule against main, so drift shows up even when no pull request touches it. Would that work for you?

[oneandonlydean]

Thanks, the diff-based rule matches Dean's suggestion and the earlier checks still pass on current main. Two small things: (1) a PR that deletes a row in docs/dev/ENVIRONMENT.md for a name core still reads isn't reported, because the diff only covers core. Could the rule also diff the document and report removed rows whose names the head still reads? (2) tools/tests/test_check_conventions_env_reference.py needs an add_test next to env_reference in CMakeLists.txt, otherwise CI never runs it. Nit: startswith("+++")/("---") also matches added lines starting with ++ and removed lines starting with --, which shifts the reported line numbers. Tracking whether you're inside a hunk (after @@) avoids that.

[011010]

@oneandonlydean both done, plus the nit.

Removed rows (061101a3e): env-reference now diffs docs/dev/ENVIRONMENT.md as well. A row the pull request removes whose name the head still reads is reported on the document, and it stays quiet when the pull request removes the row together with its last read. Tests: test_a_row_the_pull_request_removes_is_reported_while_the_code_still_reads_it and test_a_removed_row_is_kept_quiet_when_the_code_stops_reading_it_too.

Registration (99e295a7b): add_test(NAME conventions_env_reference ...) sits next to env_reference, with TIMEOUT 60 because it builds temporary repositories. Thanks for catching it.

Hunk tracking (061101a3e): the loop now opens a hunk on @@ and only reads +++ b/ as a file header outside one, so an added line whose content starts with ++ no longer skips the counter and a removed line starting with -- no longer loses its read. Tests: test_an_added_line_starting_with_plus_does_not_shift_the_reported_line and test_a_removed_line_starting_with_minus_still_counts_as_a_removed_read.

python3 tools/check_conventions.py --base origin/main --head HEAD passes, python3 tools/env_reference.py --check reports 436 names all documented, test_check_conventions_env_reference.py has 9 tests, and the 41 scripts under tools/tests pass.

The offer from my previous comment still stands: I can run the full env_reference.py --check on a schedule against main, so drift no pull request touches still shows up. Say the word and I will add it.

The requested changes are done and re-reviewed on the current head.