Skip to content

perf(memtrack): reduce serialization bottleneck#436

Open
not-matthias wants to merge 13 commits into
mainfrom
cod-3071-fix-serialization-bottleneck
Open

perf(memtrack): reduce serialization bottleneck#436
not-matthias wants to merge 13 commits into
mainfrom
cod-3071-fix-serialization-bottleneck

Conversation

@not-matthias

Copy link
Copy Markdown
Member

Summary

  • replace derived flattened MemtrackEvent serialization with a byte-identical manual serializer
  • make MemtrackWriter::finish return encoded frame bytes for batching
  • encode contiguous memtrack event batches on worker threads and write sequence-numbered zstd frames in order

Verification

  • cargo test -p runner-shared artifacts::memtrack
  • cargo test -p runner-shared
  • cargo check -p runner-shared
  • cargo test --manifest-path crates/memtrack/Cargo.toml via Nix shell with libclang/build deps; eBPF integration cases compiled but were ignored because GITHUB_ACTIONS was unset
  • cargo bench -p runner-shared --bench memtrack_writer

Notes

  • memtrack_writer bench measures the Phase A single-writer encoder path only, not the full parallel memtrack pipeline.
  • End-to-end memory-mode speedup was not measured in this environment.

@codspeed-hq

codspeed-hq Bot commented Jul 6, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 51.79%

⚠️ 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.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 4 improved benchmarks
✅ 3 untouched benchmarks
🆕 4 new benchmarks

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation write_events[1000000] 2.1 s 1.4 s +56.12%
Simulation write_events[500000] 1,036.6 ms 679.9 ms +52.47%
Simulation write_events[10000] 20 ms 13.4 ms +49.62%
Simulation write_events[100000] 207.3 ms 139.1 ms +49.05%
🆕 Simulation encode_pipeline_zstd_fast[100000] N/A 103 ms N/A
🆕 Simulation encode_pipeline[100000] N/A 142 ms N/A
🆕 Simulation encode_pipeline[1000000] N/A 1.4 s N/A
🆕 Simulation encode_pipeline_zstd_fast[1000000] N/A 1 s N/A

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing cod-3071-fix-serialization-bottleneck (346a162) with main (3278ec8)

Open in CodSpeed

@not-matthias not-matthias force-pushed the cod-3071-fix-serialization-bottleneck branch from 381bac8 to 9be9b22 Compare July 6, 2026 15:10
@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the single-threaded, streaming MemtrackEvent serialization path with a parallel batch-encode pipeline. A manual serde::Serialize for MemtrackEvent eliminates the overhead of the derived flattened serializer, and a new encode_batches function distributes per-batch compression (msgpack + zstd) across worker threads, writing sequence-numbered frames to disk in order via a dedicated writer thread.

  • Manual serializer: Removes the #[derive(Serialize)] from MemtrackEvent and replaces it with a hand-written map serializer; a manual_serialize_is_byte_identical_to_derive test verifies byte-level parity with the old derived output, preserving wire-format compatibility with existing artifacts.
  • Parallel pipeline (pipeline.rs): A bounded work channel feeds n_workers zstd-bulk compressors in parallel; frames flow through an unbounded frame channel to a sequencing writer that reorders out-of-order completions before flushing to disk; a buffer pool recycles frame Vec allocations across workers.
  • Batch-channel poller (poller.rs): Replaces the per-event mpsc channel with a crossbeam_channel that delivers Vec<Event> batches of up to 64 K events; a straggler-drain loop after shutdown catches in-flight ring-buffer events before the poller joins.

Confidence Score: 5/5

Safe to merge; the new parallel encode pipeline is well-structured, error propagation from the writer thread is correctly ordered before any fallback sentinel, and the byte-identical serializer is test-verified.

The three core invariants of this change — correct frame ordering, error surfacing, and wire-format compatibility — are each covered by tests and the code logic is sound. The only known issue (partial tail-batch loss in integration test helpers) was flagged in a prior review pass and is unchanged by this PR.

crates/memtrack/tests/shared.rs — the integration test helper still lacks the stop_polling() call needed to flush the final partial batch before recv_timeout exits.

Important Files Changed

