feat(memtrack): attach allocator probes on demand by not-matthias · Pull Request #445 · CodSpeedHQ/codspeed

not-matthias · 2026-07-08T19:44:00Z

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_file signals 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:

covers dlopen'd allocators and statically linked allocators in spawned children (previously missed entirely)
skips libraries the benchmark never loads
removes the startup discovery scan (system-wide library glob + build-dir walk), which was slow on nix-heavy systems

The eager discovery path and CODSPEED_MEMTRACK_BINARIES are removed since the watcher supersedes them (breaking: the env var is no longer read).

The crate API is now 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, 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 full consume() after all producers are stopped cannot miss requests.

Fixes COD-1801

codspeed-hq · 2026-07-08T19:46:58Z

Merging this PR will not alter performance

⚠️

Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 7 untouched benchmarks

_{Comparing cod-1801-only-attach-to-used-libraries-in-memtrack (5b76788) with main (eac7766)}

greptile-apps · 2026-07-08T20:02:54Z

Greptile Summary

This PR replaces memtrack's eager startup discovery (system-wide library glob + build-dir walk + CODSPEED_MEMTRACK_BINARIES) with an on-demand BPF-driven attach mechanism: a fentry/security_mmap_file probe signals the first exec-permission mapping of each unknown inode, a background worker SIGSTOPs the mapping process tree, classifies and attaches allocator probes, then resumes the processes.

On-demand attach worker (attach_worker.rs): fixpoint stop-the-world loop — stops all new pids, drains the ring buffer, repeats until no new pids appear, then classifies and inserts into known_inodes before releasing ContGuards, so processes resume only after the BPF map is updated.
New Tracker/Session split: Tracker owns the AttachWorker and the Arc<Mutex<MemtrackBpf>>; Session owns the spawned child and its event pipeline. The shell-stub spawn approach avoids the pre_exec-SIGSTOP deadlock.
RingBufferPoller generalized: now takes a caller-supplied parse closure and a sender, with an explicit synchronous drain() operation used by both the attach worker and the shutdown path.

Confidence Score: 4/5

The attach worker fixpoint drain and ContGuard-before-resume ordering are correct; one shutdown-path drain failure is silently discarded instead of being surfaced as a fatal error.

If the ring buffer poller thread dies unexpectedly during the shutdown drain, the worker exits cleanly, AttachWorker::finish() returns Ok(()), and the run proceeds with potentially incomplete allocator coverage and no diagnostic. All other correctness properties of the stop-the-world attach are sound.

crates/memtrack/src/ebpf/attach_worker.rs — the shutdown drain error path at the top of the shutdown branch in Worker::run.

Important Files Changed

