pack-bitmap: cache object positions during fill

The previous commits removed some redundant work from bitmap generation
by avoiding unnecessary tree recursion and by reusing selected bitmaps
that have already been computed.

Even with those changes in place, there is still an extremely hot path
from `fill_bitmap_commit()` and `fill_bitmap_tree()` to translate object
IDs into their corresponding bit positions in order to generate their
bitmaps.

In a small repository, this overhead is not significant. However, in a
very large repository (e.g., the one that we have been using as a
benchmark over the past several commits with ~57M total objects), the
overhead of locating object bit positions (often repeatedly) adds up
significantly.

Combat this by adding a small, direct-mapped cache to the bitmap writer
which maps object IDs to their corresponding bit positions. Size the
cache according to the number of objects being written, with fixed lower
and upper bounds so small repositories do not pay for a large table and
large repositories can avoid most repeated packlist and MIDX lookups.

On my machine with (a somewhat outdated) GCC 15.2.0, each entry in the
cache is 40 bytes wide:

    $ pahole -C bitmap_pos_cache_entry pack-bitmap-write.o
    struct bitmap_pos_cache_entry {
            struct object_id           oid;                  /*     0    36 */
            uint32_t                   pos;                  /*    36     4 */

            /* size: 40, cachelines: 1, members: 2 */
            /* last cacheline: 40 bytes */
    };

, and we will allocate up to 2^21 entries for a maximum total of 80 MiB
of cache overhead.

In our example repository from above and in earlier commits, this
results in a ~9.4% reduction in runtime relative to the previous commit:

    +------------------+-------------+-------------+---------------------+
    |                  | HEAD^       | HEAD        | Delta               |
    +------------------+-------------+-------------+---------------------+
    | elapsed          |   324.8 s   |   294.1 s   |    -30.7 s  (-9.4%) |
    | cycles           | 1,508.6 B   | 1,365.5 B   |   -143.0 B  (-9.5%) |
    | instructions     | 1,436.6 B   | 1,389.8 B   |    -46.9 B  (-3.3%) |
    | CPI              |     1.050   |     0.983   |   -0.068    (-6.4%) |
    +------------------+-------------+-------------+---------------------+

When generating bitmaps on this repository (to produce the above
timings), the cache grew to its maximum size of 80 MiB, and resulted in
1.024B cache hits and 59.957M cache misses.

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 42ed22feacc70216db761f35a6d7766f6c3a837e..4b6fb07edd71c907d59c1e3236151bd55792c70c 100644 (file)
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -89,6 +89,7 @@ void bitmap_writer_free(struct bitmap_writer *writer)
        ewah_free(writer->tags);
 
        kh_destroy_oid_map(writer->bitmaps);
+       free(writer->pos_cache);
 
        kh_foreach_value(writer->pseudo_merge_commits, idx,
                         free_pseudo_merge_commit_idx(idx));
@@ -213,15 +214,92 @@ void bitmap_writer_push_commit(struct bitmap_writer *writer,
        writer->selected_nr++;
 }
 
