Skip to content

Conversation

@not-matthias
Copy link
Member

@not-matthias not-matthias commented Jan 5, 2026

Changes in this PR:

  • Writing files to disk directly after reading from eBPF (reduced the overall overhead from 20s to 3s on a large example)
  • Zstd compression (which decreases the file size by ~10x)

@not-matthias not-matthias force-pushed the cod-1927-compress-memtrack-reports-with-zstd branch 2 times, most recently from 1afc939 to f58ee6c Compare January 5, 2026 15:23
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds zstd compression support to memtrack artifacts to reduce storage requirements. The implementation introduces streaming compression/decompression and refactors the event processing architecture to write events directly to disk during collection rather than accumulating them in memory.

Key changes:

  • Introduces MemtrackWriter with zstd compression for streaming event serialization
  • Refactors track_command to use a two-stage threading model (drain thread → writer thread) for immediate disk writing
  • Centralizes filename generation logic in the ArtifactExt trait

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
crates/runner-shared/src/artifacts/mod.rs Adds file_name() method to centralize filename generation for artifacts with optional PID
crates/runner-shared/src/artifacts/memtrack.rs Introduces MemtrackWriter struct with zstd compression and updates decoder to decompress streams
crates/memtrack/src/main.rs Refactors event processing to use two-stage threading (drain + writer) for streaming events to compressed disk files
crates/runner-shared/Cargo.toml Adds zstd dependency version 0.13
Cargo.lock Updates lock file with zstd and its transitive dependencies

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@not-matthias not-matthias force-pushed the cod-1927-compress-memtrack-reports-with-zstd branch 3 times, most recently from f3e451e to 2cb7cc4 Compare January 5, 2026 16:31
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 5, 2026

CodSpeed Performance Report

Congrats! CodSpeed is installed 🎉

🆕 4 new benchmarks were detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

Detected benchmarks

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +22 to +24
format!("{pid}.{}.msgpack", Self::name())
} else {
format!("{}.msgpack", Self::name())
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The file extension .msgpack is misleading now that the files are compressed with zstd. Consider changing the extension to .msgpack.zst or .msgpack.zstd to accurately reflect that the files are compressed, which will help users understand the file format and ensure proper handling.

Suggested change
format!("{pid}.{}.msgpack", Self::name())
} else {
format!("{}.msgpack", Self::name())
format!("{pid}.{}.msgpack.zst", Self::name())
} else {
format!("{}.msgpack.zst", Self::name())

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that it's misleading, but I think it's not worth it changing the file extension. The data format is hidden in the MemtrackWriter and the MemtrackEventStream, so the consumer doesn't care about it.

@not-matthias not-matthias force-pushed the cod-1927-compress-memtrack-reports-with-zstd branch from 2cb7cc4 to 35e5f3f Compare January 5, 2026 16:50
@not-matthias not-matthias merged commit 35e5f3f into main Jan 5, 2026
12 checks passed
@not-matthias not-matthias deleted the cod-1927-compress-memtrack-reports-with-zstd branch January 5, 2026 16:57
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.

3 participants