Skip to content

Conversation

folkertdev
Copy link
Contributor

fixes #2148

The issue was introduced in #1963, where it was incorrectly assumed that the max-performance-safe feature flag was only used to enable other feature flags. But this feature flag is itself used in various places, so failing to enable it leads to worse codegen. This issue was discussed/found in #2148

The issue was introduced in GitoxideLabs#1963, and incorrectly assumed that the `max-performance-safe` feature flag was only used to enable other feature flags. But this feature flag is itself used in various places, so failing to enable it leads to worse codegen. This issue was discussed/found in GitoxideLabs#2148
@cruessler
Copy link
Contributor

I just ran the benchmark and can confirm that this fixes the performance regression on my linux/x86_64 machine! Thanks a lot!

# this PR
❯ hyperfine 'target/release/gix blame README.md'
Benchmark 1: target/release/gix blame README.md
  Time (mean ± σ):     164.2 ms ±   1.6 ms    [User: 135.1 ms, System: 28.2 ms]
  Range (min … max):   161.8 ms … 168.4 ms    17 runs

# the commit before the one that introduced the regression
❯ hyperfine 'bin/gix-f3684a4f6 blame README.md'
Benchmark 1: bin/gix-f3684a4f6 blame README.md
  Time (mean ± σ):     161.5 ms ±   1.8 ms    [User: 129.6 ms, System: 30.9 ms]
  Range (min … max):   158.5 ms … 164.9 ms    18 runs

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.

Thanks SO much for figuring this out.
And I am so sorry I let that pass through the first time around.

In any case, I think digging into this also helped us gain something:

  • zlib-rs, depending on the platform, can be slower when handling many small objects (even though gix doesn't notice this on MacOS at least). Now there is a test for that as a first step towards improving this.
  • zlib-rs can be used directly, and the flate2 dependency can be removed - I will see to bringing in your commit soon.

@Byron
Copy link
Member

Byron commented Sep 6, 2025

And indeed, the natural order has been restored.

❯ hyperfine './gix-8d8dba2-bench-zlib-ng blame README.md' './gix-max-perf-safe blame README.md'
Benchmark 1: ./gix-8d8dba2-bench-zlib-ng blame README.md
  Time (mean ± σ):      47.1 ms ±   1.0 ms    [User: 36.9 ms, System: 7.6 ms]
  Range (min … max):    45.6 ms …  51.9 ms    61 runs

Benchmark 2: ./gix-max-perf-safe blame README.md
  Time (mean ± σ):      46.8 ms ±   3.0 ms    [User: 36.3 ms, System: 7.7 ms]
  Range (min … max):    45.0 ms …  69.2 ms    61 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Summary
  ./gix-max-perf-safe blame README.md ran
    1.01 ± 0.07 times faster than ./gix-8d8dba2-bench-zlib-ng blame README.md

@Byron Byron merged commit 768164a into GitoxideLabs:main Sep 6, 2025
25 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.

Performance regression in gix-blame
3 participants