Repository navigation
Conversation
…y satisfied when it is cancelled or deleted
… out The 1 ms timeout can expire and unlink the waiter before the parent polls the waiter count, so WaitForWaiters reached its deadline with a count of 0. The join already orders the timeout result before the later set.
…re the poll The set with CLEAR_PAT clears the bit under the lock, so remaining is always 0 and the waiter's result always holds; the guards and the consumed counter could never fail.
…or delete releases
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed the incremental wake-routing change and its test coverage. The per-waiter condition is protected by the same flag mutex as its outcome, and the waiter remains alive until it has reacquired that mutex and unlinked itself. Set, cancel and delete notify only outcomes they settle; deletion still waits for all waiters to exit.
I built both new event-flag test targets from this PR's exact changed files. Native CTest cannot run them on this host because its CPU guard requires AVX2/FMA; both executables pass under QEMU Haswell. The hosted Windows build passes. I also downloaded both attached traces and summary lines and reran tools/trace_digest.py; the reported capture metadata and digest values match the PR. The same-machine captures show lower wait time, with no claimed frame-rate gain.
This branch includes the five commits from #10806 before the performance commit, and #10806 is still open. I am leaving this as a comment rather than approving the full stacked diff until I have reviewed that prerequisite too. Linux full CI is skipped; the focused local test used Linux with CPU emulation, while the performance measurement is on macOS.
|
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.
|
|
Windows run at this PR's exact head Windows 11 Enterprise 10.0.22621, Ryzen 9 5950X (32 threads), WinLibs GCC 15.2.0 UCRT r7, Ninja, Release, All three pass in each of 10 runs, with no failed, skipped or not-run tests. The runs were serial,
The parallel AI-assisted: yes (Claude Code). |
What
sceKernelSetEventFlagalready marks only the waiters whose pattern it satisfies, but then callsNotifyAllon the flag's one condition, so every thread waiting on the flag wakes, takes the flag's mutex, finds itself still pending and sleeps again. Each waiter that leaves also calledNotifyAll, which onlysceKernelDeleteEventFlagneeds (it waits for the waiter count to reach 0). A flag with many waiters, like a job system's wake-up flag, pays two rounds of wake-ups and mutex contention for every set.Each waiter now sleeps on its own condition:
sceKernelSetEventFlagwakes only the waiters it satisfies;sceKernelCancelEventFlagandsceKernelDeleteEventFlagwake the waiters they release, which is all pending ones;What a wait returns is unchanged. This is built on #10806, whose per-waiter outcome (
Pendinguntil a set, cancel or delete settles it) tells each operation exactly which waiters it released.guest_event_flag_wakeis new. It checks that each set releases only the waiter whose bit it sets while the others keep waiting, that an AND waiter waits for every bit, that a timeout leaves the other waiters waiting, and that cancel and delete release all 8 waiters withECANCELEDandEACCES. A missed wake-up on any of these paths shows up as a timeout.Tested
Apple M4 Pro, macOS 27.0.1, MoltenVK 1.4.2, x86-64 Release under Rosetta 2.
maindoesn't buildlibkernelon macOS by itself, so the tree ismain3fea2a937bwith the 8 open PRs listed on #1 (#7631, #7804, #10552, #11249, #11447, #11554, #10669, #11047) and #10806. Base255c335e9f87is that tree, and the change75772816241eis the base plus this commit (fork branchesbench/evf-pr-baseandbench/evf-pron imellag/AnyPS5).guest_event_flag_wake,guest_event_flag_released_waiter(fix(libkernel): keep the result of an event flag wait that was already satisfied when it is cancelled or deleted #10806's) andguest_kernel_errorspass 10 of 10 runs, and 10 of 10 again with the two event flag tests running in parallel (-j4).Planet of Lana (PPSA19450), village after New Game, mimalloc injected in both arms, same shader cache. Its 13 job threads wait on event flags. Time Profiler (
xctrace), 10 s from second 95:sceKernelWaitEventFlagsceKernelSetEventFlag_pthread_mutex_firstfit_lock_slow/unlock_slowunder the waitFrame rate: unchanged within noise. Alternating 120 s runs, fps from second 85:
One more PR run stayed on the title screen and is left out. Lana's frame is bound by the guest building the next frame's commands after the previous one's end-of-pipe event, so the CPU saved doesn't shorten it. Titles whose event flag waiters are on the critical path may see more.
Summary lines and traces (
APS5_TRACE_SECONDS=95:10 APS5_TRACE_EVENTS=1048576) are attached below.tools/trace_digest.py:{"commit":"255c335e9f87","dirty":false,"env":{"APS5_ASYNC_PIPELINES":"1","APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_EVENTS":"1048576","APS5_TRACE_SECONDS":"95:10","APS5_VIDEOOUT_TELEMETRY_DIR":"<path>"},"gpu":"Apple M4 Pro","driver":"MoltenVK 1.4.2","os":"Darwin 27.0.0","cpu":"VirtualApple @ 2.50GHz","cpu_threads":12,"memory_mib":24576,"title_id":"PPSA19450","title":"Planet of Lana","content_version":"01.010.000","resolution":"3840x2160","window":{"spec":"seconds 95:10","first_present":3653,"ms":10000.398},"presents":304,"frame_ms":{"p50":32.554,"p90":34.574,"p99":40.066,"max":49.286},"gpu_busy_ms_per_present":23.253,"worker_busy_pct":{"queue worker 0x0":59.461},"top_phases_ms_per_frame":{"Driver.Packet":19.286,"Driver.Packet.draw":15.155,"Driver.Draw":15.005,"Graphics.Draw":13.162,"VideoOut.Flip":12.869,"Driver.Present":12.094,"VideoOut.Flip.present":12.061,"Vulkan.Retire":11.988},"trace_dropped":false,"trace":"aps5-trace-255c335e9f87-41986.json.gz"} {"commit":"75772816241e","dirty":false,"env":{"APS5_ASYNC_PIPELINES":"1","APS5_NO_PAD_INPUT":"1","APS5_PAD_INPUT_FILE":"<path>","APS5_TRACE_DIR":"<path>","APS5_TRACE_EVENTS":"1048576","APS5_TRACE_SECONDS":"95:10","APS5_VIDEOOUT_TELEMETRY_DIR":"<path>"},"gpu":"Apple M4 Pro","driver":"MoltenVK 1.4.2","os":"Darwin 27.0.0","cpu":"VirtualApple @ 2.50GHz","cpu_threads":12,"memory_mib":24576,"title_id":"PPSA19450","title":"Planet of Lana","content_version":"01.010.000","resolution":"3840x2160","window":{"spec":"seconds 95:10","first_present":3666,"ms":10018.677},"presents":308,"frame_ms":{"p50":32.274,"p90":33.877,"p99":39.434,"max":54.684},"gpu_busy_ms_per_present":21.134,"worker_busy_pct":{"queue worker 0x0":59.993},"top_phases_ms_per_frame":{"Driver.Packet":19.238,"Driver.Packet.draw":15.265,"Driver.Draw":15.111,"Graphics.Draw":13.297,"VideoOut.Flip":12.612,"Driver.Present":11.846,"VideoOut.Flip.present":11.814,"Vulkan.Retire":11.743},"trace_dropped":false,"trace":"aps5-trace-75772816241e-48919.json.gz"}Not run on Linux or Windows. The change uses the same
TimedWait::Conditionas before, only one per waiter.tools/check_conventions.py --base upstream/mainpasses.tools/pr_overlap.pyfinds conflicting lines only with #10806, which this builds on. #8285 touchesEventFlag.cpponly in its include list.aps5-summary-base-lana.txt
aps5-summary-pr-lana.txt
pr-evf-trace-base-lana.json.gz
pr-evf-trace-pr-lana.json.gz
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)