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>

diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 2d5ff8fd406db96247b0888014be9847e570ab21..726103970206643e64c9853835ecbf38d0f3aefa 100644 (file)
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -463,12 +463,6 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
        struct tree_desc desc;
        struct name_entry entry;
 
-       /*
-        * If our bit is already set, then there is nothing to do. Both this
-        * tree and all of its children will be set.
-        */
-       if (bitmap_get(bitmap, pos))
-               return 0;
        bitmap_set(bitmap, pos);
 
        if (repo_parse_tree(writer->repo, tree) < 0)
@@ -482,6 +476,15 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
                        pos = find_object_pos(writer, &entry.oid, &found);
                        if (!found)
                                return -1;
+                       if (bitmap_get(bitmap, pos)) {
+                               /*
+                                * If our bit is already set, then there
+                                * is nothing to do. Both this tree and
+                                * all of its children will be set.
+                                */
+                               break;
+                       }
+
                        if (fill_bitmap_tree(writer, bitmap,
                                             lookup_tree(writer->repo,
                                                         &entry.oid), pos) < 0)
@@ -582,6 +585,14 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
                pos = find_object_pos(writer, &t->object.oid, &found);
                if (!found)
                        return -1;
+               if (bitmap_get(ent->bitmap, pos)) {
+                       /*
+                        * If our bit is already set, then there is
+                        * nothing to do. Both this tree and all of its
+                        * children will be set.
+                        */
+                       continue;
+               }
 
                if (fill_bitmap_tree(writer, ent->bitmap, t, pos) < 0)
                        return -1;