Skip to content

Conversation

@not-matthias
Copy link
Member

@not-matthias not-matthias commented Sep 19, 2025

Changes in this PR:

  • Create runner-shared crate which is used in the perf-parser crate
  • Add support for the perf v2 format
  • Add SetVersion command to detect incompatible integrations

@not-matthias not-matthias force-pushed the cod-1364-runner-sample-without-splitting-perfdata branch from 6740e30 to b4b3c9c Compare September 22, 2025 17:47
@not-matthias not-matthias changed the base branch from main to cod-1356-unresolved-addresses-with-pie-benchmark-executable September 22, 2025 17:48
@not-matthias not-matthias marked this pull request as ready for review September 22, 2025 17:48
@not-matthias not-matthias force-pushed the cod-1364-runner-sample-without-splitting-perfdata branch from 8c41561 to 79f05c9 Compare September 23, 2025 13:39
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 refactors the perf runner to use a single perf.data file instead of splitting it across multiple files, and extracts shared data structures into a new runner-shared crate.

  • Consolidates perf data collection into a single perf.data file rather than splitting by process
  • Moves shared data structures (UnwindData, PerfMetadata, FIFO commands) to new runner-shared crate
  • Updates the benchmark tracking mechanism to use timestamps instead of per-process ordering

Reviewed Changes

Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/run/runner/wall_time/perf/unwind_data.rs Converts UnwindData struct to trait and updates implementation to use shared crate
src/run/runner/wall_time/perf/mod.rs Major refactor removing perf data splitting, updating to single file approach with timestamp-based tracking
src/run/runner/wall_time/perf/metadata.rs Removes local metadata struct (moved to shared crate)
src/run/runner/wall_time/perf/jit_dump.rs Updates imports to use shared UnwindData
src/run/runner/wall_time/perf/helpers.rs Removes PID finding logic (no longer needed without data splitting)
src/run/runner/wall_time/perf/fifo.rs Updates to use shared FIFO constants
crates/runner-shared/* New shared crate containing common data structures
Cargo.toml Adds workspace configuration and new dependency
Various other files String formatting updates using more concise syntax

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@not-matthias not-matthias force-pushed the cod-1364-runner-sample-without-splitting-perfdata branch 2 times, most recently from bf9395d to 1cb23cc Compare September 23, 2025 13:41
@art049
Copy link
Member

art049 commented Sep 25, 2025

It's weird you had to fix the clippy issues since we have this in the recommit hooks:

      - id: clippy
        args: [--all-targets, --, -D, warnings]

Did the rust version or anything else change?

@not-matthias
Copy link
Member Author

It's weird you had to fix the clippy issues since we have this in the recommit hooks:

      - id: clippy
        args: [--all-targets, --, -D, warnings]

Did the rust version or anything else change?

Ahh, I think I'm using a newer Rust version. Should I revert the commit?

@not-matthias not-matthias force-pushed the cod-1364-runner-sample-without-splitting-perfdata branch 3 times, most recently from f3baf50 to ed702f4 Compare September 25, 2025 10:07
@not-matthias not-matthias requested a review from art049 September 25, 2025 12:43
@art049
Copy link
Member

art049 commented Sep 25, 2025

It's weird you had to fix the clippy issues since we have this in the recommit hooks:

      - id: clippy
        args: [--all-targets, --, -D, warnings]

Did the rust version or anything else change?

Ahh, I think I'm using a newer Rust version. Should I revert the commit?

Anyway we'd have to do those changes at some point so we can keep it. Are you using rustup? Normally it picks up the rust-toolchain.toml automatically

@not-matthias not-matthias force-pushed the cod-1364-runner-sample-without-splitting-perfdata branch from ed702f4 to a7f7581 Compare September 25, 2025 14:14
@not-matthias not-matthias changed the base branch from cod-1356-unresolved-addresses-with-pie-benchmark-executable to main September 25, 2025 14:14
@not-matthias not-matthias force-pushed the cod-1364-runner-sample-without-splitting-perfdata branch from a7f7581 to 4c990fd Compare September 25, 2025 14:18
@not-matthias not-matthias merged commit 4c990fd into main Sep 25, 2025
9 checks passed
@not-matthias not-matthias deleted the cod-1364-runner-sample-without-splitting-perfdata branch September 25, 2025 14:37
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