Skip to content

perf(runner-shared): decode memtrack frames for module events in parallel - #578

Open
not-matthias wants to merge 3 commits into
cod-3819-memtrack-investigate-8-minutes-spent-in-teardownfrom
cod-3819-parallel-module-event-scan
Open

not-matthias wants to merge 3 commits into
cod-3819-memtrack-investigate-8-minutes-spent-in-teardownfrom
cod-3819-parallel-module-event-scan

Conversation

@not-matthias

@not-matthias not-matthias commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

TLDR: Finding the module events decoded the whole memtrack artifact on one thread through a streaming deserializer. Every frame is a self-contained zstd frame, so frames are now decoded in parallel from an in-memory buffer (stacked on #576).

  • decode_module_events splits the artifact into its zstd frames and decodes them on the rayon pool. Each worker decompresses a frame into a reused buffer and deserializes MemtrackEvent from that slice, so values are read in place instead of copied out of a stream first. Results keep artifact order.
  • The runner maps the artifact file instead of streaming it.
  • The Mapping/Fork/Exec filter moves to MemtrackEventKind::is_module_event.
  • A new test checks the decoder against a filtered full decode over every event kind, several frames and a truncated last frame.
  • The bench gets back its 10M and 100M sizes, now fast enough to run. The 100M artifact with stacks is about 4.65 GB on disk.

memtrack_reader bench, local walltime on a 32-thread machine:

Events without stacks with stacks
1M 258 → 15.4 ms 267 → 17.1 ms
10M 2.59 s → 101 ms 2.73 s → 114 ms
100M 26.0 s → 1.00 s 27.1 s → 1.05 s

Review notes

  • decode_module_events now takes &[u8] and returns a Vec.
  • A truncated last frame is still streamed, so the events before the cut are found as before. A frame in the middle that fails to decompress now returns an error; the streamed decoder stopped there silently.
  • CodSpeed simulation runs threads one at a time, so it only shows the single-threaded gain from decoding a buffer instead of a stream. The 10M and 100M cases will still take several minutes there, mostly generating the artifact.

@codspeed

codspeed Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 2 benchmarks

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

⚡ 2 improved benchmarks
❌ 2 regressed benchmarks
✅ 33 untouched benchmarks
🆕 4 new benchmarks
⏩ 6 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Memory find_module_events[1000000] 1.1 MB 4.4 MB -75.16%
❌ Memory find_module_events_with_stacks[1000000] 1.1 MB 4.5 MB -74.98%
⚡ WallTime find_module_events[1000000] 1,269.3 ms 187.1 ms ×6.8
⚡ WallTime find_module_events_with_stacks[1000000] 1,305.6 ms 193.8 ms ×6.7
🆕 WallTime find_module_events_with_stacks[10000000] N/A 1.1 s N/A
🆕 WallTime find_module_events_with_stacks[100000000] N/A 10.9 s N/A
🆕 WallTime find_module_events[10000000] N/A 1 s N/A
🆕 WallTime find_module_events[100000000] N/A 10.5 s N/A

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing cod-3819-parallel-module-event-scan (f18817f) with cod-3819-memtrack-investigate-8-minutes-spent-in-teardown (2f1f983)

Open in CodSpeed

Footnotes

  1. 6 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 marked this pull request as ready for review October 9, 2026 12:58
@not-matthias
not-matthias force-pushed the cod-3819-parallel-module-event-scan branch from b4b751a to caf6e0d Compare October 9, 2026 12:58
@greptile-apps

greptile-apps Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium impact] Changes how memory tracking artifacts are decoded.

The PR appears safe to merge, with a non-blocking recommendation to limit whole-frame decoding memory.

Fix All in Claude CodeFindings

  1. P2 Parallel decoding needs more RAM ▶
Fix with agent prompt
### Issue 1
crates/runner-shared/src/artifacts/memtrack/mod.rs:89-90
`module_events_in_frame` now expands a whole frame before discarding non-module events, and several workers can do this at once. `FRAME_EVENTS` limits event count, not decoded bytes. If an artifact has many captured stacks, these buffers can use substantially more RAM than the previous streamed reader.

Consider a decoded-size or memory-based worker limit, with a stack-heavy test, to keep memory use predictable on smaller machines.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

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

Summary

Reads module events from independent zstd frames in parallel, while preserving artifact order and streaming an incomplete final frame.

  • The runner now maps finished artifact files.
  • A shared is_module_event filter replaces the inline filter.
  • Tests compare complete and truncated artifacts with the streamed reader.
  • Benchmarks restore the 10M and 100M cases.
  • Non-blocking follow-up: bound the memory used by whole-frame decoding.

Acknowledgment: not-matthias explicitly described returning an error for a complete middle frame that cannot decompress as intentional; the previous reader stopped silently.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Finished artifact file] --> B[Read-only memory map]
  B --> C[Split complete zstd frames and remaining tail]
  C --> D[Decode complete frames in parallel]
  D --> E[Keep Mapping, Fork, and Exec events]
  E --> F[Combine results in artifact order]
  C --> G[Stream remaining tail]
  G --> F
  F --> H[Sort timeline and reconstruct process mappings]
Loading

Reviews (1) · Last reviewed commit: b4b751a · Reviewed by Greptile

Comment thread crates/runner-shared/src/artifacts/memtrack/mod.rs Outdated
@not-matthias
not-matthias force-pushed the cod-3819-parallel-module-event-scan branch 2 times, most recently from 8a11ab9 to e66e598 Compare October 9, 2026 14:15
…llel

Finding the Mapping, Fork and Exec events decoded the whole artifact
through one streaming deserializer on one thread. The streaming reader
copies every key, string and stack payload into a fresh allocation before
serde sees it.

Every frame the encoder writes is a self-contained zstd frame, so
decode_module_events now splits the artifact into its frames and decodes
them on the rayon pool. Each worker decompresses a frame into a reused
buffer and decodes events from that slice, so values are read in place
instead of copied out of a stream. Results are concatenated in artifact
order. A truncated last frame cannot be split off and is still streamed,
keeping the events before the cut. The runner maps the artifact instead of
streaming it from the file.

The Mapping/Fork/Exec filter moves to MemtrackEventKind::is_module_event,
and a test checks the new decoder against a filtered full decode over every
event kind, several frames and a truncated last frame.
…ule event bench

With frames decoded in parallel, CI-sized artifacts are fast enough to
benchmark again: add the 10M and 100M event sizes the streamed decoder was
too slow for. The 100M artifact with stacks is about 5 GB on disk and is
mapped, not held in memory.
…vents

Decompressing a whole frame before filtering it ties a worker's memory to
the frame's decoded size, which the event count per frame does not bound:
a frame of captured stacks decodes to hundreds of MB, held by every rayon
worker at once. Stream each frame through the existing event decoder
instead, so a worker holds about one zstd window and one event. Frames are
still decoded in parallel and events keep their artifact order.

This trades speed for bounded memory: on a 32-thread machine the 100M
event search takes about 1.85 s instead of 1.0 s, still well ahead of the
single-threaded streamed decoder.
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