Filename	Overview
crates/memtrack/src/ebpf/attach_worker.rs	New file implementing the stop-the-world on-demand attach worker. Core fixpoint logic is correct. One issue: drain failure during the shutdown path is silently ignored instead of being recorded as a fatal error.
crates/memtrack/src/ebpf/proc_fs.rs	New /proc utilities. wait_all_stopped correctly handles D-state and vanished processes. resolve_mapping uses map_files/ for deleted-file safety. Unit tests cover key parsing logic.
crates/memtrack/src/ebpf/c/attach.bpf.h	BPF fentry on security_mmap_file. Correctly checks PROT_EXEC, filters by tracked tgid, submits attach request before bpf_send_signal(SIGSTOP), guaranteeing ring buffer visibility before the process stops.
crates/memtrack/src/ebpf/tracker.rs	Reworked to wrap MemtrackBpf in Arc and own the AttachWorker. The spawn/stop/resume sequence correctly arms the pid before resuming the stub-stopped child.
crates/memtrack/src/ebpf/spawn.rs	Shell-stub approach avoids the pre_exec-SIGSTOP deadlock. wrap_stopped correctly preserves env and cwd. Regression test verifies the stop-then-resume cycle.
crates/memtrack/src/ebpf/poller.rs	Refactored to a generic RingBufferPoller using a control channel for synchronous drain. The Disconnected-on-drop shutdown model is clean and the drain/ack protocol is sound.
crates/memtrack/src/ebpf/memtrack.rs	Added attach_exec_watcher, insert_known_inode, attach_request_dropped_count, and poll_attach_with_channel. The 16-byte inode_key serialization matches the C struct layout.
crates/memtrack/src/session.rs	New Session type owns the child process and its event pipeline. Dropping the session stops the poller and closes the channel, terminating the event stream without a timeout.
crates/memtrack/src/main.rs	Updated to use the new Tracker/Session API. Correctly places tracker.finish() before the dropped-events check.
src/cli/exec/mod.rs	Removes CODSPEED_MEMTRACK_BINARIES env var propagation entirely, consistent with the on-demand watcher superseding eager discovery.
crates/memtrack/tests/dlopen_tests.rs	New integration tests for dlopen-based allocator discovery. Good coverage of single and concurrent-dlopen scenarios.
crates/memtrack/tests/shared.rs	Simplified test helper: drop(session) causes the poller to do a final consume and close the channel, so rx.iter() terminates cleanly. Per-feature target dirs prevent parallel test races.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Tracker
    participant AttachWorker
    participant BPF as BPF fentry security_mmap_file
    participant Child as Child Process
    Tracker->>BPF: attach_exec_watcher()
    Tracker->>AttachWorker: start(bpf)
    Tracker->>Child: spawn_stopped()
    Tracker->>BPF: add_tracked_pid(pid)
    Tracker->>Child: resume(pid)
    Child->>BPF: mmap(lib.so, PROT_EXEC)
    BPF->>BPF: check known_inodes miss
    BPF->>AttachWorker: ringbuf submit attach_request
    BPF->>Child: bpf_send_signal SIGSTOP
    AttachWorker->>Child: wait_all_stopped(pid)
    AttachWorker->>AttachWorker: poller.drain()
    AttachWorker->>AttachWorker: resolve_mapping
    AttachWorker->>BPF: attach_allocator_probes
    AttachWorker->>BPF: insert_known_inode
    AttachWorker->>Child: ContGuard drop SIGCONT
    Child->>BPF: allocations to events ring buffer
    BPF->>Tracker: MemtrackEvent via Session rx

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Tracker
    participant AttachWorker
    participant BPF as BPF fentry security_mmap_file
    participant Child as Child Process
    Tracker->>BPF: attach_exec_watcher()
    Tracker->>AttachWorker: start(bpf)
    Tracker->>Child: spawn_stopped()
    Tracker->>BPF: add_tracked_pid(pid)
    Tracker->>Child: resume(pid)
    Child->>BPF: mmap(lib.so, PROT_EXEC)
    BPF->>BPF: check known_inodes miss
    BPF->>AttachWorker: ringbuf submit attach_request
    BPF->>Child: bpf_send_signal SIGSTOP
    AttachWorker->>Child: wait_all_stopped(pid)
    AttachWorker->>AttachWorker: poller.drain()
    AttachWorker->>AttachWorker: resolve_mapping
    AttachWorker->>BPF: attach_allocator_probes
    AttachWorker->>BPF: insert_known_inode
    AttachWorker->>Child: ContGuard drop SIGCONT
    Child->>BPF: allocations to events ring buffer
    BPF->>Tracker: MemtrackEvent via Session rx

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/memtrack/src/ebpf/attach_worker.rs:119-122
**Drain failure during shutdown silently ignored**

If the `RingBufferPoller`'s poll thread dies unexpectedly (e.g., an unhandled `libbpf-rs` panic inside `ringbuf.consume()` or `ringbuf.poll()`), `drain()` returns `Err` (the ack never arrives or the ctl send fails). The worker thread then returns early without calling `record_fatal`, so `AttachWorker::finish()` sees a clean thread join, `fatal = None`, and `attach_request_dropped_count == 0` — returning `Ok(())` despite potentially unprocessed attach requests remaining in the ring buffer. The benchmark would proceed with incomplete allocator coverage and no error reported.

_{Reviews (4): Last reviewed commit: "test(memtrack): add dlopen/on-demand all..." | Re-trigger Greptile}

…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.

greptile-apps · 2026-07-09T16:59:18Z

