Repository navigation
fix(agc): read the process's private memory from task_info on macOS - #9971
Conversation
|
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.
|
jotitadev
left a comment
There was a problem hiding this comment.
Second Mac, different chip and OS build from the M4 run in the description: red→green reproduces here, and phys_footprint matches what footprint(1) reports for a translated process.
Machine: Apple M2 Max (30-core GPU), 32 GB, macOS 27.2, Apple clang 21, x86-64 build under Rosetta 2, Release, BUILD_TESTING=ON.
Base/method: main doesn't link on macOS, so the tree is #8658 (feat/macos-port) @ c5fa39620a, full cmake --build -j4. #8658's ReadProcessPrivateBytes hunk is the same as this PR's at 8f2db802d3 except for the one-line code comment, so the green build is #8658 as is. The red build is the same tree with only the #elif defined(__APPLE__) branch of ReadProcessPrivateBytes removed, so macOS falls through to the /proc/self/status path as on main.
- Red:
agc_performance_overlay_statisticsfails 3/3 withthe process's private memory cannot be read. - Green: passes 3/3.
costReportgives 1.3, 1.7 and 1.4 us per read, in line with the 1.3 us on the M4.
agc_performance_overlay_statistics doesn't create a Vulkan device, so this result doesn't depend on the GPU. I didn't run the Vulkan agc_performance_overlay test in this window.
Metric check. I wanted to confirm that phys_footprint is the right figure under Rosetta, so I wrote a small C probe (not part of the tree) with the same task_info(mach_task_self(), TASK_VM_INFO, ...) call. It reads the value before and after a 256 MiB malloc + memset, then I ran footprint <pid> against it while it slept:
| build | returned count |
delta after touching 256 MiB | final phys_footprint |
footprint |
|---|---|---|---|---|
-arch x86_64 (Rosetta) |
93 (TASK_VM_INFO_COUNT) |
256.5 MiB | 271,455,696 B | 64-bit (translated) Footprint: 259 MB |
| arm64 | 93 | 256.1 MiB | 269,763,112 B | Footprint: 257 MB |
So the value is the one footprint reports, also for a translated process. The kernel fills the whole struct, which includes phys_footprint (it arrived in rev1, TASK_VM_INFO_REV1_COUNT = 38). The info{} zero-init plus the == 0 check would also catch an older kernel that returned a shorter struct, so the error handling looks complete to me.
Diff: <mach/mach.h> is under #elif defined(__APPLE__) next to the Windows includes, and the /proc path is still the #else, so Linux and Windows don't change.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
MarcZX10
left a comment
There was a problem hiding this comment.
The ReadProcessPrivateBytes implementation in this PR is also present in the open macOS-port PR #8658 (same TASK_VM_INFO/phys_footprint hunk; #8658 currently remains open). Your description explains this split and preserves credit, but the two PRs do not record which copy should land first or how the other branch will drop its duplicate. CONTRIBUTING asks authors to agree on shared work and declare dependencies where needed. Please coordinate with #8658’s author/maintainers and record the integration order: either make #9971 the standalone change and remove the duplicate from #8658 when it rebases, or keep this hunk in #8658 and make the split PR dependent on that work. I compared the exact hunk: Linux/Windows paths remain unchanged, current-main sync is clean, and local check_main_sync, conventions and git diff --check pass (the branch is 136 commits behind, but this file has no intervening main edits). The reported two-platform macOS red/green evidence is relevant; this host cannot independently run macOS tests. This request is for avoiding duplicate implementation and clarifying integration, not queue size.
Independent test: Apple M1 Pro, macOS 27.0.1Apple M1 Pro, macOS 27.0.1, Apple clang 21, x86-64 Release build under Rosetta 2,
AI-assisted: yes (Claude Code ran the build and the tests). |
|
@MarcZX10 thanks. Recorded in the description: this lands on its own first, and #8658 drops the same hunk when it rebases. I've asked @iamthehimansh on #8658; if they'd rather keep it there, I'll close this one. AI-assisted: yes |
|
@MarcZX10 an update on the order: @iamthehimansh closed #8658 and is landing it as topic PRs based on AI-assisted: yes |
imellag
left a comment
There was a problem hiding this comment.
Tested head 8f2db802d3 on Apple M4 Pro, macOS 27.0.1, Apple clang 21, x86-64 under Rosetta 2, with MoltenVK 1.4.2 from the Vulkan SDK 1.4.363.0.
libSceAgcDriver doesn't link on main on macOS yet, so I tested on main 3fcb5e4d76 plus the open macOS build PRs and the AGC parts of mugurc's series. On that tree, with ANYPS5_REQUIRE_VULKAN=1:
- Without this PR,
agc_performance_overlay_statisticsfails withthe process's private memory cannot be read. - With it, the test passes 3 runs out of 3.
agc_performance_overlayandagc_performance_overlay_device_selectionfail with and without it: they loadlibvulkan.so.1themselves (AGC graphics: cannot load Vulkan), which is unrelated to this change.
AI-assisted: yes
Tested on Apple M1 Pro, macOS 27.0.1 (x86_64 build under Rosetta 2)Follow-up to my 2026-10-23 test, which ran on the series alone without a Vulkan loader: this run uses the same head on the series merged with Applied at
AI-assisted: yes (Claude Code applied the PR, ran the tests, read the code and wrote this report). |
|
Windows run of this head
AI-assisted: yes (Claude Code) |
ReadProcessPrivateBytes reads RssAnon and VmSwap from /proc/self/status on every host other than Windows. macOS has no /proc, so the overlay's RAM line stays empty and agc_performance_overlay_statistics fails with "the process's private memory cannot be read". On macOS it now returns the task's phys_footprint from task_info(TASK_VM_INFO): its resident and compressed private memory, which is what Activity Monitor shows. jotitadev measured that this makes the test pass on macOS (boykopovar#7886). The same change is part of the macOS port in boykopovar#8658. Co-Authored-By: iamthehimansh <67262056+iamthehimansh@users.noreply.github.com> Co-Authored-By: jotitadev <139649225+jotitadev@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8f2db80 to
78afe4c
Compare
|
Rebased onto AI-assisted: yes |
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
|
Re-checked on the rebased head: |
What
ReadProcessPrivateBytes(Execution/src/PerformanceOverlay.cpp) readsRssAnonandVmSwapfrom/proc/self/statuson every host other than Windows. macOS has no/proc, so the overlay's RAM line stays empty andagc_performance_overlay_statisticsfails withthe process's private memory cannot be read. On macOS it now returns the task'sphys_footprintfromtask_info(TASK_VM_INFO): the process's resident and compressed private memory, the figure Activity Monitor shows. Linux and Windows are unchanged.@jotitadev measured that this makes the test pass on macOS (#7886's description) and left it out of #7886 because #7332 had it; #7332 is closed. The same change is part of #8658 (@iamthehimansh). Both are credited as co-authors; this splits it out as a small change, as for the other macOS pieces on #1. #8658 has since been closed in favour of topic PRs based on
main, so no other open PR carries this change.Tested
macOS 27.0.1, Apple M4 (10-core GPU), Apple clang 21, x86-64 build under Rosetta 2, MoltenVK 1.4.2 from the Vulkan SDK 1.4.363.0 (its universal loader, through
VK_DRIVER_FILES). The libraries do not link on macOS frommainalone, so the run is on mugurc'sci/macos-series-2(53db0ba,main3fd1cc3) with #7524, the code commits of #7937 and the libSceSsl change from #7332, as described on #1, plus my local macOS fixes listed there and #7886, so thatagc_performance_overlayloadslibvulkan.1.dyliband runs.ReadProcessPrivateBytesis the same in that tree as onmain.ctest -R '^agc_performance_overlay(_statistics)?$' -VwithANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation:the process's private memory cannot be read;agc_performance_overlayshowsRAM: 0.1 GB; one read takes 1.3 us.was not found but was requested by env var VK_INSTANCE_LAYERS. Both logs have the same two validation messages,VUID-VkDeviceCreateInfo-pProperties-04451from test(agc): load the Vulkan loader with portability enumeration on macOS #7886's test instance (it doesn't enableVK_KHR_portability_subset); this change adds none.ctest -j4on the same tree with Vulkan, with and without this change: 1648 tests each. Without it 60 failed, with it 59 in two runs; the two overlay tests are the only consistent difference. The others that differ are flaky under load on both builds and never reach this code (their recorders don't present):agc_driver_vertex_target_range_importedalso fails without the change (6 of 100 serial runs),agc_driver_sgpr_pattern_fill_keysfailed in a full run without it, andprecise_waitis a known flake.python3 tools/check_conventions.py --base upstream/main: passed.tools/pr_overlap.pylists only feat: macOS host support (Apple Silicon via Rosetta 2, Vulkan on MoltenVK) #8658 for this file, with this same change.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)🤖 Generated with Claude Code