[Perf] Fix memory gauge crash using task_info (#16121)#16220
[Perf] Fix memory gauge crash using task_info (#16121)#16220JesusRojass wants to merge 10 commits into
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
|
Work in progress!!! |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request replaces the use of malloc_zone_statistics with task_info(TASK_VM_INFO) in FPRMemoryGaugeCollector to prevent crashes on devices using the XZM allocator. It also updates the corresponding documentation, tests, and changelog. The review feedback suggests initializing the vmInfo struct to zero to avoid reading uninitialized stack memory and verifying that the returned count is at least TASK_VM_INFO_REV1_COUNT before accessing phys_footprint to ensure it was populated by the kernel.
…tor.m Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request replaces the use of malloc_zone_statistics with task_info(TASK_VM_INFO) in FPRMemoryGaugeCollector to prevent crashes on devices using the XZM allocator. It also updates the associated documentation and adds unit tests to verify the new memory collection behavior. The review feedback suggests simplifying the memory collection logic by removing the unused freeBytes variable and improving the debug logging to distinguish between a failed task_info call and an insufficient returned data count.
|
Ready for review @tejasd |
FPRMemoryGaugeCollectorsamples app memory on a timer. On iOS 17 and laterdevices running the XZone Malloc (XZM) allocator, the malloc zone statistics
call crashes inside
xzm_statistics_selfwhile taking a non-reentrant allocatorlock, killing the process with
EXC_BAD_ACCESS/EXC_BREAKPOINT.12.10.0 (#15595) replaced
mstats()withmalloc_zone_statistics()to address#15501. However
mstats()is a thin wrapper aroundmalloc_zone_statistics(NULL, &s), so the crashing call path was unchanged andreports kept coming in (#16121).
This change collects memory with
task_info(mach_task_self(), TASK_VM_INFO, ...)and reads
phys_footprintinstead.task_infois a kernel RPC that does nottouch libmalloc and takes no userspace allocator lock, so it cannot reach
xzm_statistics_self. This removes the crash. It also matches the Mach APIapproach already used by
FPRCPUGaugeCollector.Behavior note:
phys_footprintis the per-process physical memory footprint(the value Jetsam accounting and the Xcode memory gauge use). It is not a
heap-only statistic; it includes heap memory along with other dirty and
compressed pages.
heapUsednow carriesphys_footprint, andheapAvailableis reported as 0 because
phys_footprintreports only used memory. The backendfield
free_app_heap_memory_kbwill therefore be 0.Fixes #16121
Testing
FPRMemoryGaugeCollectorunit tests pass.API Changes