Filename Overview
crates/runner-shared/src/artifacts/memtrack/pipeline.rs New parallel encode pipeline: dispatch loop feeds N zstd-bulk workers via bounded channel; writer reorders frames by sequence number before writing; error handling correctly surfaces root-cause IO errors before the 'workers stopped early' sentinel via deferred joins.
crates/runner-shared/src/artifacts/memtrack/mod.rs Manual Serialize impl for MemtrackEvent replaces the derived flatten path; map-length calculation and field ordering are correct and verified byte-identical by the new shadow-struct test; Deserialize keeps the flatten attribute for backward compat.
crates/runner-shared/src/artifacts/memtrack/writer.rs MemtrackWriter refactored to return the inner writer W from finish(), enabling callers to collect the encoded frame bytes; compression level changed to -5 (zstd fast) from 1 to favour throughput over ratio in the streaming path.
crates/memtrack/src/ebpf/poller.rs Replaces per-event mpsc with crossbeam batch channel; STAGED thread-local accumulates events synchronously in the ring-buffer callback and is drained in batches each poll cycle; straggler drain loop correctly handles late-arriving events after shutdown.
crates/memtrack/src/main.rs Removed drain thread and writer thread; replaced with a single encode_batches call on a pipeline thread; stop_polling() correctly closes the batch channel so the pipeline drains to completion before joining.
crates/memtrack/src/ebpf/tracker.rs Tracker now owns the RingBufferPoller; stop_polling() drops it (triggering Drop::shutdown), cleanly joining the poll thread and flushing the final batch before closing the channel.
crates/memtrack/tests/shared.rs Updated to extend events from batch Vec rather than push individual events; however stop_polling() is still not called before the recv_timeout loop, so the final partial batch (for processes generating fewer than 64K events) will be silently dropped in integration tests (previously flagged).
crates/runner-shared/benches/memtrack_writer.rs Added encode_pipeline and encode_pipeline_zstd_fast benchmarks covering the parallel bulk-encode path at both level 1 and level -5; covers 100K and 1M event cases.
crates/memtrack/src/ebpf/memtrack.rs Renamed start_polling_with_channel to start_polling_with_batches and updated signature to accept batch_size; straightforward delegation to the updated poller.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant PT as Poll Thread
    participant BC as Batch Channel
    participant DL as Dispatch Loop
    participant WC as Work Channel
    participant W1 as Encode Workers
    participant FC as Frame Channel
    participant WT as Writer Thread
    participant OF as Output File

    PT->>BC: "Vec of events (batched)"
    DL->>BC: "recv() via IntoIterator"
    DL->>WC: "send((seq, batch))"
    WC->>W1: "recv()"
    W1->>W1: "msgpack serialize to scratch"
    W1->>W1: "zstd bulk compress to frame"
    W1->>FC: "send((seq, frame))"
    FC->>WT: "recv()"
    WT->>WT: "reorder via BTreeMap"
    WT->>OF: "write_all(frame in order)"
    WT->>WC: "pool_tx.send(frame) recycled buf"
    Note over PT: "stop_polling() called"
    PT->>PT: "straggler drain loop"
    PT->>BC: "drop tx, channel closed"
    DL->>WC: "drop work_tx"
    W1->>FC: "drop frame_tx clones"
    WT->>OF: "flush()"
