Repository navigation
Performance regression in gix-blame #2148
Description
gix blame on tag gix-blame-v0.1.0 is much faster than on main. The main difference is the usage of zlib-ng than zlib-rs.
❯ hyperfine './gix-main blame README.md' './gix-0.1 blame README.md' 'git blame README.md'
Benchmark 1: ./gix-main blame README.md
Time (mean ± σ): 157.4 ms ± 0.6 ms [User: 149.5 ms, System: 5.9 ms]
Range (min … max): 156.1 ms … 158.4 ms 18 runs
Benchmark 2: ./gix-0.1 blame README.md
Time (mean ± σ): 50.9 ms ± 0.5 ms [User: 41.7 ms, System: 6.9 ms]
Range (min … max): 50.0 ms … 52.6 ms 56 runs
Benchmark 3: git blame README.md
Time (mean ± σ): 52.1 ms ± 0.7 ms [User: 43.7 ms, System: 6.3 ms]
Range (min … max): 49.9 ms … 53.2 ms 54 runs
Summary
./gix-0.1 blame README.md ran
1.02 ± 0.02 times faster than git blame README.md
3.10 ± 0.03 times faster than ./gix-main blame README.md
Before we jump to conclusions, I may also note that this could be related to the object caching being less effective or disabled for some reason. Also, the traversal algorithm changed and maybe it does more work.
Maybe a good first check would be to bring back zlib-ng for a test-run on main.
diff --git a/gix-features/Cargo.toml b/gix-features/Cargo.toml
index a955c1f2f0..7996f9d7fe 100644
--- a/gix-features/Cargo.toml
+++ b/gix-features/Cargo.toml
@@ -121,7 +121,7 @@ bytesize = { version = "2.0.1", optional = true }
bytes = { version = "1.0.0", optional = true }
# zlib module
-flate2 = { version = "1.1.1", optional = true, default-features = false, features = ["zlib-rs"] }
+flate2 = { version = "1.1.1", optional = true, default-features = false, features = ["zlib-ng"] }
thiserror = { version = "2.0.0", optional = true }
once_cell = { version = "1.21.3", optional = true }
Turns out that zlib-ng is slower than zlib-rs, and that the ZLIB backend is only hit more in main, it's not slower.
Activity
Byron commentedon Sep 1, 2025
CC Christoph Rüßler (@cruessler) - I recommend to bisect it.
cruessler commentedon Sep 2, 2025
For the record, the results of bisecting and running
gix blame README.mdat each step.96164c5 is the commit that switched
gitoxidetozlib-rs. While this finding explains my initial observations, it still somehow feels wrong as that switch was supposed to make everything faster. :-)Details
8d8dba212-main4f2717960f965540c149fa9f38146227e6d1aa91f9ae4633f0a86b96164c593f3684a4f6d139fa469b41312b47-gix-blame-v0.1.0Byron commentedon Sep 2, 2025
The strange thing is that I have no idea how that can be, i.e. when reverting 96164c5 I think performance actually went down, i.e.
zlib-ngis slightly slower at the same task.Is this something you could validate? Alternatively, the patch above can be used as well, but I'd recommend to double-check everything.
Thanks again for your help with this!
cruessler commentedon Sep 3, 2025
I reverted 96164c5 and performance went back to roughly as fast as it was before. My changes are in this commit cruessler@065d578, on a branch called
bisect-gix-blame-performance:The only difference to the state pre 96164c5 is that I had to go back to
zip2.4.2 because the version originally used pre 96164c5 (2.6.1) had been yanked in the meantime.Next, I’m going to try and figure out if there’s a more specific change responsible, such as a feature toggle or a dependency upgrade.
Byron commentedon Sep 3, 2025
Alright, if bisect wasn't enough, this is clear proof that the
zlib-rsintroduction had this effect.It's impossible that it's this slow, I would have noticed, so this is still puzzling to me.
But the results could be more clear on AARCH64 either.
Could we get a quick sanity-check from Folkert de Vries (@folkertdev) about cruessler@065d578 - how can it possibly be that it's so much slower here if my verify-pack tests (which decode a lot of small packed objects in an optimised fashion) come out faster than
zlib-ng?I am still puzzled about this, and the factor of 3.3x makes no sense.
Thanks so much!
folkertdev commentedon Sep 3, 2025
Hmm, it does look like zlib-rs is slower here, but I can't reproduce your 3x numbers. I get
This is on f7bfff4, I've compiled with
cargo build --bin gix --profile=bench. This is on linux/x86_64. Has all of your profiling been on aarch64/macos?The workload here appears to consist of many tiny invocations. I'll look into updating our benchmarks with such a workload.
Byron commentedon Sep 4, 2025
Thanks so much!
It's good to know that this doesn't seem to be happening everywhere.
My testing has been on MacOS/aarch64 always, but Christoph Rüßler (@cruessler) may also have been on linux/x86-64 if I remember correctly, seeing differences of 2x (please correct me though).
Yes, this is what happens when the packed entries are decoded, and that's particularly intense when verifying packs after cloning for instance. However, that is what I tested before switching to
zlib-rs, and even though it might be a little slower under these conditions, ultimately it was a little faster overall. There was no issue, particularly not a 3x one.My test-runs were with
--releaseonly, maybe that's related.Just to be sure, I re-ran with
--profile=benchand have the same result.The version I use for
zlib-ngisn't the very latest, that shoudln't be material (it's cruessler@065d578).cruessler commentedon Sep 4, 2025
Thanks a lot!
My numbers all come from linux/x86_64 (but I was able to confirm the rough magnitude of the effect on macos/aarch64).
folkertdev commentedon Sep 4, 2025
I've tried to isolate the pattern here
trifectatechfoundation/zlib-rs#411
could you give that branch/example a go and see if that still reproduces the 3X difference? If so, I truly have no idea what is happening there. I've tried a bunch of rust versions (hence LLVM versions), but that does not help much.
On my machine that example is 6% slower with zlib-rs versus zlib-ng.
So we'll have to shave that down. It's a pattern of input we haven't really looked at, so I'm assuming we'll be able to get on-par performance here.
cruessler commentedon Sep 4, 2025
I ran your example on my linux/x86_64 machine. The difference is almost completely gone:
In case that matters, my machine’s already a bit older, running an Intel(R) Core(TM) i5-4570 CPU from around 2013.
Byron commentedon Sep 4, 2025
Thanks so much! On my machine, the 3x result doesn't reproduce at all, and to add some spice,
zlib-rsis a whopping 13% faster:The question is if this is still equivalent to the issue we are measuring here as well.
folkertdev commentedon Sep 4, 2025
Interesting. and weird.
That example file uses the first 100 inputs to zlib-{rs, ng} that
gixpasses. Apparently that is not representative, but using much more input is kind of cumbersome (e.g. the file with the inputs is already 5kloc).I guess I'll see if I can capture the longest-running inputs from that
gix blamerun, or is there some other pattern in the data that you think might be interesting (e.g. are the first or last inputs larger/smaller or harder/easier to decompress)?Byron commentedon Sep 4, 2025
I really like this approach! Somehow I have trouble imagining that multiple sequential inputs influence to the point where there is such a slowdown, but at least in theory it's possible as there is shared state:
gitoxide/gix-features/src/zlib/mod.rs
Lines 34 to 43 in beba720
This method is used for decoding packs, which is where many small inputs are coming from.
gitoxide/gix-pack/src/data/file/decode/entry.rs
Lines 119 to 132 in fce7095
I see that the reset is also happening in the minimised example that doesn't go through
flate2, so I don't know what else it could be except for some phenomenal miscompilation. Maybe you already ruled that out though.folkertdev commentedon Sep 4, 2025
I used this hack to get at all (93k!) inflate invocations of
gix blame README.md, and put them linearly in a file:I also noticed that the
flushparameter was different (I usedFinish, you useNone). The PR now uses this file and runs through that a couple of times. I get roughly the same numbers though.If you see the same, it's not the actual algorithm. Something I did notice is that creation of a new
Decompressis a lot more expensive with zlib-rs. That is because we construct a large struct on the stack and then move it to the heap. zlib-ng instead uses uninitialized memory. From what I can see you reuse the state (a good idea anyway), but it might be something like that then.Byron commentedon Sep 5, 2025
Amazing, thanks!
Like your results, mine are still very similar, but now even more in favor of
zlib-rs, with 22% faster.It's notable that the
Inflatestructure is also re-used ingitoxide. Even if it wasn't, I am pretty sure that it wouldn't be responsible for a 3x difference.In any case, thanks to this experiment we know that it's not
zlib-rswhich is at fault.Now I wonder… should we attempt a fix by using
zlib-rsdirectly?flate2serves no purpose anymore. Is this something you could hack together? And I really mean: Hack, as PoC, and I could take it from there if it solves the performance problem whereever it is coming from?Because I have a feeling it just might - the commit in question only deals with features of
flate2, so not usingflate2should fix it, right?folkertdev commentedon Sep 5, 2025
Sure, I hacked something together here main...folkertdev:gitoxide:skip-flate2. Flate2 disappears from the flamegraph, though for me there is no change in performance (nor does one seem likely, having read the flate2 source).
Byron commentedon Sep 6, 2025
Thank you! I think I'd like to keep this branch just so that all the
zlibrelated features can be removed in favor of this one implementation. It's low-level, but that's exactly whatgixneeds.Besides that, your prediction holds, and the performance numbers aren't different. This means nothing related to
flate2seems to be causing this.Then… can it be that we have been accidentally comparing apples to oranges? I know that I have compared whatever version of
gix blamethere was at 96164c5 with the latest version inmain. Christoph Rüßler (@cruessler) most certainly didn't do that, comparing that version with the one before, but I certainly have been… messy to the point where I am not sure the major change between these versions is notgix blameitself.So I will go back to step 1 to be sure I compare the right commits with an otherwise unchanged
gix blameversion, and then profile these just to be sure. If all this makes no sense, then probably the error sits in front of the computer.folkertdev commentedon Sep 6, 2025
I did some further benchmarking, and I'm pretty sure that there is something in #1963 that causes the difference besides switching to
zlib-rs. Going back to its base commit f3684a4 and just flipping the switchLeads for me to a 5% degradation in performance. But switching to 96164c5 it's an over 30% difference.
folkertdev commentedon Sep 6, 2025
got it, the cause is this change
that seems correct because
but in fact
max-performance-safeenables a bunch of things itself.fix `gix-blame` performance regresion
Byron commentedon Sep 6, 2025
When trying to bring this commit in I noticed that it probably also needs an implementation to compress bytes. Is this easy for you so you could contribute a
Compressimplementation as well? Maybe I could also make it happen, but since it'sunsafeI'd be extra-afraid.folkertdev commentedon Sep 6, 2025
Sure, I'll put something together (I also noticed I missed a
Dropimplementation here)Byron commentedon Sep 7, 2025
That's great, thanks so much!
When merging #2155, I also validated that the implementation is just as fast as the previous one that went through the
flate2layer, so it's all great!Finally, blame is basically just as fast, with the potential for being an inkling faster. But maybe these are just inaccuracies in the measurements.
With that, I think the whole topic is a wrap :).
fix `gix-blame` performance regresion