Skip to content

bench: Add benchmarks for revset graph#9025

Open
ThierryBerger wants to merge 1 commit intojj-vcs:mainfrom
ThierryBerger:benchmark-log
Open

bench: Add benchmarks for revset graph#9025
ThierryBerger wants to merge 1 commit intojj-vcs:mainfrom
ThierryBerger:benchmark-log

Conversation

@ThierryBerger
Copy link

This adds simple, isolated and reproducible benchmarks for revsets.

Context

Cloning a big repository to test performance is cumbersome, difficult to compare baseline for, and not intuitive.

From what I understand, jj bench log is what's used right now by jj developers, with https://github.com/jj-vcs/jj/blob/main/cli/testing/bench-revsets-git.txt being a suggestion of revset to pass to. This file is regularly outdated so using it wasn't very fun.

Details on this PR

  • This builds on what exists on this repository, within lib/benches. It seems a bit underused/old, but still on the repository so I'm hopeful that the design is still relevant.
  • No additional dependencies
  • Small by design, I want a baseline performance comparison for future discussions on Fix #3099: count number of elided revisions #8968, or other topics.
  • Future possibilities: This is a simpler setup to run on a CI: cargo bench

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added/updated tests to cover my changes
  • I fully understand the code that I am submitting (what it does,
    how it works, how it's organized), including any code drafted by AI
  • For any prose generated by AI, I have proof-read and copy-edited with an
    eye towards deleting anything that is irrelevant, clarifying anything that
    is confusing, and adding details that are relevant. This includes, for
    example, commit descriptions, PR descriptions, and code comments.

@ThierryBerger ThierryBerger requested a review from a team as a code owner March 4, 2026 21:53
Copy link
Contributor

@PhilipMetzger PhilipMetzger left a comment

Choose a reason for hiding this comment

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

Please expand the motivation in the commit description not on the PR page since the project cares more about individual commits instead of PR descriptions.

@ThierryBerger ThierryBerger force-pushed the benchmark-log branch 2 times, most recently from 9180c77 to 2034129 Compare March 5, 2026 06:11
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines +54 to +59
let index: &DefaultReadonlyIndex = repo.readonly_index().downcast_ref().unwrap();
let expression =
ResolvedExpression::Commits(vec![root_visible.id().clone(), tip_visible.id().clone()]);
let revset = index
.evaluate_revset_impl(&expression, repo.store())
.unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Should we use the external API instead here? I suppose it adds a little bit of overhead, but the timing of the internal API doesn't really matter anyway: if we make changes to the default revset engine that impact performance in a small enough way that it's drowned out by the overhead of the external API, then the performance impact doesn't matter anyway (whether it's positive or negative).

(same applies to the other benchmark below)

Copy link
Author

@ThierryBerger ThierryBerger Mar 6, 2026

Choose a reason for hiding this comment

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

Indeed, I'm not too familiar with the API, I changed it to use:

  • RevsetExpression::commits
  • revset.iter_graph()
  • TopoGroupedGraphIterator

I can't use directly log because it's in the cli crate.

Edit: I consider this resolved but I think I shouldn't be the one resolving reviews, just a heads up since that's blocking CI

This adds simple, isolated and reproducible benchmarks for revsets.

## Context

- While working on jj-vcs#8968 ; performance was mentioned, as well as trying impacts on big repositories such as linux.

Cloning a big repository to test performance is cumbersome, difficult to compare baseline for, and not intuitive.

From what I understand, `jj bench log` is what's used right now by jj developers, with https://github.com/jj-vcs/jj/blob/main/cli/testing/bench-revsets-git.txt being a suggestion of revset to pass to. This file is regularly outdated so using it wasn't very fun.

## Details on this PR

- This builds on what exists on this repository, within `lib/benches`. It seems a bit underused/old, but still on the repository so I'm hopeful that the design is still relevant.
- No additional dependencies
- Small by design, I want a baseline performance comparison for future discussions on jj-vcs#8968, or other topics.
- Future possibilities: This is a simpler setup to run on a CI: `cargo bench`
@yuja
Copy link
Contributor

yuja commented Mar 6, 2026

Somewhat related: I think it's also good to add --graph option to jj bench revset. It'll be more interesting to run benchmarks against real-word repositories.

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.

4 participants