Repository navigation
Conversation
…nter queries - On Linux/POSIX systems where QueryPerformanceFrequency is fixed at 1 GHz, fast-path a64_cntpct_host with (now - base) >> 5 instead of performing 128-bit integer multiplications and 32-bit shifts on every invocation. - In hos_process_memory, include <psapi.h> directly on POSIX so K32GetProcessMemoryInfo populates working set and private usage accurately rather than returning zeros due to failing GetProcAddress lookup on kernel32.dll.
Joey-Magnus
left a comment
There was a problem hiding this comment.
I don't think this changes anything at runtime, so I'd suggest closing it.
a64_cntpct_host: on POSIX,QueryPerformanceFrequencyis 1e9, so main'smultis(31250000 << 32) / 1e9= 2^27, and(d * 2^27) >> 32is already exactlyd >> 5. The PR returns the same values. It saves one 64x64 multiply beside aclock_gettimecall that costs far more, and it adds a second copy of the function that silently depends on the 1 GHz frequency.hos_process_memory: the diagnosis is right (GetModuleHandleA("kernel32.dll")is NULL on Linux, so the counters read 0). But its only caller isgpu/render/rx_core.cpp:1477, the WebGPU renderer, andnx2/build.py:886builds that only whennot LINUX. Nothing reads these counters on Linux. The Vulkan back end callsK32GetProcessMemoryInfodirectly (vk_texture.cpp:3826).
If you have a profile showinga64_cntpct_hostis hot, I'd like to see it, but the arithmetic can't be the cost.
ghost
left a comment
There was a problem hiding this comment.
Thanks for looking at this. I checked it against main and agree with @Joey-Magnus's review: neither half changes anything at runtime on Linux.
a64_cntpct_host: the POSIXQueryPerformanceFrequencyshim (host/posix/windows.h) returns 1e9, so main'smultis already exactly 2^27 and(d * 2^27) >> 32 == d >> 5for everyd. The new branch returns identical values, and the multiply it removes sits next to aclock_gettimethat costs far more. It also leaves two copies of the function, one of which silently assumes the shim's 1 GHz frequency. If you have a profile where this shows up, please share it.hos_process_memory: right diagnosis (theGetProcAddresslookup fails on Linux), but its only caller is the WebGPU renderer ingpu/render/rx_core.cpp, whichnx2/build.pydoesn't build on Linux. The Vulkan back end already callsK32GetProcessMemoryInfofromhost/posix/psapi.hdirectly, so the Linux memory log lines aren't zero today.
No overlap with your other open PRs (#2462, #2467, #2469, #2482, #2487 touch gpu/vk/ and host/posix/, not host/mkw_glue.c). CI hasn't run yet (workflows are waiting for approval), but given the above I'd also suggest closing this one.
|
Thanks @Joey-Magnus and @hckrman101 for checking this against main — agreed that POSIX QueryPerformanceFrequency=1e9 makes (d * 2^27) >> 32 already reduce to d >> 5, and the Vulkan backend queries K32GetProcessMemoryInfo directly rather than going through hos_process_memory. Closing this out! |
Problem
a64_cntpct_host(which emulates CNTPCT_EL0 at 31.25 MHz and is queried frequently across game frames and timing loops) performed a 128-bit integer multiplication and 32-bit shift on every single invocation:(uint64_t)(((unsigned __int128)(now - base) * mult) >> 32)On Linux / POSIX systems,
QueryPerformanceFrequencyis fixed at 1 GHz (1000000000ULL). Because(now - base) >> 5).hos_process_memorydynamically looked upK32GetProcessMemoryInfofromkernel32.dllviaGetProcAddress. On Linux/POSIX,K32GetProcessMemoryInfois an inline function provided in<psapi.h>(reading/proc/self/status), soGetProcAddressalways failed and process memory counters remained zero.Solution
a64_cntpct_hostto(uint64_t)(now.QuadPart - base) >> 5, eliminating 128-bit multiplication overhead.hos_process_memory, include<psapi.h>directly on POSIX to populate resident set and private working set metrics accurately for memory logging and HUD display.Impact
tests/run_all.py(359 passed, 0 failed).