prof: grow the dump's location table instead of spinning when it fills - #28
Merged
Merged
Conversation
…s, free it afterwards, and let a failed write end the dump mi_prof_dump_pb collected unique frame addresses into a fixed 4096-slot open-addressed table with no occupancy check, so a profile with more unique frames than that made mi_prof_loc_find probe forever while holding the profiler lock; the table was also never freed. It now doubles (kept at most half full) and is released when the dump finishes. pb_flush discards the staging buffer once an error is recorded, so a short write (or the new OOM path) makes the dump return -1 instead of pb_raw spinning on a full buffer. test-prof gains a case with 5000 distinct call sites sampled at rate 1. No-Verification-Needed: allocator-fork profiler; covered by mimalloc's own test-prof / test-prof-adversarial
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of the startup-snapshot branch (#13 / #16), where it rode along with unrelated changes.
mi_prof_dump_pbdedups frame addresses through an open-addressed table that was a fixed 4096 slots with no occupancy check: a profile with more unique frames than that (easy withMIMALLOC_PROF_SAMPLE_RATEon a real program — 32 frames per sample) mademi_prof_loc_findprobe a full table forever, withmi_prof.lockheld. The table was also allocated with_mi_os_zallocand never freed, once per dump.pb_flushnow discards the staging buffer onceerris set. Before, a shortwrite()seterr, the next flush returned early withposstill at capacity, andpb_rawlooped ontake == 0— the same hang by another route (found in review by forcing the grow allocation to fail).test-profadds 5000 distinct call sites sampled at rate 1 and dumps them; on the base branch that case times out.ctestin a DebugMI_DEBUG_FULLbuild: everything passes excepttest-stress-subprocsandtest-purge-holes, which fail identically onbun-dev3-v2without this change on this machine (the heap-teardown work in #27 is in that area).