Skip to content

feat(memtrack): pause producers under ring pressure - #543

Open
not-matthias wants to merge 10 commits into
cod-3222-add-ebpf-based-dwarffp-unwindingfrom
feat/memtrack-pause-worker
Open

not-matthias wants to merge 10 commits into
cod-3222-add-ebpf-based-dwarffp-unwindingfrom
feat/memtrack-pause-worker

Conversation

@not-matthias

Copy link
Copy Markdown
Member

Adds opt-in, best-effort backpressure for memtrack's BPF ring buffers.

When a ring crosses its watermark, BPF latches the episode and stops the winning tracked producer. A userspace coordinator then stops the owned process tree using pidfds, drains the event, stack, mapping, and attach pipelines, clears the latch, and resumes the tree. Finite timeouts terminate the tracked tree and fail the capture; 0 remains the default and preserves drop detection, while inf waits until recovery. Detected ring loss still fails the capture.

The change also:

  • lowers ring polling latency from 10 ms to 1 ms
  • keeps attach/startup resume ownership coordinated with pressure pauses
  • propagates the timeout through the runner's experimental memory-mode flag
  • avoids x86 per-CPU private BPF stack corruption during nested uprobes by keeping hash scratch in the unpublished ring record and reducing BPF stack use

This PR is stacked on #522.

Verification

  • cargo fmt --all -- --check
  • cargo clippy --release -p memtrack -p codspeed-runner -- -D warnings
  • Runtime tests are deferred to CI.

@codspeed

codspeed Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

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.

✅ 33 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing feat/memtrack-pause-worker (4953b6c) with cod-3222-add-ebpf-based-dwarffp-unwinding (2152fe2)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@not-matthias
not-matthias force-pushed the feat/memtrack-pause-worker branch from 15e7943 to 89556be Compare September 23, 2026 12:23
@not-matthias
not-matthias marked this pull request as ready for review September 23, 2026 12:42
@not-matthias
not-matthias added this pull request to stack #545 September 23, 2026 12:43
@greptile-apps

greptile-apps Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

The PR is not yet safe to merge because a pressure-release race can leave a tracked producer permanently stopped.

Fix All in Claude CodeFindings

  1. P1 Pressure Stop Can Be Lost ▶
Fix with agent prompt
### Issue 1
crates/memtrack/src/ebpf/c/utils/pressure.bpf.h:34-37
After the BPF code records a pressure hold, the independent poller can observe an empty ring, delete that record, and send `SIGCONT` before `bpf_send_signal(SIGSTOP)` runs. The later `SIGSTOP` then suspends the producer with no remaining record that can resume it, causing the tracked workload and capture to hang.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR adds stack-ring backpressure to memtrack, coordinates pressure and attach stop ownership, bounds synchronous draining by a producer-position snapshot, moves stack-hash scratch into unpublished ring records, and adds high-pressure integration fixtures.

  • Stops tracked stack-producing processes when the stack ring crosses its watermark.
  • Coordinates pressure and attach holds through shared BPF maps.
  • Reduces poll latency and validates stack-ring loss as capture loss.
  • Adds multithreaded and multiprocess pressure tests.
  • Preserves teardown work when waiting for the tracked command fails.
Diagram
sequenceDiagram
    participant P as Tracked producer
    participant B as BPF stack probe
    participant R as Stack ring
    participant U as Userspace poller
    participant S as Stop coordinator

    P->>B: Allocation probe
    B->>R: Submit stack record
    B->>S: Record pressure hold
    B-->>P: Request SIGSTOP
    U->>R: Drain snapshot
    U->>S: Release pressure holds
    S-->>P: SIGCONT when no attach hold remains
Loading

Reviews (5) · Last reviewed commit: "fixup! feat(memtrack): pause producers u..."

Comment thread crates/memtrack/src/ebpf/attach_worker.rs Outdated
Comment thread crates/memtrack/tests/pressure_tests.rs
@not-matthias
not-matthias force-pushed the feat/memtrack-pause-worker branch from 31b6da1 to 0431cd9 Compare September 24, 2026 13:10
@not-matthias

Copy link
Copy Markdown
Member Author

@greptileai

