@matheusbsbs / matheus-bagliano / commits / 1760c37258

pack-bitmap: check subtree bits before recursing

In the previous commit, we adjusted the callers of `fill_bitmap_tree()` to pass in the bit position of the tree they wish to fill. This commit makes use of that information at the call site to avoid setting up a stack frame for fill_bitmap_tree() entirely whenever a tree's bit position is already set. Since this is such a hot path, the avoided cost of setting up and tearing down stack frames for each noop'd call to `fill_bitmap_tree()` is significant: +--------------+-------------+-------------+-------------------+ | | HEAD^ | HEAD | Delta | +--------------+-------------+-------------+-------------------+ | elapsed | 582.4 s | 562.8 s | -19.6 s (-3.4%) | | cycles | 2,713.3 B | 2,621.3 B | -92.0 B (-3.4%) | | instructions | 2,415.5 B | 2,348.9 B | -66.6 B (-2.8%) | | CPI | 1.123 | 1.116 | -0.007 (-0.7%) | +--------------+-------------+-------------+-------------------+ In the same repository as in the previous commit, our timings dropped from ~582.4 seconds down to ~562.77 seconds. While the cycles-per-instruction ratio is basically unchanged, we execute significantly fewer instructions, and correspondingly fewer cycles. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed May 27, 2026 at 15:55 UTC 1760c372589af09ff0b986c57bfe0b9101275674

1 file changed +17 -6

pack-bitmap-write.c

+17 -6

@@ -463,12 +463,6 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,

463 struct tree_desc desc;

464 struct name_entry entry;

465

466 - /*

467 - * If our bit is already set, then there is nothing to do. Both this

468 - * tree and all of its children will be set.

469 - */

470 - if (bitmap_get(bitmap, pos))

471 - return 0;

466 bitmap_set(bitmap, pos);

467

468 if (repo_parse_tree(writer->repo, tree) < 0)

@@ -482,6 +476,15 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,

476 pos = find_object_pos(writer, &entry.oid, &found);

477 if (!found)

478 return -1;

479 + if (bitmap_get(bitmap, pos)) {

480 + /*

481 + * If our bit is already set, then there

482 + * is nothing to do. Both this tree and

483 + * all of its children will be set.

484 + */

485 + break;

486 + }

487 +

488 if (fill_bitmap_tree(writer, bitmap,

489 lookup_tree(writer->repo,

490 &entry.oid), pos) < 0)

@@ -582,6 +585,14 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,

585 pos = find_object_pos(writer, &t->object.oid, &found);

586 if (!found)

587 return -1;

588 + if (bitmap_get(ent->bitmap, pos)) {

589 + /*

590 + * If our bit is already set, then there is

591 + * nothing to do. Both this tree and all of its

592 + * children will be set.

593 + */

594 + continue;

595 + }

596

597 if (fill_bitmap_tree(writer, ent->bitmap, t, pos) < 0)

598 return -1;