+impl Worker {
+    fn run(self) {
+        let mut known: HashSet<(u64, u64)> = HashSet::new();
+


Drain failure during shutdown silently ignored

If the RingBufferPoller's poll thread dies unexpectedly (e.g., an unhandled libbpf-rs panic inside ringbuf.consume() or ringbuf.poll()), drain() returns Err (the ack never arrives or the ctl send fails). The worker thread then returns early without calling record_fatal, so AttachWorker::finish() sees a clean thread join, fatal = None, and attach_request_dropped_count == 0 — returning Ok(()) despite potentially unprocessed attach requests remaining in the ring buffer. The benchmark would proceed with incomplete allocator coverage and no error reported.

Prompt To Fix With AI

This is a comment left during a code review. Path: crates/memtrack/src/ebpf/attach_worker.rs Line: 119-122 Comment: **Drain failure during shutdown silently ignored** If the `RingBufferPoller`'s poll thread dies unexpectedly (e.g., an unhandled `libbpf-rs` panic inside `ringbuf.consume()` or `ringbuf.poll()`), `drain()` returns `Err` (the ack never arrives or the ctl send fails). The worker thread then returns early without calling `record_fatal`, so `AttachWorker::finish()` sees a clean thread join, `fatal = None`, and `attach_request_dropped_count == 0` — returning `Ok(())` despite potentially unprocessed attach requests remaining in the ring buffer. The benchmark would proceed with incomplete allocator coverage and no error reported. How can I resolve this? If you propose a fix, please make it concise.

GuillaumeLagrange

Stopping here becuase I want us to dig the TRACEME thing before leaning in the stop wrapper

GuillaumeLagrange · 2026-07-10T13:09:22Z

@@ -0,0 +1,54 @@
+/* == Exec-mapping watcher: on-demand allocator attach == */


Slightly misleading: this is not really on demand allocator attach, this is a mmap watcher that stops the program during memmap and queues the request in a ring buffer for further processing

GuillaumeLagrange · 2026-07-10T13:11:44Z

    return 0;
 }

+#include "attach.bpf.h"


that's slightly weird to attach.bpf.h like this no?
is this to keep a single .bpf.c entry to easy build.rs compilation?

Maybe we could have a main.bpf.c or sthing that just includes sibling module.bpf.c files to keep things a bit tidier?

GuillaumeLagrange · 2026-07-10T13:12:51Z

    /* Default to enabled if map not initialized */
    if (!enabled) {
        return 1;
    }


completely unrelated to your code, but is this really the default behavior we want? To me, if map not initialized, the whoel thing is just not enabled

GuillaumeLagrange · 2026-07-10T13:14:00Z

+#[derive(Debug, Clone, Copy)]
+pub struct AttachRequest {
+    pub pid: u32,
+    pub dev: u64,


we dropped the tid here, is this on purpose?

If we don't need it do we need to emit it from the bpf script?

GuillaumeLagrange · 2026-07-10T13:17:33Z

+    /// Create a new tracker with on-demand allocator attach.
    ///
-    /// This will:
-    /// - Initialize the BPF subsystem
-    /// - Bump memlock limits
-    /// - Attach uprobes to all libc instances
-    /// - Attach tracepoints for fork tracking
+    /// No allocators are pre-attached: the exec-mapping watcher discovers and
+    /// attaches them as the tracked tree maps executable files.


Overfitted comment that focuses on the difference between previous impl and this one, maybe a good candidate for the deslop skill?

GuillaumeLagrange · 2026-07-10T13:20:24Z

        Self::bump_memlock_rlimit()?;

        let mut bpf = MemtrackBpf::new()?;
        bpf.attach_tracepoints()?;


would be clearer to get read of the attach_tracepoints indirection in favor of a direct attach_sched_fork call.

GuillaumeLagrange · 2026-07-10T13:23:42Z

+    /// `uid_gid` drops the child's privileges (a `Command`'s uid/gid cannot be
+    /// read back, so it cannot be preserved through the wrap).
+    pub fn spawn(&self, cmd: &Command, uid_gid: Option<(u32, u32)>) -> Result<Session> {
+        let mut wrapped = wrap_stopped(cmd);


Why do we need to have the command self-stopped ??
Isnt it precisely the role of the bpf exec mapper that we've attached??

GuillaumeLagrange · 2026-07-10T13:46:43Z

+pub fn stopped_command(program: impl AsRef<OsStr>, args: &[OsString]) -> Command {
+    let mut cmd = Command::new("sh");
+    cmd.arg("-c")
+        .arg(r#"kill -STOP "$$"; exec "$@""#)
+        .arg("sh")
+        .arg(program.as_ref())
+        .args(args);
+    cmd
+}


I am 99% sure that we'll get both simpler code and much more reliable results by adding a ptrace(PTRACE_TRACEME) syscall, WDYT?

Did you already try it and discard it? Because while I see why we dont do a self SIGSTOP, ptrace seems more tried and tested, as well as saving us from yet another shell wrapper

greptile-apps Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread crates/memtrack/src/ebpf/attach_worker.rs Outdated

feat(memtrack): add exec-mapping watcher BPF prog and inode maps

f15720b

not-matthias force-pushed the cod-1801-only-attach-to-used-libraries-in-memtrack branch from 8bce092 to 16ffc38 Compare July 9, 2026 16:28

not-matthias marked this pull request as ready for review July 9, 2026 16:29

not-matthias added 4 commits July 9, 2026 18:48

test(memtrack): add dlopen/on-demand allocator tests + CI matrix

5b76788

not-matthias force-pushed the cod-1801-only-attach-to-used-libraries-in-memtrack branch from 16ffc38 to 5b76788 Compare July 9, 2026 16:50

not-matthias requested a review from GuillaumeLagrange July 9, 2026 16:51

greptile-apps Bot reviewed Jul 9, 2026

View reviewed changes

GuillaumeLagrange requested changes Jul 10, 2026

View reviewed changes

		@@ -0,0 +1,54 @@
		/* == Exec-mapping watcher: on-demand allocator attach == */

Uh oh!

Conversation

not-matthias commented Jul 8, 2026

Uh oh!

codspeed-hq Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

greptile-apps Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

greptile-apps Bot Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

GuillaumeLagrange left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codspeed-hq Bot commented Jul 8, 2026 •

edited

Loading

greptile-apps Bot commented Jul 8, 2026 •

edited

Loading