Skip to content

Conversation

@cpcloud
Copy link
Contributor

@cpcloud cpcloud commented Dec 3, 2025

Add a tool to compare benchmarks on two commits. Run with pixi run bench-against

@copy-pr-bot
Copy link

copy-pr-bot bot commented Dec 3, 2025

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@gmarkall
Copy link
Contributor

gmarkall commented Dec 3, 2025

/ok to test

@gmarkall gmarkall added the 3 - Ready for Review Ready for review by team label Dec 3, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 3, 2025

Greptile Overview

Greptile Summary

This PR adds a benchmark comparison tool that allows developers to compare performance between two git commits without modifying the current working directory.

  • Introduces scripts/bench-against.py which uses git worktrees to safely check out and benchmark two commits
  • Adds a new bench-against pixi environment with test dependencies for running the tool
  • Adds pixi run bench-against task (Linux only) to invoke the script
  • Suppresses PytestBenchmarkWarning about differing machine_info when comparing benchmarks across commits

The tool takes two required arguments (baseline and proposed git refs) and an optional environment flag (defaults to cu-12-9-py312).

Confidence Score: 4/5

  • This PR is safe to merge - it adds a new developer tool without modifying existing functionality.
  • The implementation is clean and uses git worktrees correctly with proper cleanup. The tool is isolated (new files and environment only) and doesn't affect existing code paths. Minor risk is around edge cases in error handling during worktree operations.
  • No files require special attention - the main script is straightforward and follows good practices.

Important Files Changed

File Analysis

Filename Score Overview
scripts/bench-against.py 4/5 New benchmark comparison script using git worktrees. Creates temporary worktree to check out baseline and proposed commits, runs benchmarks on each, and compares results. Clean implementation with proper cleanup on exit.
pixi.toml 5/5 Adds new bench-against environment with test features and task to run the benchmark comparison script.
testing/pytest.ini 5/5 Adds filter to ignore PytestBenchmarkWarning about differing machine_info when comparing benchmarks across commits.
pixi.lock 5/5 Lock file updates for new bench-against environment dependencies across linux-64, linux-aarch64, and win-64 platforms.

Sequence Diagram

sequenceDiagram
    participant User
    participant Script as bench-against.py
    participant Git
    participant Pixi
    participant Pytest

    User->>Script: pixi run bench-against <baseline> <proposed>
    Script->>Git: rev-parse --show-toplevel
    Git-->>Script: git root directory
    Script->>Git: worktree add --detach <temp-path> HEAD
    Script->>Script: chdir(worktree_path)
    
    Note over Script: Baseline benchmarks
    Script->>Git: checkout <baseline>
    Script->>Pixi: reinstall -e <env> numba-cuda
    Script->>Pixi: run -e <env> bench
    Pixi->>Pytest: benchmark tests (autosave results)
    
    Note over Script: Proposed benchmarks
    Script->>Git: checkout <proposed>
    Script->>Pixi: reinstall -e <env> numba-cuda
    Script->>Pixi: run -e <env> benchcmp
    Pixi->>Pytest: benchmark tests (compare with saved)
    Pytest-->>User: Comparison results
    
    Script->>Git: worktree remove --force <temp-path>
    Script->>Script: chdir(original_dir)
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@cpcloud
Copy link
Contributor Author

cpcloud commented Dec 3, 2025

I am not sure why pixi thinks the lockfile isn't up-to-date.

@cpcloud cpcloud force-pushed the benchmark-comparison-tool branch from 7866553 to 7ce9e50 Compare December 3, 2025 17:02
@cpcloud
Copy link
Contributor Author

cpcloud commented Dec 3, 2025

/ok to test

@cpcloud cpcloud requested a review from gmarkall December 3, 2025 17:03
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@cpcloud cpcloud force-pushed the benchmark-comparison-tool branch 2 times, most recently from 8108cea to 5f0c7f8 Compare December 3, 2025 17:32
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@cpcloud
Copy link
Contributor Author

cpcloud commented Dec 3, 2025

@gmarkall I am happy to add a smoke test for this if you'd like, but I'd like to use it on #600 before doing so if that's okay.

@cpcloud cpcloud force-pushed the benchmark-comparison-tool branch from 61ec608 to e88031c Compare December 3, 2025 17:52
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@cpcloud cpcloud force-pushed the benchmark-comparison-tool branch from e88031c to 10ff7c9 Compare December 3, 2025 17:56
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

This works for me - many thanks!

@gmarkall
Copy link
Contributor

gmarkall commented Dec 4, 2025

/ok to test

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@gmarkall
Copy link
Contributor

gmarkall commented Dec 4, 2025

/ok to test

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@gmarkall gmarkall removed the 3 - Ready for Review Ready for review by team label Dec 4, 2025
@gmarkall gmarkall added 4 - Waiting on CI Waiting for a CI run to finish successfully 5 - Ready to merge Testing and reviews complete, ready to merge and removed 4 - Waiting on CI Waiting for a CI run to finish successfully labels Dec 4, 2025
@gmarkall gmarkall merged commit b53640f into NVIDIA:main Dec 4, 2025
71 checks passed
@cpcloud cpcloud deleted the benchmark-comparison-tool branch December 4, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Testing and reviews complete, ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants