@matheusbsbs / matheus-bagliano / commits / 20b031fede

packfile: add kept-pack cache for find_kept_pack_entry()

In a recent patch we added a function 'find_kept_pack_entry()' to look for an object only among kept packs. While this function avoids doing any lookup work in non-kept packs, it is still linear in the number of packs, since we have to traverse the linked list of packs once per object. Let's cache a reduced version of that list to save us time. Note that this cache will last the lifetime of the program. We could invalidate it on reprepare_packed_git(), but there's not much point in being rigorous here: - we might already fail to notice new .keep packs showing up after the program starts. We only reprepare_packed_git() when we fail to find an object. But adding a new pack won't cause that to happen. Somebody repacking could add a new pack and delete an old one, but most of the time we'd have a descriptor or mmap open to the old pack anyway, so we might not even notice. - in pack-objects we already cache the .keep state at startup, since 56dfeb6263 (pack-objects: compute local/ignore_pack_keep early, 2016-07-29). So this is just extending that concept further. - we don't have to worry about any packed_git being removed; we always keep the old structs around, even after reprepare_packed_git() We do defensively invalidate the cache in case the set of kept packs being asked for changes (e.g., only in-core kept packs were cached, but suddenly the caller also wants on-disk kept packs, too). In theory we could build all three caches and switch between them, but it's not necessary, since this patch (and series) never changes the set of kept packs that it wants to inspect from the cache. So that "optimization" is more about being defensive in the face of future changes than it is about asking for multiple kinds of kept packs in this patch. Here are p5303 results (as always, measured against the kernel): Test HEAD^ HEAD ----------------------------------------------------------------------------------------------- 5303.5: repack (1) 57.34(54.66+10.88) 56.98(54.36+10.98) -0.6% 5303.6: repack with kept (1) 57.38(54.83+10.49) 57.17(54.97+10.26) -0.4% 5303.11: repack (50) 71.70(88.99+4.74) 71.62(88.48+5.08) -0.1% 5303.12: repack with kept (50) 72.58(89.61+4.78) 71.56(88.80+4.59) -1.4% 5303.17: repack (1000) 217.19(491.72+14.25) 217.31(490.82+14.53) +0.1% 5303.18: repack with kept (1000) 246.12(520.07+14.93) 217.08(490.37+15.10) -11.8% and the --stdin-packs case, which scales a little bit better (although not by that much even at 1,000 packs): 5303.7: repack with --stdin-packs (1) 0.00(0.00+0.00) 0.00(0.00+0.00) = 5303.13: repack with --stdin-packs (50) 3.43(11.75+0.24) 3.43(11.69+0.30) +0.0% 5303.19: repack with --stdin-packs (1000) 130.50(307.15+7.66) 125.13(301.36+8.04) -4.1% Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 22, 2021 at 21:25 UTC 20b031fedeeb6e8df428b6b7cdc05c14375cd1e1
2 files changed +61 -43
object-store.h
+5
@@ -153,6 +153,11 @@ struct raw_object_store {
153 /* A most-recently-used ordered version of the packed_git list. */
154 struct list_head packed_git_mru;
155
156 + struct {
157 + struct packed_git **packs;
158 + unsigned flags;
159 + } kept_pack_cache;
160 +
161 /*
162 * A map of packfiles to packed_git structs for tracking which
163 * packs have been loaded already.
packfile.c
+56 -43
@@ -2042,10 +2042,7 @@ static int fill_pack_entry(const struct object_id *oid,
2042 return 1;
2043 }
2044
2045 -static int find_one_pack_entry(struct repository *r,
2046 - const struct object_id *oid,
2047 - struct pack_entry *e,
2048 - int kept_only)
2045 +int find_pack_entry(struct repository *r, const struct object_id *oid, struct pack_entry *e)
2046 {
2047 struct list_head *pos;
2048 struct multi_pack_index *m;
@@ -2055,49 +2052,63 @@ static int find_one_pack_entry(struct repository *r,
2052 return 0;
2053
2054 for (m = r->objects->multi_pack_index; m; m = m->next) {
2058 - if (!fill_midx_entry(r, oid, e, m))
2059 - continue;
2060 -
2061 - if (!kept_only)
2062 - return 1;
2063 -
2064 - if (((kept_only & ON_DISK_KEEP_PACKS) && e->p->pack_keep) ||
2065 - ((kept_only & IN_CORE_KEEP_PACKS) && e->p->pack_keep_in_core))
2055 + if (fill_midx_entry(r, oid, e, m))
2056 return 1;
2057 }
2058
2059 list_for_each(pos, &r->objects->packed_git_mru) {
2060 struct packed_git *p = list_entry(pos, struct packed_git, mru);
2071 - if (p->multi_pack_index && !kept_only) {
2072 - /*
2073 - * If this pack is covered by the MIDX, we'd have found
2074 - * the object already in the loop above if it was here,
2075 - * so don't bother looking.
2076 - *
2077 - * The exception is if we are looking only at kept
2078 - * packs. An object can be present in two packs covered
2079 - * by the MIDX, one kept and one not-kept. And as the
2080 - * MIDX points to only one copy of each object, it might
2081 - * have returned only the non-kept version above. We
2082 - * have to check again to be thorough.
2083 - */
2084 - continue;
2085 - }
2086 - if (!kept_only ||
2087 - (((kept_only & ON_DISK_KEEP_PACKS) && p->pack_keep) ||
2088 - ((kept_only & IN_CORE_KEEP_PACKS) && p->pack_keep_in_core))) {
2089 - if (fill_pack_entry(oid, e, p)) {
2090 - list_move(&p->mru, &r->objects->packed_git_mru);
2091 - return 1;
2092 - }
2061 + if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) {
2062 + list_move(&p->mru, &r->objects->packed_git_mru);
2063 + return 1;
2064 }
2065 }
2066 return 0;
2067 }
2068
2098 -int find_pack_entry(struct repository *r, const struct object_id *oid, struct pack_entry *e)
2069 +static void maybe_invalidate_kept_pack_cache(struct repository *r,
2070 + unsigned flags)
2071 +{
2072 + if (!r->objects->kept_pack_cache.packs)
2073 + return;
2074 + if (r->objects->kept_pack_cache.flags == flags)
2075 + return;
2076 + FREE_AND_NULL(r->objects->kept_pack_cache.packs);
2077 + r->objects->kept_pack_cache.flags = 0;
2078 +}
2079 +
2080 +static struct packed_git **kept_pack_cache(struct repository *r, unsigned flags)
2081 {
2100 - return find_one_pack_entry(r, oid, e, 0);
2082 + maybe_invalidate_kept_pack_cache(r, flags);
2083 +
2084 + if (!r->objects->kept_pack_cache.packs) {
2085 + struct packed_git **packs = NULL;
2086 + size_t nr = 0, alloc = 0;
2087 + struct packed_git *p;
2088 +
2089 + /*
2090 + * We want "all" packs here, because we need to cover ones that
2091 + * are used by a midx, as well. We need to look in every one of
2092 + * them (instead of the midx itself) to cover duplicates. It's
2093 + * possible that an object is found in two packs that the midx
2094 + * covers, one kept and one not kept, but the midx returns only
2095 + * the non-kept version.
2096 + */
2097 + for (p = get_all_packs(r); p; p = p->next) {
2098 + if ((p->pack_keep && (flags & ON_DISK_KEEP_PACKS)) ||
2099 + (p->pack_keep_in_core && (flags & IN_CORE_KEEP_PACKS))) {
2100 + ALLOC_GROW(packs, nr + 1, alloc);
2101 + packs[nr++] = p;
2102 + }
2103 + }
2104 + ALLOC_GROW(packs, nr + 1, alloc);
2105 + packs[nr] = NULL;
2106 +
2107 + r->objects->kept_pack_cache.packs = packs;
2108 + r->objects->kept_pack_cache.flags = flags;
2109 + }
2110 +
2111 + return r->objects->kept_pack_cache.packs;
2112 }
2113
2114 int find_kept_pack_entry(struct repository *r,
@@ -2105,13 +2116,15 @@ int find_kept_pack_entry(struct repository *r,
2116 unsigned flags,
2117 struct pack_entry *e)
2118 {
2108 - /*
2109 - * Load all packs, including midx packs, since our "kept" strategy
2110 - * relies on that. We're relying on the side effect of it setting up
2111 - * r->objects->packed_git, which is a little ugly.
2112 - */
2113 - get_all_packs(r);
2114 - return find_one_pack_entry(r, oid, e, flags);
2119 + struct packed_git **cache;
2120 +
2121 + for (cache = kept_pack_cache(r, flags); *cache; cache++) {
2122 + struct packed_git *p = *cache;
2123 + if (fill_pack_entry(oid, e, p))
2124 + return 1;
2125 + }
2126 +
2127 + return 0;
2128 }
2129
2130 int has_object_pack(const struct object_id *oid)