Skip to content

Commit d99e3bf

Browse files
committed
perf(memtrack): resolve stack fp chains off the ring poll thread
Each stack record needs its frame-pointer chain looked up in the stack_traces map, which is a syscall per record. Doing that inside the ring-buffer parse callback made the poll thread pay it, so a burst of stacks could push it behind the producer and records were dropped. ResolvingPoller wraps a RingBufferPoller with a dedicated resolver thread: the poll thread only parses (event, stackid) and hands it over an internal channel, and the resolver does the map lookup and forwards the completed event. Drop order keeps the existing shutdown contract, the ring is dropped first so its poll thread joins and closes the internal sender, which lets the resolver drain what it already has before its join returns.
1 parent a1ca01e commit d99e3bf

3 files changed

Lines changed: 81 additions & 17 deletions

File tree

‎crates/memtrack/src/ebpf/memtrack/mod.rs‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::collections::HashMap;
66
use std::mem::MaybeUninit;
77
use std::path::Path;
88

9-
use crate::ebpf::poller::RingBufferPoller;
9+
use crate::ebpf::poller::{RingBufferPoller, ThreadedRingBufferPoller};
1010
use crate::ebpf::tracker::TrackerOptions;
1111

1212
mod token {
@@ -248,33 +248,37 @@ impl MemtrackBpf {
248248
))
249249
}
250250

251-
/// Poll the stack-record ring buffer into `tx`.
251+
/// Poll the stack-record ring buffer into `tx`. The map lookup for each
252+
/// stack's frame-pointer chain is a syscall, so it runs on a resolver
253+
/// thread rather than the ring poll thread: see [`ThreadedRingBufferPoller`].
252254
pub(crate) fn poll_stacks(
253255
&self,
254256
poll_interval_ms: u64,
255257
tx: std::sync::mpsc::Sender<runner_shared::artifacts::MemtrackEvent>,
256-
) -> Result<RingBufferPoller> {
258+
) -> Result<ThreadedRingBufferPoller> {
257259
use crate::ebpf::events;
258260
use runner_shared::artifacts::MemtrackEventKind;
259261

260-
// The poller outlives this borrow of the skeleton, so the chain lookup
261-
// needs an owned handle rather than a reference to the skeleton map.
262+
// The resolver thread outlives this borrow of the skeleton, so the
263+
// chain lookup needs an owned handle rather than a reference to the
264+
// skeleton map.
262265
let stack_traces = with_skel!(self, skel => {
263266
libbpf_rs::MapHandle::try_from(&skel.maps.stack_traces)
264267
.context("Failed to create handle for stack_traces map")?
265268
});
266269

267-
let parse = move |data: &[u8]| {
268-
let (mut event, stackid) = events::parse_stack(data)?;
269-
if let MemtrackEventKind::Stack { record } = &mut event.kind {
270-
record.fp_chain = events::fp_chain(&stack_traces, stackid);
271-
}
272-
Some(event)
273-
};
270+
let resolve =
271+
move |(mut event, stackid): (runner_shared::artifacts::MemtrackEvent, i64)| {
272+
if let MemtrackEventKind::Stack { record } = &mut event.kind {
273+
record.fp_chain = events::fp_chain(&stack_traces, stackid);
274+
}
275+
event
276+
};
274277

275-
with_skel!(self, skel => RingBufferPoller::new(
278+
with_skel!(self, skel => ThreadedRingBufferPoller::new(
276279
&skel.maps.stacks,
277-
parse,
280+
events::parse_stack,
281+
resolve,
278282
tx,
279283
poll_interval_ms,
280284
))

‎crates/memtrack/src/ebpf/poller.rs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,63 @@ impl Drop for RingBufferPoller {
7777
}
7878
}
7979
}
80+
81+
/// A [`RingBufferPoller`] whose parsed items need a further, potentially
82+
/// expensive step (e.g. a BPF map lookup, which is a syscall) before they are
83+
/// forwarded on `tx`. That step runs on a dedicated resolver thread instead
84+
/// of the poll thread, so a slow per-record resolve can't make the poll
85+
/// thread fall behind the ring and drop records.
86+
pub struct ThreadedRingBufferPoller {
87+
// Declaration order is the shutdown order: dropping `ring` first
88+
// disconnects its control channel and joins its poll thread, which drops
89+
// the internal sender the resolver reads from. Only then can the
90+
// resolver's `recv` loop observe disconnection, finish forwarding
91+
// whatever it already has, and let `resolver`'s join below return. This
92+
// gives callers the same "disconnect, fully drain, then join" contract
93+
// as a plain `RingBufferPoller`.
94+
ring: Option<RingBufferPoller>,
95+
resolver: Option<JoinHandle<()>>,
96+
}
97+
98+
impl ThreadedRingBufferPoller {
99+
/// Poll `rb_map` with `parse` like [`RingBufferPoller::new`], but run
100+
/// `resolve` on a separate thread: `parse` results are forwarded over an
101+
/// internal channel, and `resolve` turns each one into the value sent on
102+
/// `tx`.
103+
pub fn new<M, T, U, F, R>(
104+
rb_map: &M,
105+
parse: F,
106+
resolve: R,
107+
tx: Sender<U>,
108+
poll_interval_ms: u64,
109+
) -> Result<Self>
110+
where
111+
M: MapCore,
112+
T: Send + 'static,
113+
U: Send + 'static,
114+
F: Fn(&[u8]) -> Option<T> + Send + 'static,
115+
R: Fn(T) -> U + Send + 'static,
116+
{
117+
let (parsed_tx, parsed_rx) = mpsc::channel::<T>();
118+
let ring = RingBufferPoller::new(rb_map, parse, parsed_tx, poll_interval_ms)?;
119+
let resolver = std::thread::spawn(move || {
120+
for item in parsed_rx {
121+
let _ = tx.send(resolve(item));
122+
}
123+
});
124+
125+
Ok(Self {
126+
ring: Some(ring),
127+
resolver: Some(resolver),
128+
})
129+
}
130+
}
131+
132+
impl Drop for ThreadedRingBufferPoller {
133+
fn drop(&mut self) {
134+
drop(self.ring.take());
135+
if let Some(resolver) = self.resolver.take() {
136+
let _ = resolver.join();
137+
}
138+
}
139+
}

‎crates/memtrack/src/session.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::ebpf::poller::RingBufferPoller;
1+
use crate::ebpf::poller::{RingBufferPoller, ThreadedRingBufferPoller};
22
use crate::perf_mappings::PerfMappingPoller;
33
use crate::prelude::*;
44
use runner_shared::artifacts::MemtrackEvent;
@@ -17,7 +17,7 @@ pub struct Session {
1717
// join their poll threads before PerfMappingPoller drops and emits its
1818
// buffered Mapping records as the terminal stream suffix.
1919
_poller: RingBufferPoller,
20-
_stack_poller: Option<RingBufferPoller>,
20+
_stack_poller: Option<ThreadedRingBufferPoller>,
2121
_perf_mapping_poller: Option<PerfMappingPoller>,
2222
}
2323

@@ -26,7 +26,7 @@ impl Session {
2626
child: Child,
2727
events: Receiver<MemtrackEvent>,
2828
poller: RingBufferPoller,
29-
stack_poller: Option<RingBufferPoller>,
29+
stack_poller: Option<ThreadedRingBufferPoller>,
3030
perf_mapping_poller: Option<PerfMappingPoller>,
3131
) -> Self {
3232
Self {

0 commit comments

Comments
 (0)