Re: [PATCH] pack-bitmap: remove trace2 region from hot path
From: Junio C Hamano <hidden>
Date: 2022-09-23 17:38:14
"Derrick Stolee via GitGitGadget" [off-list ref] writes:
I noticed this while trying to backport the Abhradeep's lookup table
work into GitHub's fork. Something went wrong in that process, causing
this region to significantly slow down. It turns out that slow down does
not reproduce on current 'master', which is good. I must have missed
something while I was backporting.
Regardless, the use of trace2_region_enter() here should be removed or
replaced. For the sake of 2.38.0, this simple removal is safe enough.Well, not doing this removal is even safer, if we know that it is not hurting in the -rc code. It would be better than breaking our tests without knowing ;-) I am not upset that the patch broke the test, by the way. Compared to things silently breaking without surfacing, a loud breakage in tests is a much easier thing to handle. My test run with the attached just finished, so that's what I am going to queue tentatively on top. Thanks. pack-bitmap.c | 1 + t/t5310-pack-bitmaps.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git c/pack-bitmap.c w/pack-bitmap.c
index 13457dd77e..a2bbbbd811 100644
--- c/pack-bitmap.c
+++ w/pack-bitmap.c@@ -830,6 +830,7 @@ struct ewah_bitmap *bitmap_for_commit(struct bitmap_index *bitmap_git, if (!bitmap_git->table_lookup) return NULL; + /* this is a fairly hot codepath - no trace2_region please */ /* NEEDSWORK: cache misses aren't recorded */ bitmap = lazy_bitmap_for_commit(bitmap_git, commit); if (!bitmap)
diff --git c/t/t5310-pack-bitmaps.sh w/t/t5310-pack-bitmaps.sh
index 7e50f8e765..2b1efc923b 100755
--- c/t/t5310-pack-bitmaps.sh
+++ w/t/t5310-pack-bitmaps.sh@@ -455,7 +455,7 @@ test_expect_success 'verify writing bitmap lookup table when enabled' ' grep "\"label\":\"writing_lookup_table\"" trace2 ' -test_expect_success 'lookup table is actually used to traverse objects' ' +: test_expect_success 'lookup table is actually used to traverse objects' ' git repack -adb && GIT_TRACE2_EVENT="$(pwd)/trace3" \ git rev-list --use-bitmap-index --count --all &&