feat(memtrack): attach allocator probes on demand#445
Conversation
Merging this PR will not alter performance
|
Greptile SummaryThis PR replaces the eager startup-discovery scan (system-wide library glob + build-dir walk +
Confidence Score: 5/5Safe to merge; the correctness argument for the stop-the-world drain is sound, error paths are well-guarded by ContGuard drop semantics and explicit fatal-error recording. The fixpoint stop loop, synchronous drain after all producers are paused, and ContGuard resumption-on-drop cover the main correctness surface of the design. The remaining findings are style-only and carry no runtime risk. No files require special attention; the core attach_worker.rs and attach.bpf.h logic is well-reasoned and commented. Important Files Changed
|
8bce092 to
16ffc38
Compare
16ffc38 to
5b76788
Compare
GuillaumeLagrange
left a comment
There was a problem hiding this comment.
Stopping here becuase I want us to dig the TRACEME thing before leaning in the stop wrapper
…tcher Instead of pre-attaching every allocator found on the system, a BPF fentry on security_mmap_file signals the first mapping of each unknown executable inode. A background worker stops the mapping processes, classifies the file by its symbols, attaches probes, and resumes them. This covers dlopen'd and statically linked allocators in spawned children, and skips libraries the benchmark never loads. The public API shrinks to Tracker (owns the attach worker) and Session (owns the spawned child and its event pipeline); ring buffers are polled through a generic RingBufferPoller with caller-supplied parsing. Fixes COD-1801
On-demand attach supersedes the startup scan: delete the system-wide library glob, the build-dir walk, and the CODSPEED_MEMTRACK_BINARIES env var (the runner no longer resolves exec target binaries for it). BREAKING CHANGE: CODSPEED_MEMTRACK_BINARIES is no longer read Refs COD-1801
Distro libjemalloc is built without the je_ symbol prefix, so symbol classification missed it and fell through to libc++ (jemalloc exports operator new), leaving plain malloc/aligned_alloc unprobed. Match on mallocx, which keeps its name in unprefixed builds and is unique to jemalloc.
a0a194d to
4d3fbb0
Compare
Attach allocator uprobes only to libraries the tracked process tree actually maps, instead of pre-attaching every allocator discovered on the system at startup.
A BPF fentry on
security_mmap_filesignals the first mapping of each unknown executable inode. A background worker SIGSTOPs the mapping processes, classifies the file by its allocator symbols, attaches probes, and resumes them. This:dlopen'd allocators and statically linked allocators in spawned children (previously missed entirely)The eager discovery path and
CODSPEED_MEMTRACK_BINARIESare removed since the watcher supersedes them (breaking: the env var is no longer read).The crate API is now
Tracker(owns the attach worker) andSession(owns the spawned child and its event pipeline); ring buffers are polled through a genericRingBufferPollerwith caller-supplied parsing, replacing the keepalive forwarding thread.Review focus: the stop-the-world attach in
attach_worker.rs(fixpoint stop + synchronous ring buffer drain) — its correctness argument is that a fullconsume()after all producers are stopped cannot miss requests.Fixes COD-1801