+struct bitmap_pos_cache_entry {
+       struct object_id oid;
+       uint32_t pos;
+};
+
+#define BITMAP_POS_MIN_CACHE_SIZE (1U << 10)
+#define BITMAP_POS_MAX_CACHE_SIZE (1U << 21)
+#define BITMAP_POS_CACHE_VALID    (1U << 31)
+
+static void bitmap_writer_init_pos_cache(struct bitmap_writer *writer)
+{
+       if (writer->pos_cache)
+               return;
+
+       writer->pos_cache_nr = BITMAP_POS_MIN_CACHE_SIZE;
+
+       while (writer->pos_cache_nr < writer->to_pack->nr_objects &&
+              writer->pos_cache_nr < BITMAP_POS_MAX_CACHE_SIZE)
+               writer->pos_cache_nr <<= 1;
+
+       CALLOC_ARRAY(writer->pos_cache, writer->pos_cache_nr);
+}
+
+static size_t bitmap_writer_pos_cache_slot(struct bitmap_writer *writer,
+                                          const struct object_id *oid)
+{
+       return oidhash(oid) & (writer->pos_cache_nr - 1);
+}
+
+static bool bitmap_writer_pos_cache_valid(struct bitmap_writer *writer,
+                                         size_t slot)
+{
+       return !!(writer->pos_cache[slot].pos & BITMAP_POS_CACHE_VALID);
+}
+
+static int find_cached_object_pos(struct bitmap_writer *writer,
+                                 const struct object_id *oid, uint32_t *pos)
+{
+       size_t slot = bitmap_writer_pos_cache_slot(writer, oid);
+
+       if (bitmap_writer_pos_cache_valid(writer, slot) &&
+           oideq(&writer->pos_cache[slot].oid, oid)) {
+               writer->pos_cache_hits++;
+               *pos = writer->pos_cache[slot].pos & ~BITMAP_POS_CACHE_VALID;
+               return 1;
+       }
+
+       writer->pos_cache_misses++;
+       return 0;
+}
+
+static uint32_t store_cached_object_pos(struct bitmap_writer *writer,
+                                       const struct object_id *oid,
+                                       uint32_t pos)
+{
+       size_t slot;
+
+       if (pos & BITMAP_POS_CACHE_VALID)
+               return pos; /* too large to cache */
+
+       slot = bitmap_writer_pos_cache_slot(writer, oid);
+
+       oidcpy(&writer->pos_cache[slot].oid, oid);
+       writer->pos_cache[slot].pos = pos | BITMAP_POS_CACHE_VALID;
+
+       return pos;
+}
+
 static uint32_t find_object_pos(struct bitmap_writer *writer,
                                const struct object_id *oid, int *found)
 {
        struct object_entry *entry;
        uint32_t pos;
 
+       bitmap_writer_init_pos_cache(writer);
+
+       if (find_cached_object_pos(writer, oid, &pos)) {
+               if (found)
+                       *found = 1;
+               return pos;
+       }
+
        entry = packlist_find(writer->to_pack, oid);
        if (entry) {
                uint32_t base_objects = 0;
+
                if (writer->midx)
                        base_objects = writer->midx->num_objects +
                                writer->midx->num_objects_in_base;
@@ -239,7 +317,7 @@ static uint32_t find_object_pos(struct bitmap_writer *writer,
 
        if (found)
                *found = 1;
-       return pos;
+       return store_cached_object_pos(writer, oid, pos);
 
 missing:
        if (found)
@@ -662,6 +740,10 @@ int bitmap_writer_build(struct bitmap_writer *writer)
                writer->progress = start_progress(writer->repo,
                                                  "Building bitmaps",
                                                  writer->selected_nr);
+
+       writer->pos_cache_hits = 0;
+       writer->pos_cache_misses = 0;
+
        trace2_region_enter("pack-bitmap-write", "building_bitmaps_total",
                            writer->repo);
 
@@ -726,6 +808,10 @@ int bitmap_writer_build(struct bitmap_writer *writer)
        trace2_data_intmax("pack-bitmap-write", writer->repo,
                           "fill_bitmap_commit_found_ancestor_nr",
                           fill_bitmap_commit_found_ancestor_nr);
+       trace2_data_intmax("pack-bitmap-write", writer->repo,
+                          "bitmap_pos_cache_hits", writer->pos_cache_hits);
+       trace2_data_intmax("pack-bitmap-write", writer->repo,
+                          "bitmap_pos_cache_misses", writer->pos_cache_misses);
 
        stop_progress(&writer->progress);
 

diff --git a/pack-bitmap.h b/pack-bitmap.h
index a95e1c2d115a31d45473758852806c3a1186b9a8..19a86554579f7c905c876d55f216f05ccc88e833 100644 (file)
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -132,6 +132,8 @@ int bitmap_has_oid_in_uninteresting(struct bitmap_index *, const struct object_i
 
 off_t get_disk_usage_from_bitmap(struct bitmap_index *, struct rev_info *);
 
+struct bitmap_pos_cache_entry;
+
 struct bitmap_writer {
        struct repository *repo;
        struct ewah_bitmap *commits;
@@ -143,6 +145,11 @@ struct bitmap_writer {
        struct packing_data *to_pack;
        struct multi_pack_index *midx; /* if appending to a MIDX chain */
 
+       struct bitmap_pos_cache_entry *pos_cache;
+       size_t pos_cache_nr;
+       uint64_t pos_cache_hits;
+       uint64_t pos_cache_misses;
+
        struct bitmapped_commit *selected;
        unsigned int selected_nr, selected_alloc;