Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace odb.find_commit by gix_traverse::commit::find #1824

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

cruessler
Copy link
Contributor

@cruessler cruessler commented Feb 1, 2025

This is another performance win. The speed-up varies greatly, depending on the specific case. As two examples, I compared gix blame STABILITY.md in gitoxide and gix blame README in linux.

gitoxide on  main [$?] is 📦 v0.41.0 via 🦀 v1.84.0 took 4s
❯ hyperfine "$HOME/github/Byron/gitoxide/target/release/gix blame STABILITY.md" "$HOME/worktrees/gitoxide/branch-1/target/release/gix blame STABILITY.md"
Benchmark 1: /home/christoph/github/Byron/gitoxide/target/release/gix blame STABILITY.md
  Time (mean ± σ):     111.6 ms ±   8.1 ms    [User: 91.9 ms, System: 12.5 ms]
  Range (min … max):   102.1 ms … 142.4 ms    28 runs

Benchmark 2: /home/christoph/worktrees/gitoxide/branch-1/target/release/gix blame STABILITY.md
  Time (mean ± σ):      69.8 ms ±   5.1 ms    [User: 51.7 ms, System: 11.2 ms]
  Range (min … max):    64.2 ms …  86.5 ms    44 runs

Summary
  '/home/christoph/worktrees/gitoxide/branch-1/target/release/gix blame STABILITY.md' ran
    1.60 ± 0.17 times faster than '/home/christoph/github/Byron/gitoxide/target/release/gix blame STABILITY.md'
linux on  master [⇣]
❯ hyperfine "$HOME/github/Byron/gitoxide/target/release/gix blame README" "$HOME/worktrees/gitoxide/branch-1/target/release/gix blame README"
Benchmark 1: /home/christoph/github/Byron/gitoxide/target/release/gix blame README
  Time (mean ± σ):     422.6 ms ±   3.4 ms    [User: 365.7 ms, System: 55.5 ms]
  Range (min … max):   419.4 ms … 429.6 ms    10 runs

Benchmark 2: /home/christoph/worktrees/gitoxide/branch-1/target/release/gix blame README
  Time (mean ± σ):     408.6 ms ±   3.1 ms    [User: 352.7 ms, System: 54.4 ms]
  Range (min … max):   403.9 ms … 414.1 ms    10 runs

Summary
  '/home/christoph/worktrees/gitoxide/branch-1/target/release/gix blame README' ran
    1.03 ± 0.01 times faster than '/home/christoph/github/Byron/gitoxide/target/release/gix blame README'

@cruessler cruessler force-pushed the replace-find-commit-by-find branch from f948f12 to 2a66304 Compare February 1, 2025 19:32
@cruessler cruessler force-pushed the replace-find-commit-by-find branch from 2a66304 to e09ec3e Compare February 2, 2025 09:23
@cruessler cruessler marked this pull request as ready for review February 2, 2025 09:23
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Sorry for the late response and thanks a lot, that's a great catch!

It's interesting how huge the performance improvement is in the gitoxide repository, and how small it is in Linux, but we take everything we get :D.

@@ -666,6 +688,13 @@ fn collect_parents(
Ok(parent_ids)
}

fn tree_id(commit: gix_traverse::commit::Either<'_, '_>) -> Result<ObjectId, Error> {
Copy link
Member

Choose a reason for hiding this comment

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

I could imagine providing this function as method on Either one day, which by then could probably also be renamed in something more specific.

@Byron Byron merged commit 8ab0a6b into GitoxideLabs:main Feb 4, 2025
21 checks passed
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.

2 participants