Comment thread crates/memtrack/src/ebpf/poller.rs Outdated
Comment thread crates/memtrack/src/ebpf/poller.rs Outdated
Comment thread crates/memtrack/src/ebpf/pause.rs Outdated
@not-matthias
not-matthias force-pushed the feat/memtrack-pause-worker branch from 0431cd9 to 58f8c9b Compare September 24, 2026 13:36
Share one poll interval across the event, stack and attach pollers and
lower it from 10ms to 1ms so bursts drain before the rings fill.
A full allocation-stack ring loses stack records the same way a full
event ring loses events, so a run that overflowed it must fail the same
incompleteness check.
The FNV lanes lived on the BPF stack. Large kprobe-family programs may
spill that to per-CPU storage, which a nested uprobe on the same CPU can
overwrite mid-capture, corrupting the hash. Accumulate the lanes in the
not-yet-submitted ring record instead, which is private to this
reservation.
@not-matthias
not-matthias force-pushed the feat/memtrack-pause-worker branch from 58f8c9b to cba9475 Compare September 24, 2026 13:47
@greptile-apps

This comment has been minimized.

After every event or stack submission, BPF checks the ring's fill level.
Once it is 75% full, the writing tracked process is recorded in
`pressure_stopped` and gets SIGSTOP, so processes that don't write keep
running.

The event and stack pollers resume every recorded process once a poll
leaves their ring empty, and resume everything still recorded on
shutdown. A tracked process that writes to a nearly full ring after the
pollers are gone stays stopped.

A process can be stopped both for ring pressure and for an allocator
attach request, and SIGSTOP is not counted. The exec-mapping watcher
therefore records its stops in `attach_stopped`. Each side deletes its
own entry before checking the other's, so the process resumes only once
both are done with it.

`RingBufferPoller::drain` no longer acknowledges a consume that stopped at
an uncommitted reservation, and `wait_all_stopped` treats exited threads
as stopped.

The event and stack poll interval is configurable through the
`poll_interval_ms` tracker option (env `CODSPEED_MEMTRACK_POLL_INTERVAL_MS`,
default 1ms), which lets the event ring cross its watermark on demand. The
attach poller keeps its fixed interval.
Add `alloc_storm` (threads) and `alloc_storm_procs` (forked processes)
fixtures and pressure tests that run them with a 10s poll interval and
assert that no events are dropped. The multi-process test checks that
every writing process is stopped and resumed on its own.
On glibc >= 2.42 the per-thread tcache is initialized lazily. A thread's
first small free() whose tcache is still inactive goes through
tcache_free_init(), which tail-calls __libc_free() again, so the free
uprobe fires twice for one call. Whether a thread reaches that path
depends on arena assignment, i.e. scheduling, so the Free count of the
same workload varies between runs.

for_each_variant compared raw Free counts between the Legacy and Token
runs, which made test_thread_dlopen flaky on ubuntu-26.04-arm
(glibc 2.43). GLIBC_TUNABLES (tcache_count=0, tcache_max=0) does not
avoid the re-entry.

event_profile now replays events in timestamp order and counts a Free
only when it releases an allocation still live in that run, which drops
the duplicate hit as well as frees of memory allocated before tracking.
The stop maps kept a process's entry after it exited, so a later release
could send SIGCONT to an unrelated process that reused the pid.

The exit handler now deletes a process from `pressure_stopped` and
`attach_stopped`, and a release resumes a process only if it removed its
own entry. The exec-mapping watcher stops a process only once it is
recorded, like the pressure check, so every stop has an entry to release.
Both maps move to a shared header so the exit handler can reach them.
Comment thread crates/memtrack/src/ebpf/poller.rs Outdated
Comment on lines +34 to +37
__u8 marker = 1;
if (bpf_map_update_elem(&pressure_stopped, &current_tgid, &marker, BPF_ANY) != 0) {
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Pressure Stop Can Be Lost

After the BPF code records a pressure hold, the independent poller can observe an empty ring, delete that record, and send SIGCONT before bpf_send_signal(SIGSTOP) runs. The later SIGSTOP then suspends the producer with no remaining record that can resume it, causing the tracked workload and capture to hang.

Knowledge Base Used: eBPF memory tracker

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/memtrack/src/ebpf/c/utils/pressure.bpf.h
Line: 34-37

Comment:
**Pressure Stop Can Be Lost**

After the BPF code records a pressure hold, the independent poller can observe an empty ring, delete that record, and send `SIGCONT` before `bpf_send_signal(SIGSTOP)` runs. The later `SIGSTOP` then suspends the producer with no remaining record that can resume it, causing the tracked workload and capture to hang.

**Knowledge Base Used:** [eBPF memory tracker](https://app.greptile.com/codspeed/-/custom-context/knowledge-base/codspeedhq/codspeed/-/docs/ebpf-memory-tracker.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant