Repository navigation
fix(tests): build the frame limit failure messages only when a check fails - #14995
Conversation
…fails Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (1, not counted against this PR)Validation IDs already present on main (5 IDs in 346 tests)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
BunnyFl
left a comment
There was a problem hiding this comment.
Looks right to me. The std::invocable overload only matches the three lambdas. The existing calls pass a std::string or a literal, which isn't invocable, so they still pick the original overload, and the messages and conditions are unchanged. pacer.Admit(now) in the slow-game loop is still evaluated once, in the condition.
Tested on Windows 11, WinLibs GCC 15.2.0, Release (no sanitizer), with head bf74e82641 merged onto main: agc_frame_limit passes in 0.15 s, against 1.46 s with main's FrameLimit.cpp. Even without ASan the eager message building dominates, which supports your Sanitizers timeout analysis. I didn't run the ASan/UBSan build itself.
|
Windows + NVIDIA result for head Host: Windows 11 26300, Intel i7-12700K, NVIDIA GeForce RTX 2070 SUPER, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, Same machine, both trees, with
No new validation errors. AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine. |
What
agc_frame_limittimed out in the nightly Sanitizers run on 2026-11-02 (run 41384606155,main926f7dd492):***Timeout 30.06 sec, and the run's summary lists no sanitizer report for it. The same test passed in 1.07 s on Linux in the normal build of4569fe654a(run 41395467000).The test spends its time building failure messages that are never shown.
require(condition, reason)takes the message as astd::string, so every call builds it before the check, including the calls inside the loops: 8 limits × 1,000,000 frames in the deadline test, 100,000 frames in the overshoot test and 3 × 10,000 in the slow-game test, each message made of severalstd::to_stringcalls and concatenations. Under AddressSanitizer each of those allocations is instrumented.This adds a
requireoverload that takes a callable and builds the message only when the check fails, and uses it for the three checks inside those loops. The checks themselves, the messages and theTIMEOUT 30are unchanged.Changed:
core/libs/prx/libSceAgcDriver/tests/FrameLimit.cpponly. No product code, no CMake change.Rechecked on 2026-11-02 at 08:54 UTC: this head merges with
main88e804b0a7without a conflict, and none of the 11 pull requests opened since 07:30 UTC (#14979 to #14993) changesFrameLimit.cpp.Tested
The test executable built on its own with the flags the Sanitizers workflow gives it (
g++ 13.3.0,-O3 -DNDEBUG -std=c++20 -g1 -fsanitize=undefined -fno-sanitize=vptr -fsanitize=address -fno-omit-frame-pointer -DANYPS5_ENABLE_ASAN=1) and run with the workflow'sASAN_OPTIONS,UBSAN_OPTIONSandAPS5_SKIP_TIMING_TESTS, three runs each, on 2026-11-02 between 07:26 UTC and 07:31 UTC:main4569fe654a)All runs print
frame limit tests passed. These local runs stay under the nightly's 30 s, so they do not reproduce the timeout; the CI runner, which runs four tests at once (ctest --parallel 4), was not measured. What they show is that building the messages is where the time goes: the change makes the sanitizer run 60 to 220 times faster (12.05 s / 0.20 s to 17.63 s / 0.08 s).The test still fails when the pacer is wrong. With the extra nanosecond that
FramePacer::step()adds for the carried remainder removed (++length), it fails both before and after this change with the same message,frame 3 at limit 30000 is presented at 99999999 instead of 100000000, now built by the lazy overload. Withnextset from the current time plus the wait instead of from the schedule, it fails both before and after in the late-frame test. Both mutations were made on copies;FrameLimit.hppis unchanged.tools/check_conventions.py --base upstream/mainpasses.Overlap
All 1914 open PRs were checked on 2026-11-02 at 07:32 UTC against
main4569fe654aforFrameLimit.cpp,FrameLimit.hppandagc_frame_limit_tests.cmake. Four list these files (#10351, #9588, #8630, #7694), each only through a merge ofmain: none of them changes the files againstmain, and none conflicts with this change. No open PR changes the test; #11566, which comes up in a search foragc_frame_limit, changes onlytests/Graphics.cppandtests/cmake/agc_driver_graphics_tests.cmake. It merges cleanly intomain88e804b0a7(2026-11-02 08:07 UTC), which does not change these files or the Sanitizers workflow.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)AI-assisted (Claude Code).
🤖 Generated with Claude Code