Loading
%%{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 PT as Poll Thread
    participant BC as Batch Channel
    participant DL as Dispatch Loop
    participant WC as Work Channel
    participant W1 as Encode Workers
    participant FC as Frame Channel
    participant WT as Writer Thread
    participant OF as Output File

    PT->>BC: "Vec of events (batched)"
    DL->>BC: "recv() via IntoIterator"
    DL->>WC: "send((seq, batch))"
    WC->>W1: "recv()"
    W1->>W1: "msgpack serialize to scratch"
    W1->>W1: "zstd bulk compress to frame"
    W1->>FC: "send((seq, frame))"
    FC->>WT: "recv()"
    WT->>WT: "reorder via BTreeMap"
    WT->>OF: "write_all(frame in order)"
    WT->>WC: "pool_tx.send(frame) recycled buf"
    Note over PT: "stop_polling() called"
    PT->>PT: "straggler drain loop"
    PT->>BC: "drop tx, channel closed"
    DL->>WC: "drop work_tx"
    W1->>FC: "drop frame_tx clones"
    WT->>OF: "flush()"
Loading

Reviews (8): Last reviewed commit: "perf(memtrack): recycle frame buffers be..." | Re-trigger Greptile

Comment thread crates/memtrack/src/main.rs Outdated
Comment thread crates/memtrack/Cargo.toml Outdated
Comment thread crates/memtrack/src/main.rs Outdated
The custom Serialize impl emits ~13 tiny writes per event (map header +
per-field key/value). With the BufWriter on the compressed output side, each
of those hit zstd's streaming compressor directly, whose per-call overhead
dominated: ~2.4x slower than the derived flatten path, which coalesced each
event into one write.

Move the BufWriter to the encoder's input side so the tiny writes are batched
before reaching zstd. Restores baseline throughput.
@not-matthias not-matthias force-pushed the cod-3071-fix-serialization-bottleneck branch from 92a19a6 to d48193f Compare July 7, 2026 17:14
The drain path fed events through three per-event std::mpsc hops (poll ->
keepalive -> drain -> dispatcher) before batching, each send allocating a node
and locking.

Batch events in the poll callback instead: the single poll thread coalesces
into fixed-size batches and hands off one Vec at a time. The poller now lives in
the Tracker (removing the keepalive thread) and track() returns batches; main's
drain thread is gone and the dispatcher just tags ordered batches with a
sequence number. Per-event work drops from three mpsc sends to one Vec push.
@not-matthias not-matthias force-pushed the cod-3071-fix-serialization-bottleneck branch from a225638 to 9561cdd Compare July 7, 2026 17:43
Comment thread crates/runner-shared/src/artifacts/memtrack/pipeline.rs
The drain->encode->write pipeline lived inline in main.rs, so it was reachable
only through the eBPF path and had no test or benchmark coverage.

Move it into runner_shared::artifacts::memtrack::encode_batches, which takes any
ordered batch source (the live tracker channel or synthetic data) and an output
writer. Split the memtrack artifact module into mod/writer/pipeline. main.rs now
spawns one thread running encode_batches.

Adds unit tests for ordering across parallel workers and the empty-run frame,
plus an encode_pipeline divan benchmark writing to a sink.
The raw (uncompressed) sink existed only to measure the serialization floor in benches. With it gone the Compression enum collapses to a single Zstd variant, so pass the zstd level as a plain i32: encode_batches_with_config becomes encode_batches_with_level and encode_frame loses its match.
Compression is ~2/3 of the encode cost. Level -5 (fast strategy) raises pipeline throughput ~1.3x (11M -> 14.5M ev/s at 4 workers) so encoding stays ahead of the allocation rate and nothing backs up when eBPF finishes. On real traces the repeated map keys compress away regardless of level, so the size cost is small.
encode_frame built a fresh MemtrackWriter (new zstd Encoder = new CCtx) and output Vec for every batch. Give each worker one zstd::bulk::Compressor and one msgpack scratch buffer, reused across frames: serialize the batch into the scratch, then compress it in one shot. Amortizes the CCtx allocation (RawVecInner::with_capacity_in was ~5.6% self) and drops the per-frame streaming BufWriter. ~9% faster pipeline throughput (14.3M -> 15.7M ev/s, 1M events, level -5, 4 workers); output frames unchanged, decoder untouched.
Drop the min(8) cap on encode workers. The pipeline scales near-linearly with core count and memtrack does not measure wall-time, so using every core to keep encoding ahead of the allocation rate is worth the contention with the tracked process.
@not-matthias not-matthias force-pushed the cod-3071-fix-serialization-bottleneck branch from 9561cdd to 9292b3f Compare July 7, 2026 18:00
…per-event lock

The ring-buffer callback locked an Arc<Mutex<BatchAccumulator>> on every event, even though libbpf invokes it synchronously on the poll thread and nothing else touches the accumulator. Stage events in a thread-local Vec instead: the callback does a plain push (no atomic) and the poll loop, on the same thread, drains and batches them. Removes the lock/unlock atomics from the hottest serial stage, which gates ring-buffer overflow at high allocation rates.
The shutdown path did a fixed 50ms sleep between two consume() calls, paying the worst case even when the ring buffer drained immediately. Consume in a loop instead, sleeping only between empty rounds and stopping after a few consecutive idle rounds. Drains as fast as stragglers arrive (typically a few ms) while still tolerating late events, cutting the fixed tail wait after the tracked process exits.
@not-matthias not-matthias marked this pull request as ready for review July 7, 2026 18:19
Workers allocated a fresh output Vec per frame via Compressor::compress. Add a pool channel: the writer clears each written frame and returns it to the workers, which compress into a recycled buffer with compress_to_buffer. Amortizes the per-frame output allocation. The effect is negligible for low frame counts but adds up under the many-small-frames streaming that per-poll-cycle draining produces on high-rate runs. Best-effort: an empty pool just falls back to a fresh Vec.
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