* Re: [PATCH 4/7] hash: make git_hash_discard() idempotent
From: Junio C Hamano @ 2026-07-07 16:22 UTC (permalink / raw)
To: Jeff King; +Cc: git, Patrick Steinhardt, brian m. carlson
In-Reply-To: <20260707050700.GD1288294@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> You must always either finalize or discard a hash context to release any
> resources, but you must call only one such function. This creates extra
> work for some callers, since their cleanup code paths need to know
> whether they got there via their happy path (and the finalization
> happened) or due to an error (in which case they need to discard).
>
> Let's add an "active" flag that turns a redundant discard into a noop.
> That lets you safely do this:
>
> git_hash_init(&ctx, algo);
> ...
> if (some_error)
> goto out;
> ...
> git_hash_final(result, &ctx);
>
> out:
> git_hash_discard(&ctx);
>
> This should avoid future errors, and will also let us simplify a few
> existing callers (in future patches).
Hmph, so is the point of this change to allow _discard() to be
called even after _final() was already called that we do not need an
early return or something before the out: label?
Unlike commit_*() and rollback_*() used in lockfile API, where the
names clearly say which one is for happy and which one is for error
case, the _final() and _discard() pair does not exactly tell me
which is which, but I guess I will get used to it, perhaps.
But the change nevertheless looks mostly good except for one "hmph".
When _init() is called, active gets turned on automatically, and
either _discard() or _final() turns it off. Only _discard() is
protected from getting called multiple times. Is this because
it is already a no-op to call _final() multiple times?
Thanks.
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> hash.c | 6 ++++++
> hash.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/hash.c b/hash.c
> index 55d1d41770..b1296f0018 100644
> --- a/hash.c
> +++ b/hash.c
> @@ -285,6 +285,7 @@ void git_hash_free(struct git_hash_ctx *ctx)
> void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)
> {
> algop->init_fn(ctx);
> + ctx->active = true;
> }
>
> void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)
> @@ -300,16 +301,21 @@ void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)
> void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)
> {
> ctx->algop->final_fn(hash, ctx);
> + ctx->active = false;
> }
>
> void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)
> {
> ctx->algop->final_oid_fn(oid, ctx);
> + ctx->active = false;
> }
>
> void git_hash_discard(struct git_hash_ctx *ctx)
> {
> + if (!ctx->active)
> + return;
> ctx->algop->discard_fn(ctx);
> + ctx->active = false;
> }
>
> uint32_t hash_algo_by_name(const char *name)
> diff --git a/hash.h b/hash.h
> index 5686914b71..f97f7b9ff4 100644
> --- a/hash.h
> +++ b/hash.h
> @@ -281,6 +281,7 @@ struct git_hash_ctx {
> git_SHA_CTX_unsafe sha1_unsafe;
> git_SHA256_CTX sha256;
> } state;
> + bool active;
> };
>
> typedef void (*git_hash_init_fn)(struct git_hash_ctx *ctx);
^ permalink raw reply
* Re: [PATCH 2/7] hash: convert remaining direct function calls
From: Junio C Hamano @ 2026-07-07 16:15 UTC (permalink / raw)
To: Jeff King; +Cc: git, Patrick Steinhardt, brian m. carlson
In-Reply-To: <20260707050417.GB1288294@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The previous patch added a coccinelle rule to make sure callers always
> use git_hash_init() rather than direct function pointers from the algo
> struct.
>
> Let's do the same for the rest of the git_hash_*() wrappers. I split
> these out because they're a bit different: they implicitly use the algop
> pointer in the git_hash_ctx. So when we convert:
>
> -algo->update_fn(&ctx, buf, len);
> +git_hash_update(&ctx, buf, len);
>
> we drop the reference to algo entirely! But this is always going to be
> the right thing. If "algo" does not match what is in ctx.algop, then
> we'd already be invoking undefined behavior.
>
> So in addition to making it possible to add more logic to the
> git_hash_*() functions, we're avoiding the need to pass around the extra
> algo pointer and make sure that it matches what's in "ctx".
>
> The rest of the patch is the mechanical application of that coccinelle
> patch, plus a minor cleanup in test-synthesize.c to drop a now-unused
> function parameter (since we don't have to pass around the algo
> separately anymore).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> builtin/submodule--helper.c | 8 +++---
> t/helper/test-synthesize.c | 29 ++++++++++----------
> tools/coccinelle/hash.cocci | 53 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 71 insertions(+), 19 deletions(-)
Looks very straight-forward.
^ permalink raw reply
* Re: [PATCH 1/2] t: add tests for ref tombstone scenarios
From: Kristofer Karlsson @ 2026-07-07 16:12 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <ak0aNrBpuo7ZwZ2k@pks.im>
On Tue, 7 Jul 2026 at 17:24, Patrick Steinhardt <ps@pks.im> wrote:
>
> On Mon, Jul 06, 2026 at 01:35:55PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> > diff --git a/t/perf/p1401-ref-store-tombstones.sh b/t/perf/p1401-ref-store-tombstones.sh
> > new file mode 100755
> > index 0000000000..e40a6dcbf4
> > --- /dev/null
> > +++ b/t/perf/p1401-ref-store-tombstones.sh
> > @@ -0,0 +1,44 @@
> > +#!/bin/sh
> > +
> > +test_description="Tests performance of ref operations with many tombstones"
> > +
> > +. ./perf-lib.sh
> > +
> > +test_expect_success "setup" '
> > + git init --ref-format=reftable repo &&
> > + blob=$(echo foo | git -C repo hash-object -w --stdin) &&
> > + for i in $(test_seq 8000)
> > + do
> > + printf "create refs/tags/tag-%d %s\n" "$i" "$blob" ||
> > + return 1
> > + done >repo/input &&
> > + git -C repo update-ref --stdin <repo/input &&
> > + git -C repo for-each-ref --format="delete %(refname)" |
> > + git -C repo update-ref --stdin
> > +'
> > +
> > +test_perf "recreate refs after mass delete" '
> > + git -C repo update-ref --stdin <repo/input &&
> > + git -C repo for-each-ref --format="delete %(refname)" |
> > + git -C repo update-ref --stdin
> > +'
>
> You're not only benchmarking the reference recreation, but also their
> deletion. If I'm not misreading things, then you can queue cleanups via
> `test_when_finished`, and these calls will not be measured.
I don't think measuring the full create+delete cycle is wrong per se,
but you are right that if we can benchmark something more isolated
is even more useful. I will try to split this up better.
> > +test_expect_success "setup asymmetric" '
> > + for i in $(test_seq 8000)
> > + do
> > + printf "create refs/tags/old-%d %s\n" "$i" "$blob" ||
> > + return 1
> > + done >repo/input-old &&
> > + sed "s/old-/new-/" <repo/input-old >repo/input-new &&
> > + git -C repo update-ref --stdin <repo/input-old &&
> > + git -C repo for-each-ref --format="delete %(refname)" |
> > + git -C repo update-ref --stdin
> > +'
>
> Would it make sense to use separate repositories? Otherwise, state from
> the preceding benchmark(s) will impact subsequent ones.
Agreed, I can use a fresh repo for each scenario.
>
> > diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> > +test_expect_success 'delete and re-create refs with tombstones' '
>
> I wonder whether this test really adds any value. We probably have lots
> of tests already that test creation/deletion of references.
I could not find an existing test that covers the delete-then-recreate
flow (where tombstones are present when the new refs are created).
The existing tests cover creation and deletion separately but not the
interaction with tombstones.
(But perhaps such a test exists and I just can't find it.)
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword
From: D. Ben Knoble @ 2026-07-07 16:10 UTC (permalink / raw)
To: Dominique Martinet
Cc: Pablo Sabater, Junio C Hamano, git, Patrick Steinhardt,
Kaartic Sivaraam
In-Reply-To: <akyKDtuHTHZGEpFx@codewreck.org>
On Tue, Jul 7, 2026 at 1:09 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> [context: I just played with git history reword/fixup and dug through
> archives for anything like this, so chiming in.
> First, thanks for the new git history commands, they all look promising!]
>
> Ben Knoble wrote on Mon, Jun 08, 2026 at 12:47:41PM -0400:
[snip]
> >> They do not, they are thought with the rule of silence in mind.
> >> However I think that this output is valuable information I might have
> >> explained myself better at [1] but my thought is:
> >>
> >> git history reword aabb
> >>
> >> Now that I have my commit aabb rewritten I want to check it again just
> >> to make sure I did what I wanted correctly,
> >
> > Some thoughts:
> >
> > - If the rewritten commit is an ancestor of HEAD, look at the log of HEAD@{1} or the log between HEAD and the aforementioned reflog entry. (git-range-diff may also be helpful there.)
> > - Similarly, if the rewritten commit is reachable from some ref R, check R@{1} etc.
>
> During my quick tests I was surprised with how git history reword/fixup
> behave with commits that aren't ancestors of HEAD/any branch (that can
> happen for example if you print `git log --oneline` once and refer to it
> after editing.
Indeed, this is a bit of a "trap":
> This transcript is a bit ugly but should illustrate the issue:
> ```
> $ git init
> Initialized empty Git repository in ...test/.git/
> $ echo a > aa
> $ git add aa
> $ git commit -m init
> [master (root-commit) 62884dc4d43c] init
> 1 file changed, 1 insertion(+)
> create mode 100644 aa
> $ echo b > b
> $ git add b
> $ git commit -m b
> [master 058294f87a36] b
> 1 file changed, 1 insertion(+)
> create mode 100644 b
> $ echo c > c
> $ git add c
> $ git commit -m c
> [master 0c4ad0c9337c] c
> 1 file changed, 1 insertion(+)
> create mode 100644 c
> $ git log --oneline --graph
> * 0c4ad0c9337c (HEAD -> master) c
> * 058294f87a36 b
> * 62884dc4d43c init
> $ echo d > d
> $ git add d
> $ git history fixup HEAD^
> $ echo e > e
> $ git add e
> $ git history fixup 058294f87a36
> $ git status
> On branch master
> Changes to be committed:
> (use "git restore --staged <file>..." to unstage)
> new file: e
> $ git history reword 058294f87a36
> (editor showed up, commit message modified and saved)
> $ git log --oneline --graph
> * 5cc5551381a3 (HEAD -> master) c
> * 0b7ab36bf167 b
> * 62884dc4d43c init
> ```
> -> fixup didn't show any message (and exited with 0), but didn't unstage
> the hunk either and didn't do anything, so one cannot differentiate with
> the fixup actually happening
> -> reword showed up editor but didn't actually do anything visible
> (probably did create a new commit somewhere that's unreachable?)
I think what probably happened here (and what you might find with `git
fsck` for example) is that you have new commit objects in chains
corresponding to those operations, but no refs were rewritten.
> So I agree with Pablo's suggestion: printing old/new short hash on
> success would help visualy confirming something worked.
I think we have the machinery for this (see --update-refs=print for
git-replay, for example), but I'm surprised to learn that we don't
accept --update-refs=print for history.
In any case, I second the "we should emit something"—I wonder what, though.
- In the case of rewritten refs, we might like to emit the list of
rewrites, a bit like a fetch or push will do: "+ $old...$new $ref
(forced update)" or something
- For new objects that aren't pointed to… maybe silence is a better
indicator that "we didn't do what you intended"? Or we could just
print the new commit objects "$new [unreferenced object]" or something
> ... But it might be worth to ensure that the commit has any ref we can
> handle (if --update-refs is set then the commit we edit is ancestor to
> some branch, if not set then it must be an ancestor of HEAD)
>
> What do you think?
I don't think it's worth restricting the operation (I can imagine a
use case where someone creates an unpointed-to object and later makes
the ref, even if that's a bit weird), but
- we could have a "strict" mode that ensured inputs are pointed to
- we could warn when only unreferenced objects are rewritten
? I see git-history as very "porcelain"/user-focused, so I think it's
feasible to add output niceties (and optionally a quiet mode to
suppress the messages).
--
D. Ben Knoble
^ permalink raw reply
* Re: [PATCH 2/2] reftable: fix quadratic behavior when re-creating deleted refs
From: Kristofer Karlsson @ 2026-07-07 16:04 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <ak0aRtvSxSyIWieg@pks.im>
On Tue, 7 Jul 2026 at 17:24, Patrick Steinhardt <ps@pks.im> wrote:
>
> > This affects two code paths during ref creation:
> >
> > - refs_verify_refnames_available() seeks to "refs/tags/foo-1/" to
> > check for D/F conflicts and must scan through all subsequent
> > tombstones before the caller can see that they are past the prefix
> > of interest.
> >
> > - reftable_backend_read_ref() seeks to a specific refname and must
> > scan through all subsequent tombstones before returning "not
> > found", because the merged iterator skips the matching tombstone
> > and searches for the next live record.
>
> It probably not only impacts reference creation, but also every reader
> that wants to search for a specific reference that doesn't exist.
Hm good point, I will try to rephrase this better.
> > Fix this by removing suppress_deletions from the merged iterator and
> > instead handling deletion records at each call site in the reftable
> > backend, where prefix and refname bounds are available. Tombstones
> > are now returned to callers, which skip them after their existing
> > bounds checks. This allows iteration to terminate as soon as a
> > tombstone past the relevant bound is encountered.
>
> This option is still used by downstream users of the reftable library,
> like libgit2. So we shouldn't just delete it outright.
Good catch! I can keep suppress_deletions as-is and just
stop setting it from stack.c. That way libgit2 is unchanged, while
we still optimize it at the other call sites. The reftable library
diff then shrinks to a single removed line.
> > diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> > index 4ae22922de..8c4f119ff1 100644
> > --- a/refs/reftable-backend.c
> > +++ b/refs/reftable-backend.c
> > @@ -633,6 +633,9 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
> > break;
> > }
> >
> > + if (iter->ref.value_type == REFTABLE_REF_DELETION)
> > + continue;
> > +
> > if (iter->exclude_patterns && should_exclude_current_ref(iter))
> > continue;
> >
>
> Okay. I was first wondering whether we should move this call earlier.
> But we actually don't want to, as this is the code that precedes the
> above:
>
> if (iter->prefix_len &&
> strncmp(iter->prefix, iter->ref.refname, iter->prefix_len)) {
> iter->err = 1;
> break;
> }
>
> So this allows us to not only skip the current iteration, but completely
> abort iteration by observing tombstones that sort after our prefix.
Indeed, this is the primary win.
> In any case, as far as I can see all sites where we iterate through
> either ref or log records have been adapted to handle deletions.
Thanks! Appreciate the review (and spotting the libgit breakage!)
Kristofer
^ permalink raw reply
* Re: [PATCH v2 00/12] coverity: fix leaks and error paths
From: Patrick Steinhardt @ 2026-07-07 15:55 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.2163.v2.git.1783239870.gitgitgadget@gmail.com>
On Sun, Jul 05, 2026 at 08:24:17AM +0000, Johannes Schindelin via GitGitGadget wrote:
> I wanted to whittle down the many issues reported by Coverity in the Git for
> Windows project. Turns out: The vast majority of the issues are false
> positives. Most of the remaining issues are in core Git proper.
>
> This effort was forced on pause while Coverity was down from May 16
> [https://web.archive.org/web/20260516152422/https://scan.coverity.com/] to
> June 22
> [https://web.archive.org/web/20260622182153/https://scan.coverity.com/]).
>
> Here is a first batch of fixes for those issues.
>
> Changes since v1:
>
> * Edited the commit messages to put function names in backticks, and
> reflowed the messages afterwards.
> * Took Junio's suggestion to avoid (ab-)using errno to determine the return
> value of load_one_loose_object_map().
> * Dropped the obsolete patch "run_diff_files: avoid memory leak".
> * Rewrote the commit message of "dir: free allocations on parse-error paths
> in read_one_dir()" to clarify ownership of the allocated untracked/dirs
> buffers.
> * Changed "submodule: fix cwd leak in get_superproject_working_tree()" to
> reduce the cognitive load on the reader (i.e. to make it a lot easier to
> reason about the correctness of the patch).
Thanks. The reflow of the commit messages made the range-diff somewhat
hard to read, but from all I could see the changes all make sense.
Patrick
^ permalink raw reply
* Re: [PATCH v2 01/12] load_one_loose_object_map(): fix resource leak
From: Patrick Steinhardt @ 2026-07-07 15:55 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <80ae35227d566977ad21eb6e35f49e1ca5d5a940.1783239870.git.gitgitgadget@gmail.com>
On Sun, Jul 05, 2026 at 08:24:18AM +0000, Johannes Schindelin via GitGitGadget wrote:
> @@ -98,13 +98,12 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_
> insert_loose_map(loose, &oid, &compat_oid);
> }
>
> - strbuf_release(&buf);
> - strbuf_release(&path);
> - return errno ? -1 : 0;
> + ret = ferror(fp) ? -1 : 0;
> err:
> + fclose(fp);
> strbuf_release(&buf);
> strbuf_release(&path);
> - return -1;
> + return ret;
Nit: it might've made sense to explain the switch to ferror(3p) in the
commit message, but that alone isn't worth a reroll.
Patrick
^ permalink raw reply
* Re: [PATCH v2 07/12] submodule: fix cwd leak in `get_superproject_working_tree()`
From: Patrick Steinhardt @ 2026-07-07 15:52 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <5397ea785c6da50e977598a35d03af82cb2a5e4d.1783239870.git.gitgitgadget@gmail.com>
On Sun, Jul 05, 2026 at 08:24:24AM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/submodule.c b/submodule.c
> index fd91201a92..92dfb0fc2d 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -2627,13 +2627,12 @@ int get_superproject_working_tree(struct strbuf *buf)
> * We might have a superproject, but it is harder
> * to determine.
> */
> - return 0;
> + goto out;
>
> if (!strbuf_realpath(&one_up, "../", 0))
> - return 0;
> + goto out;
>
> subpath = relative_path(cwd, one_up.buf, &sb);
> - strbuf_release(&one_up);
>
> prepare_submodule_repo_env(&cp.env);
> strvec_pop(&cp.env);
Right. `ret` is already zero-initialized at the beginning of the
function, so it's fine to just `goto out` here.
> @@ -2678,20 +2677,22 @@ int get_superproject_working_tree(struct strbuf *buf)
> ret = 1;
> free(super_wt);
> }
> - free(cwd);
> - strbuf_release(&sb);
>
> code = finish_command(&cp);
>
> if (code == 128)
> /* '../' is not a git repository */
> - return 0;
> - if (code == 0 && len == 0)
> + ret = 0;
> + else if (code == 0 && len == 0)
> /* There is an unrelated git repository at '../' */
> - return 0;
> - if (code)
> + ret = 0;
> + else if (code)
> die(_("ls-tree returned unexpected return code %d"), code);
The diff is a bit hard to read as we also convert this to use `else if`,
but overall the end result is easier to reason about.
> +out:
> + strbuf_release(&sb);
> + strbuf_release(&one_up);
> + free(cwd);
> return ret;
> }
All of these variables are always initialized, so this change looks good
to me.
Thanks!
Patrick
^ permalink raw reply
* [PATCH 11/11] odb: make optimizations pluggable
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
Move `odb_optimize()` and `odb_optimize_required()` from "builtin/gc.c"
into the "files" source and wire them up via newly introduced vtable
pointers for the object database sources. This makes the logic pluggable
and thus allows other backends to have their own, custom implementation.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 490 +----------------------------------------------------
odb.c | 12 ++
odb.h | 45 +++++
odb/source-files.c | 470 ++++++++++++++++++++++++++++++++++++++++++++++++++
odb/source-files.h | 15 ++
odb/source.h | 36 ++++
6 files changed, 579 insertions(+), 489 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 8cf3781313..ac1a21e912 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -30,16 +30,11 @@
#include "commit-graph.h"
#include "packfile.h"
#include "object-file.h"
-#include "pack.h"
-#include "pack-objects.h"
+#include "odb.h"
#include "path.h"
#include "reflog.h"
-#include "repack.h"
#include "rerere.h"
#include "revision.h"
-#include "blob.h"
-#include "tree.h"
-#include "promisor-remote.h"
#include "refs.h"
#include "remote.h"
#include "exec-cmd.h"
@@ -428,203 +423,6 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
return should_gc;
}
-static int too_many_loose_objects(struct odb_source_files *files, int limit)
-{
- unsigned long loose_count;
-
- if (limit <= 0)
- return 0;
-
- if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
- &loose_count) < 0)
- return 0;
-
- /*
- * This is weird, but stems from legacy behaviour: the GC auto
- * threshold was always essentially interpreted as if it was rounded up
- * to the next multiple 256 of, so we retain this behaviour for now.
- */
- return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
-}
-
-static struct packed_git *find_base_packs(struct odb_source_files *files,
- struct string_list *packs,
- unsigned long limit)
-{
- struct packfile_list_entry *e;
- struct packed_git *base = NULL;
-
- for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
- if (e->pack->is_cruft)
- continue;
- if (limit) {
- if ((uintmax_t) e->pack->pack_size >= limit)
- string_list_append(packs, e->pack->pack_name);
- } else if (!base || base->pack_size < e->pack->pack_size) {
- base = e->pack;
- }
- }
-
- if (base)
- string_list_append(packs, base->pack_name);
-
- return base;
-}
-
-static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
-{
- struct packfile_list_entry *e;
- int cnt = 0;
-
- if (gc_auto_pack_limit <= 0)
- return 0;
-
- for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
- if (e->pack->pack_keep)
- continue;
- /*
- * Perhaps check the size of the pack and count only
- * very small ones here?
- */
- cnt++;
- }
- return gc_auto_pack_limit < cnt;
-}
-
-static uint64_t total_ram(void)
-{
-#if defined(HAVE_SYSINFO)
- struct sysinfo si;
-
- if (!sysinfo(&si)) {
- uint64_t total = si.totalram;
-
- if (si.mem_unit > 1)
- total *= (uint64_t)si.mem_unit;
- return total;
- }
-#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64))
- uint64_t physical_memory;
- int mib[2];
- size_t length;
-
- mib[0] = CTL_HW;
-# if defined(HW_MEMSIZE)
- mib[1] = HW_MEMSIZE;
-# elif defined(HW_PHYSMEM64)
- mib[1] = HW_PHYSMEM64;
-# else
- mib[1] = HW_PHYSMEM;
-# endif
- length = sizeof(physical_memory);
- if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) {
- if (length == 4) {
- uint32_t mem;
-
- if (!sysctl(mib, 2, &mem, &length, NULL, 0))
- physical_memory = mem;
- }
- return physical_memory;
- }
-#elif defined(GIT_WINDOWS_NATIVE)
- MEMORYSTATUSEX memInfo;
-
- memInfo.dwLength = sizeof(MEMORYSTATUSEX);
- if (GlobalMemoryStatusEx(&memInfo))
- return memInfo.ullTotalPhys;
-#endif
- return 0;
-}
-
-static uint64_t estimate_repack_memory(struct odb_source_files *files,
- struct packed_git *pack)
-{
- unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
- unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
- unsigned long nr_objects;
- size_t os_cache, heap;
-
- if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
- &nr_objects) < 0)
- return 0;
-
- if (!pack || !nr_objects)
- return 0;
-
- repo_config_get_ulong(the_repository, "pack.deltacachesize", &max_delta_cache_size);
- repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &delta_base_cache_limit);
-
- /*
- * First we have to scan through at least one pack.
- * Assume enough room in OS file cache to keep the entire pack
- * or we may accidentally evict data of other processes from
- * the cache.
- */
- os_cache = pack->pack_size + pack->index_size;
- /* then pack-objects needs lots more for book keeping */
- heap = sizeof(struct object_entry) * nr_objects;
- /*
- * internal rev-list --all --objects takes up some memory too,
- * let's say half of it is for blobs
- */
- heap += sizeof(struct blob) * nr_objects / 2;
- /*
- * and the other half is for trees (commits and tags are
- * usually insignificant)
- */
- heap += sizeof(struct tree) * nr_objects / 2;
- /* and then obj_hash[], underestimated in fact */
- heap += sizeof(struct object *) * nr_objects;
- /* revindex is used also */
- heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects;
- /*
- * read_sha1_file() (either at delta calculation phase, or
- * writing phase) also fills up the delta base cache
- */
- heap += delta_base_cache_limit;
- /* and of course pack-objects has its own delta cache */
- heap += max_delta_cache_size;
-
- return os_cache + heap;
-}
-
-static int keep_one_pack(struct string_list_item *item, void *data)
-{
- struct strvec *args = data;
- strvec_pushf(args, "--keep-pack=%s", basename(item->string));
- return 0;
-}
-
-enum odb_optimize_strategy {
- ODB_OPTIMIZE_INCREMENTAL,
- ODB_OPTIMIZE_GEOMETRIC,
-};
-
-enum odb_optimize_flags {
- /* Enable verbose logging and progress reporting. */
- ODB_OPTIMIZE_VERBOSE = (1 << 0),
-
- /* Perform auto-maintenance, only optimizing objects as required. */
- ODB_OPTIMIZE_AUTO = (1 << 1),
-
- /* Recompute existing deltas. */
- ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
-};
-
-struct odb_optimize_options {
- enum odb_optimize_strategy strategy;
- enum odb_optimize_flags flags;
- const char *prune_expire;
- const char *expire_to;
- int depth;
- int window;
-
- /* Backend-specific options. */
- int keep_largest_pack;
- int cruft_packs;
- unsigned long max_cruft_size;
-};
-
#define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
.prune_expire = (cfg)->prune_expire, \
.expire_to = (cfg)->repack_expire_to, \
@@ -633,133 +431,6 @@ struct odb_optimize_options {
.window = (aggressive) ? (cfg)->aggressive_window : 0, \
.depth = (aggressive) ? (cfg)->aggressive_depth : 0
-static void add_repack_all_option(const struct odb_optimize_options *opts,
- struct string_list *keep_pack,
- struct strvec *args)
-{
- char *repack_filter = NULL;
- char *repack_filter_to = NULL;
-
- repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
- repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
-
- if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
- !(opts->cruft_packs && opts->expire_to))
- strvec_push(args, "-a");
- else if (opts->cruft_packs) {
- strvec_push(args, "--cruft");
- if (opts->prune_expire)
- strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
- if (opts->max_cruft_size)
- strvec_pushf(args, "--max-cruft-size=%lu",
- opts->max_cruft_size);
- if (opts->expire_to)
- strvec_pushf(args, "--expire-to=%s", opts->expire_to);
- } else {
- strvec_push(args, "-A");
- if (opts->prune_expire)
- strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
- }
-
- if (keep_pack)
- for_each_string_list(keep_pack, keep_one_pack, args);
-
- if (repack_filter && *repack_filter)
- strvec_pushf(args, "--filter=%s", repack_filter);
- if (repack_filter_to && *repack_filter_to)
- strvec_pushf(args, "--filter-to=%s", repack_filter_to);
-
- free(repack_filter);
- free(repack_filter_to);
-}
-
-static void add_repack_incremental_option(struct strvec *args)
-{
- strvec_push(args, "--no-write-bitmap-index");
-}
-
-static bool odb_optimize_required(struct object_database *odb,
- const struct odb_optimize_options *opts)
-{
- struct odb_source_files *files = odb_source_files_downcast(odb->sources);
-
- switch (opts->strategy) {
- case ODB_OPTIMIZE_INCREMENTAL: {
- int gc_auto_threshold = 6700;
- int gc_auto_pack_limit = 50;
-
- repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
- repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
-
- /*
- * Setting gc.auto to 0 or negative can disable the
- * automatic gc.
- */
- if (gc_auto_threshold <= 0)
- return false;
- if (!too_many_packs(files, gc_auto_pack_limit) &&
- !too_many_loose_objects(files, gc_auto_threshold))
- return false;
-
- return true;
- }
- case ODB_OPTIMIZE_GEOMETRIC: {
- struct pack_geometry geometry = {
- .split_factor = 2,
- };
- struct pack_objects_args po_args = {
- .local = 1,
- };
- struct existing_packs existing_packs = EXISTING_PACKS_INIT;
- struct string_list kept_packs = STRING_LIST_INIT_DUP;
- int auto_value = 100;
- bool ret;
-
- repo_config_get_int(odb->repo, "maintenance.geometric-repack.auto",
- &auto_value);
- if (!auto_value)
- return false;
- if (auto_value < 0)
- return true;
-
- repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
- &geometry.split_factor);
-
- existing_packs.repo = odb->repo;
- existing_packs_collect(&existing_packs, &kept_packs);
- pack_geometry_init(&geometry, &existing_packs, &po_args);
- pack_geometry_split(&geometry);
-
- /*
- * When we'd merge at least two packs with one another we always
- * perform the repack.
- */
- if (geometry.split) {
- ret = true;
- goto out;
- }
-
- /*
- * Otherwise, we estimate the number of loose objects to determine
- * whether we want to create a new packfile or not.
- */
- if (too_many_loose_objects(files, auto_value)) {
- ret = true;
- goto out;
- }
-
- ret = false;
-
- out:
- existing_packs_release(&existing_packs);
- pack_geometry_release(&geometry);
- return ret;
- }
- default:
- BUG("unknown maintenance strategy '%d'", opts->strategy);
- }
-}
-
/* return NULL on success, else hostname running the gc */
static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
{
@@ -887,165 +558,6 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
return 0;
}
-static int odb_optimize(struct object_database *odb,
- const struct odb_optimize_options *opts)
-{
- struct odb_source_files *files = odb_source_files_downcast(odb->sources);
- struct child_process repack_cmd = CHILD_PROCESS_INIT;
- unsigned long big_pack_threshold = 0;
- int gc_auto_threshold = 6700;
- int gc_auto_pack_limit = 50;
- int ret;
-
- repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
- repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
- repo_config_get_ulong(odb->repo, "gc.bigpackthreshold", &big_pack_threshold);
-
- if (odb->repo->repository_format_precious_objects)
- return 0;
-
- repack_cmd.git_cmd = 1;
- repack_cmd.odb_to_close = odb->repo->objects;
-
- strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
- if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
- strvec_push(&repack_cmd.args, "-f");
- if (opts->depth > 0)
- strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
- if (opts->window > 0)
- strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
- if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
- strvec_push(&repack_cmd.args, "-q");
-
- /*
- * There's three cases we need to consider:
- *
- * - If we're invoked without `--auto` we'll need to perform a full
- * repack.
- *
- * - If we're invoked with `--auto` and there's too many packs, then
- * we perform a full repack, as well.
- *
- * - Otherwise we perform an incremental repack.
- */
- switch (opts->strategy) {
- case ODB_OPTIMIZE_INCREMENTAL:
- if (!(opts->flags & ODB_OPTIMIZE_AUTO)) {
- struct string_list keep_pack = STRING_LIST_INIT_NODUP;
-
- if (opts->keep_largest_pack != -1) {
- if (opts->keep_largest_pack)
- find_base_packs(files, &keep_pack, 0);
- } else if (big_pack_threshold) {
- find_base_packs(files, &keep_pack, big_pack_threshold);
- }
-
- add_repack_all_option(opts, &keep_pack, &repack_cmd.args);
- string_list_clear(&keep_pack, 0);
- } else {
- if (too_many_packs(files, gc_auto_pack_limit)) {
- struct string_list keep_pack = STRING_LIST_INIT_NODUP;
-
- if (big_pack_threshold) {
- find_base_packs(files, &keep_pack, big_pack_threshold);
- if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
- string_list_clear(&keep_pack, 0);
- find_base_packs(files, &keep_pack, 0);
- }
- } else {
- struct packed_git *p = find_base_packs(files, &keep_pack, 0);
- uint64_t mem_have, mem_want;
-
- mem_have = total_ram();
- mem_want = estimate_repack_memory(files, p);
-
- /*
- * Only allow 1/2 of memory for pack-objects, leave
- * the rest for the OS and other processes in the
- * system.
- */
- if (!mem_have || mem_want < mem_have / 2)
- string_list_clear(&keep_pack, 0);
- }
-
- add_repack_all_option(opts, &keep_pack, &repack_cmd.args);
- string_list_clear(&keep_pack, 0);
- } else {
- add_repack_incremental_option(&repack_cmd.args);
- }
- }
-
- break;
- case ODB_OPTIMIZE_GEOMETRIC: {
- struct pack_geometry geometry = {
- .split_factor = 2,
- };
- struct pack_objects_args po_args = {
- .local = 1,
- };
- struct existing_packs existing_packs = EXISTING_PACKS_INIT;
- struct string_list kept_packs = STRING_LIST_INIT_DUP;
-
- repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
- &geometry.split_factor);
-
- existing_packs.repo = odb->repo;
- existing_packs_collect(&existing_packs, &kept_packs);
- pack_geometry_init(&geometry, &existing_packs, &po_args);
- pack_geometry_split(&geometry);
-
- if (geometry.split < geometry.pack_nr) {
- strvec_pushf(&repack_cmd.args, "--geometric=%d",
- geometry.split_factor);
- } else {
- add_repack_all_option(opts, NULL, &repack_cmd.args);
- }
- if (odb->repo->settings.core_multi_pack_index)
- strvec_push(&repack_cmd.args, "--write-midx");
-
- existing_packs_release(&existing_packs);
- pack_geometry_release(&geometry);
- break;
- }
- default:
- die("unknown maintenance strategy '%d'", opts->strategy);
- }
-
- if (run_command(&repack_cmd)) {
- ret = error(FAILED_RUN, repack_cmd.args.v[0]);
- goto out;
- }
-
- /* Geometric repacking uses cruft packs, so we don't have to prune separately. */
- if (opts->strategy != ODB_OPTIMIZE_GEOMETRIC && opts->prune_expire) {
- struct child_process prune_cmd = CHILD_PROCESS_INIT;
-
- strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
- /* run `git prune` even if using cruft packs */
- strvec_push(&prune_cmd.args, opts->prune_expire);
- if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
- strvec_push(&prune_cmd.args, "--no-progress");
- if (repo_has_promisor_remote(odb->repo))
- strvec_push(&prune_cmd.args,
- "--exclude-promisor-objects");
- prune_cmd.git_cmd = 1;
-
- if (run_command(&prune_cmd)) {
- ret = error(FAILED_RUN, prune_cmd.args.v[0]);
- goto out;
- }
- }
-
- if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
- warning(_("There are too many unreachable loose objects; "
- "run 'git prune' to remove them."));
-
- ret = 0;
-
-out:
- return ret;
-}
-
static int maintenance_task_odb(struct maintenance_run_opts *opts,
struct gc_config *cfg,
int keep_largest_pack,
diff --git a/odb.c b/odb.c
index 7d555be09f..89660981fe 100644
--- a/odb.c
+++ b/odb.c
@@ -1003,6 +1003,18 @@ int odb_write_object_stream(struct object_database *odb,
return odb_source_write_object_stream(odb->sources, stream, len, oid);
}
+int odb_optimize(struct object_database *odb,
+ const struct odb_optimize_options *opts)
+{
+ return odb_source_optimize(odb->sources, opts);
+}
+
+bool odb_optimize_required(struct object_database *odb,
+ const struct odb_optimize_options *opts)
+{
+ return odb_source_optimize_required(odb->sources, opts);
+}
+
struct object_database *odb_new(struct repository *repo,
const char *primary_source,
const char *secondary_sources)
diff --git a/odb.h b/odb.h
index 3834a0dcbf..7e1c85c22e 100644
--- a/odb.h
+++ b/odb.h
@@ -117,6 +117,51 @@ struct object_database *odb_new(struct repository *repo,
/* Free the object database and release all resources. */
void odb_free(struct object_database *o);
+enum odb_optimize_strategy {
+ ODB_OPTIMIZE_INCREMENTAL,
+ ODB_OPTIMIZE_GEOMETRIC,
+};
+
+enum odb_optimize_flags {
+ /* Enable verbose logging and progress reporting. */
+ ODB_OPTIMIZE_VERBOSE = (1 << 0),
+
+ /* Perform auto-maintenance, only optimizing objects as required. */
+ ODB_OPTIMIZE_AUTO = (1 << 1),
+
+ /* Recompute existing deltas. */
+ ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
+};
+
+struct odb_optimize_options {
+ enum odb_optimize_strategy strategy;
+ enum odb_optimize_flags flags;
+ const char *prune_expire;
+ const char *expire_to;
+ int depth;
+ int window;
+
+ /* Backend-specific options. */
+ int keep_largest_pack;
+ int cruft_packs;
+ unsigned long max_cruft_size;
+};
+
+/*
+ * Optimize the object database. Returns 0 on success, a negative error code
+ * otherwise.
+ */
+int odb_optimize(struct object_database *odb,
+ const struct odb_optimize_options *opts);
+
+/*
+ * Check whether optimization of the object database is required given the
+ * provided options. Returns true if optimization should be performed, false
+ * otherwise.
+ */
+bool odb_optimize_required(struct object_database *odb,
+ const struct odb_optimize_options *opts);
+
/*
* Close the object database and all of its sources so that any held resources
* will be released. The database can still be used after closing it, in which
diff --git a/odb/source-files.c b/odb/source-files.c
index bbd1784b33..82cf61da4a 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -1,6 +1,8 @@
#include "git-compat-util.h"
#include "abspath.h"
+#include "blob.h"
#include "chdir-notify.h"
+#include "config.h"
#include "gettext.h"
#include "lockfile.h"
#include "object-file.h"
@@ -8,8 +10,16 @@
#include "odb/source.h"
#include "odb/source-files.h"
#include "odb/source-loose.h"
+#include "pack-objects.h"
#include "packfile.h"
+#include "path.h"
+#include "promisor-remote.h"
+#include "repack.h"
+#include "run-command.h"
#include "strbuf.h"
+#include "string-list.h"
+#include "strvec.h"
+#include "tree.h"
#include "write-or-die.h"
static void odb_source_files_reparent(const char *name UNUSED,
@@ -260,6 +270,464 @@ static int odb_source_files_write_alternate(struct odb_source *source,
return ret;
}
+static int too_many_loose_objects(struct odb_source_files *files, int limit)
+{
+ unsigned long loose_count;
+
+ if (limit <= 0)
+ return 0;
+
+ if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+ &loose_count) < 0)
+ return 0;
+
+ /*
+ * This is weird, but stems from legacy behaviour: the GC auto
+ * threshold was always essentially interpreted as if it was rounded up
+ * to the next multiple 256 of, so we retain this behaviour for now.
+ */
+ return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
+}
+
+static struct packed_git *find_base_packs(struct odb_source_files *files,
+ struct string_list *packs,
+ unsigned long limit)
+{
+ struct packfile_list_entry *e;
+ struct packed_git *base = NULL;
+
+ for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+ if (e->pack->is_cruft)
+ continue;
+ if (limit) {
+ if ((uintmax_t) e->pack->pack_size >= limit)
+ string_list_append(packs, e->pack->pack_name);
+ } else if (!base || base->pack_size < e->pack->pack_size) {
+ base = e->pack;
+ }
+ }
+
+ if (base)
+ string_list_append(packs, base->pack_name);
+
+ return base;
+}
+
+static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
+{
+ struct packfile_list_entry *e;
+ int cnt = 0;
+
+ if (gc_auto_pack_limit <= 0)
+ return 0;
+
+ for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+ if (e->pack->pack_keep)
+ continue;
+ /*
+ * Perhaps check the size of the pack and count only
+ * very small ones here?
+ */
+ cnt++;
+ }
+ return gc_auto_pack_limit < cnt;
+}
+
+static uint64_t total_ram(void)
+{
+#if defined(HAVE_SYSINFO)
+ struct sysinfo si;
+
+ if (!sysinfo(&si)) {
+ uint64_t total = si.totalram;
+
+ if (si.mem_unit > 1)
+ total *= (uint64_t)si.mem_unit;
+ return total;
+ }
+#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64))
+ uint64_t physical_memory;
+ int mib[2];
+ size_t length;
+
+ mib[0] = CTL_HW;
+# if defined(HW_MEMSIZE)
+ mib[1] = HW_MEMSIZE;
+# elif defined(HW_PHYSMEM64)
+ mib[1] = HW_PHYSMEM64;
+# else
+ mib[1] = HW_PHYSMEM;
+# endif
+ length = sizeof(physical_memory);
+ if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) {
+ if (length == 4) {
+ uint32_t mem;
+
+ if (!sysctl(mib, 2, &mem, &length, NULL, 0))
+ physical_memory = mem;
+ }
+ return physical_memory;
+ }
+#elif defined(GIT_WINDOWS_NATIVE)
+ MEMORYSTATUSEX memInfo;
+
+ memInfo.dwLength = sizeof(MEMORYSTATUSEX);
+ if (GlobalMemoryStatusEx(&memInfo))
+ return memInfo.ullTotalPhys;
+#endif
+ return 0;
+}
+
+static uint64_t estimate_repack_memory(struct odb_source_files *files,
+ struct packed_git *pack)
+{
+ unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
+ unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
+ unsigned long nr_objects;
+ size_t os_cache, heap;
+
+ if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+ &nr_objects) < 0)
+ return 0;
+
+ if (!pack || !nr_objects)
+ return 0;
+
+ repo_config_get_ulong(files->base.odb->repo, "pack.deltacachesize",
+ &max_delta_cache_size);
+ repo_config_get_ulong(files->base.odb->repo, "core.deltabasecachelimit",
+ &delta_base_cache_limit);
+
+ /*
+ * First we have to scan through at least one pack.
+ * Assume enough room in OS file cache to keep the entire pack
+ * or we may accidentally evict data of other processes from
+ * the cache.
+ */
+ os_cache = pack->pack_size + pack->index_size;
+ /* then pack-objects needs lots more for book keeping */
+ heap = sizeof(struct object_entry) * nr_objects;
+ /*
+ * internal rev-list --all --objects takes up some memory too,
+ * let's say half of it is for blobs
+ */
+ heap += sizeof(struct blob) * nr_objects / 2;
+ /*
+ * and the other half is for trees (commits and tags are
+ * usually insignificant)
+ */
+ heap += sizeof(struct tree) * nr_objects / 2;
+ /* and then obj_hash[], underestimated in fact */
+ heap += sizeof(struct object *) * nr_objects;
+ /* revindex is used also */
+ heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects;
+ /*
+ * read_sha1_file() (either at delta calculation phase, or
+ * writing phase) also fills up the delta base cache
+ */
+ heap += delta_base_cache_limit;
+ /* and of course pack-objects has its own delta cache */
+ heap += max_delta_cache_size;
+
+ return os_cache + heap;
+}
+
+static int keep_one_pack(struct string_list_item *item, void *data)
+{
+ struct strvec *args = data;
+ strvec_pushf(args, "--keep-pack=%s", basename(item->string));
+ return 0;
+}
+
+static void add_repack_all_option(struct repository *repo,
+ const struct odb_optimize_options *opts,
+ struct string_list *keep_pack,
+ struct strvec *args)
+{
+ char *repack_filter = NULL;
+ char *repack_filter_to = NULL;
+
+ repo_config_get_string(repo, "gc.repackfilter", &repack_filter);
+ repo_config_get_string(repo, "gc.repackfilterto", &repack_filter_to);
+
+ if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
+ !(opts->cruft_packs && opts->expire_to))
+ strvec_push(args, "-a");
+ else if (opts->cruft_packs) {
+ strvec_push(args, "--cruft");
+ if (opts->prune_expire)
+ strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
+ if (opts->max_cruft_size)
+ strvec_pushf(args, "--max-cruft-size=%lu",
+ opts->max_cruft_size);
+ if (opts->expire_to)
+ strvec_pushf(args, "--expire-to=%s", opts->expire_to);
+ } else {
+ strvec_push(args, "-A");
+ if (opts->prune_expire)
+ strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
+ }
+
+ if (keep_pack)
+ for_each_string_list(keep_pack, keep_one_pack, args);
+
+ if (repack_filter && *repack_filter)
+ strvec_pushf(args, "--filter=%s", repack_filter);
+ if (repack_filter_to && *repack_filter_to)
+ strvec_pushf(args, "--filter-to=%s", repack_filter_to);
+
+ free(repack_filter);
+ free(repack_filter_to);
+}
+
+static void add_repack_incremental_option(struct strvec *args)
+{
+ strvec_push(args, "--no-write-bitmap-index");
+}
+
+bool odb_source_files_optimize_required(struct odb_source *source,
+ const struct odb_optimize_options *opts)
+{
+ struct odb_source_files *files = odb_source_files_downcast(source);
+ struct repository *repo = source->odb->repo;
+
+ switch (opts->strategy) {
+ case ODB_OPTIMIZE_INCREMENTAL: {
+ int gc_auto_threshold = 6700;
+ int gc_auto_pack_limit = 50;
+
+ repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+ repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+
+ /*
+ * Setting gc.auto to 0 or negative can disable the
+ * automatic gc.
+ */
+ if (gc_auto_threshold <= 0)
+ return false;
+ if (!too_many_packs(files, gc_auto_pack_limit) &&
+ !too_many_loose_objects(files, gc_auto_threshold))
+ return false;
+
+ return true;
+ }
+ case ODB_OPTIMIZE_GEOMETRIC: {
+ struct pack_geometry geometry = {
+ .split_factor = 2,
+ };
+ struct pack_objects_args po_args = {
+ .local = 1,
+ };
+ struct existing_packs existing_packs = EXISTING_PACKS_INIT;
+ struct string_list kept_packs = STRING_LIST_INIT_DUP;
+ int auto_value = 100;
+ bool ret;
+
+ repo_config_get_int(repo, "maintenance.geometric-repack.auto",
+ &auto_value);
+ if (!auto_value)
+ return false;
+ if (auto_value < 0)
+ return true;
+
+ repo_config_get_int(repo, "maintenance.geometric-repack.splitFactor",
+ &geometry.split_factor);
+
+ existing_packs.repo = repo;
+ existing_packs_collect(&existing_packs, &kept_packs);
+ pack_geometry_init(&geometry, &existing_packs, &po_args);
+ pack_geometry_split(&geometry);
+
+ /*
+ * When we'd merge at least two packs with one another we always
+ * perform the repack.
+ */
+ if (geometry.split) {
+ ret = true;
+ goto out;
+ }
+
+ /*
+ * Otherwise, we estimate the number of loose objects to determine
+ * whether we want to create a new packfile or not.
+ */
+ if (too_many_loose_objects(files, auto_value)) {
+ ret = true;
+ goto out;
+ }
+
+ ret = false;
+
+ out:
+ existing_packs_release(&existing_packs);
+ pack_geometry_release(&geometry);
+ return ret;
+ }
+ default:
+ BUG("unknown maintenance strategy '%d'", opts->strategy);
+ }
+}
+
+int odb_source_files_optimize(struct odb_source *source,
+ const struct odb_optimize_options *opts)
+{
+ struct odb_source_files *files = odb_source_files_downcast(source);
+ struct repository *repo = source->odb->repo;
+ struct child_process repack_cmd = CHILD_PROCESS_INIT;
+ unsigned long big_pack_threshold = 0;
+ int gc_auto_threshold = 6700;
+ int gc_auto_pack_limit = 50;
+ int ret;
+
+ repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+ repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+ repo_config_get_ulong(repo, "gc.bigpackthreshold", &big_pack_threshold);
+
+ if (repo->repository_format_precious_objects)
+ return 0;
+
+ repack_cmd.git_cmd = 1;
+ repack_cmd.odb_to_close = repo->objects;
+
+ strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
+ if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
+ strvec_push(&repack_cmd.args, "-f");
+ if (opts->depth > 0)
+ strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
+ if (opts->window > 0)
+ strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
+ if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
+ strvec_push(&repack_cmd.args, "-q");
+
+ /*
+ * There's three cases we need to consider:
+ *
+ * - If we're invoked without `--auto` we'll need to perform a full
+ * repack.
+ *
+ * - If we're invoked with `--auto` and there's too many packs, then
+ * we perform a full repack, as well.
+ *
+ * - Otherwise we perform an incremental repack.
+ */
+ switch (opts->strategy) {
+ case ODB_OPTIMIZE_INCREMENTAL:
+ if (!(opts->flags & ODB_OPTIMIZE_AUTO)) {
+ struct string_list keep_pack = STRING_LIST_INIT_NODUP;
+
+ if (opts->keep_largest_pack != -1) {
+ if (opts->keep_largest_pack)
+ find_base_packs(files, &keep_pack, 0);
+ } else if (big_pack_threshold) {
+ find_base_packs(files, &keep_pack, big_pack_threshold);
+ }
+
+ add_repack_all_option(repo, opts, &keep_pack, &repack_cmd.args);
+ string_list_clear(&keep_pack, 0);
+ } else {
+ if (too_many_packs(files, gc_auto_pack_limit)) {
+ struct string_list keep_pack = STRING_LIST_INIT_NODUP;
+
+ if (big_pack_threshold) {
+ find_base_packs(files, &keep_pack, big_pack_threshold);
+ if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
+ string_list_clear(&keep_pack, 0);
+ find_base_packs(files, &keep_pack, 0);
+ }
+ } else {
+ struct packed_git *p = find_base_packs(files, &keep_pack, 0);
+ uint64_t mem_have, mem_want;
+
+ mem_have = total_ram();
+ mem_want = estimate_repack_memory(files, p);
+
+ /*
+ * Only allow 1/2 of memory for pack-objects, leave
+ * the rest for the OS and other processes in the
+ * system.
+ */
+ if (!mem_have || mem_want < mem_have / 2)
+ string_list_clear(&keep_pack, 0);
+ }
+
+ add_repack_all_option(repo, opts, &keep_pack, &repack_cmd.args);
+ string_list_clear(&keep_pack, 0);
+ } else {
+ add_repack_incremental_option(&repack_cmd.args);
+ }
+ }
+
+ break;
+ case ODB_OPTIMIZE_GEOMETRIC: {
+ struct pack_geometry geometry = {
+ .split_factor = 2,
+ };
+ struct pack_objects_args po_args = {
+ .local = 1,
+ };
+ struct existing_packs existing_packs = EXISTING_PACKS_INIT;
+ struct string_list kept_packs = STRING_LIST_INIT_DUP;
+
+ repo_config_get_int(repo, "maintenance.geometric-repack.splitFactor",
+ &geometry.split_factor);
+
+ existing_packs.repo = repo;
+ existing_packs_collect(&existing_packs, &kept_packs);
+ pack_geometry_init(&geometry, &existing_packs, &po_args);
+ pack_geometry_split(&geometry);
+
+ if (geometry.split < geometry.pack_nr) {
+ strvec_pushf(&repack_cmd.args, "--geometric=%d",
+ geometry.split_factor);
+ } else {
+ add_repack_all_option(repo, opts, NULL, &repack_cmd.args);
+ }
+ if (repo->settings.core_multi_pack_index)
+ strvec_push(&repack_cmd.args, "--write-midx");
+
+ existing_packs_release(&existing_packs);
+ pack_geometry_release(&geometry);
+ break;
+ }
+ default:
+ die("unknown maintenance strategy '%d'", opts->strategy);
+ }
+
+ if (run_command(&repack_cmd)) {
+ ret = error("failed to run %s", repack_cmd.args.v[0]);
+ goto out;
+ }
+
+ /* Geometric repacking uses cruft packs, so we don't have to prune separately. */
+ if (opts->strategy != ODB_OPTIMIZE_GEOMETRIC && opts->prune_expire) {
+ struct child_process prune_cmd = CHILD_PROCESS_INIT;
+
+ strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
+ /* run `git prune` even if using cruft packs */
+ strvec_push(&prune_cmd.args, opts->prune_expire);
+ if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
+ strvec_push(&prune_cmd.args, "--no-progress");
+ if (repo_has_promisor_remote(repo))
+ strvec_push(&prune_cmd.args,
+ "--exclude-promisor-objects");
+ prune_cmd.git_cmd = 1;
+
+ if (run_command(&prune_cmd)) {
+ ret = error("failed to run %s", prune_cmd.args.v[0]);
+ goto out;
+ }
+ }
+
+ if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
+ warning(_("There are too many unreachable loose objects; "
+ "run 'git prune' to remove them."));
+
+ ret = 0;
+
+out:
+ return ret;
+}
+
struct odb_source_files *odb_source_files_new(struct object_database *odb,
const char *path,
bool local)
@@ -285,6 +753,8 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
files->base.begin_transaction = odb_source_files_begin_transaction;
files->base.read_alternates = odb_source_files_read_alternates;
files->base.write_alternate = odb_source_files_write_alternate;
+ files->base.optimize = odb_source_files_optimize;
+ files->base.optimize_required = odb_source_files_optimize_required;
/*
* Ideally, we would only ever store absolute paths in the source. This
diff --git a/odb/source-files.h b/odb/source-files.h
index d7ac3c1c81..044242bc36 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -21,6 +21,21 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
const char *path,
bool local);
+/*
+ * Optimize the files object database source by repacking loose objects and
+ * packfiles as needed. Returns 0 on success, a negative error code otherwise.
+ */
+int odb_source_files_optimize(struct odb_source *source,
+ const struct odb_optimize_options *opts);
+
+/*
+ * Check whether optimization of the files object database source is required
+ * given the provided options. Returns true if optimization should be
+ * performed, false otherwise.
+ */
+bool odb_source_files_optimize_required(struct odb_source *source,
+ const struct odb_optimize_options *opts);
+
/*
* Cast the given object database source to the files backend. This will cause
* a BUG in case the source doesn't use this backend.
diff --git a/odb/source.h b/odb/source.h
index 8767708c9c..88a48ba3c3 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -258,6 +258,21 @@ struct odb_source {
*/
int (*write_alternate)(struct odb_source *source,
const char *alternate);
+
+ /*
+ * This callback is expected to optimize the object database source.
+ * Returns 0 on success, a negative error code otherwise.
+ */
+ int (*optimize)(struct odb_source *source,
+ const struct odb_optimize_options *opts);
+
+ /*
+ * This callback is expected to check whether optimization of the
+ * object database source is required given the provided options.
+ * Returns true if optimization should be performed, false otherwise.
+ */
+ bool (*optimize_required)(struct odb_source *source,
+ const struct odb_optimize_options *opts);
};
/*
@@ -475,4 +490,25 @@ static inline int odb_source_begin_transaction(struct odb_source *source,
return source->begin_transaction(source, out);
}
+/*
+ * Optimize the object database source. Returns 0 on success, a negative error
+ * code otherwise.
+ */
+static inline int odb_source_optimize(struct odb_source *source,
+ const struct odb_optimize_options *opts)
+{
+ return source->optimize(source, opts);
+}
+
+/*
+ * Check whether optimization of the object database source is required given
+ * the provided options. Returns true if optimization should be performed,
+ * false otherwise.
+ */
+static inline bool odb_source_optimize_required(struct odb_source *source,
+ const struct odb_optimize_options *opts)
+{
+ return source->optimize_required(source, opts);
+}
+
#endif
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 10/11] builtin/gc: fix signedness issues in ODB-related functionality
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
There are a couple of signedness issues in ODB-related functionality.
These are not a problem because we disable -Wsign-compare in this file,
but once we move these functions into "odb/source-files.c" they will
result in warnings.
Fix those issues:
- In `too_many_loose_objects()` we receive a signed limit, but compare
it with the unsigned actual number of loose objects. This is fixed
by bailing out immediately when the limit is smaller than or equal
to zero, which we also do similarly in other places. The warning is
then squelched via a cast.
- In `find_base_packs()` we compare the signed size of the pack
against the unsigned limit. As the pack size is always going to be a
positive file size it's safe to cast it to an unsigned value.
- In `odb_optimize()` we compare the unsigned `keep_pack.nr` value
against the signed `gc_auto_pack_limit`. We only reach this code
when `too_many_packs()` returns true-ish, and that can only happen
when `gc_auto_pack_limit > 0`. Consequently, we can fix the warning
by casting the limit to an unsigned value.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 3207182488..8cf3781313 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -430,19 +430,21 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
static int too_many_loose_objects(struct odb_source_files *files, int limit)
{
- /*
- * This is weird, but stems from legacy behaviour: the GC auto
- * threshold was always essentially interpreted as if it was rounded up
- * to the next multiple 256 of, so we retain this behaviour for now.
- */
- int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
unsigned long loose_count;
+ if (limit <= 0)
+ return 0;
+
if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
&loose_count) < 0)
return 0;
- return loose_count > auto_threshold;
+ /*
+ * This is weird, but stems from legacy behaviour: the GC auto
+ * threshold was always essentially interpreted as if it was rounded up
+ * to the next multiple 256 of, so we retain this behaviour for now.
+ */
+ return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
}
static struct packed_git *find_base_packs(struct odb_source_files *files,
@@ -456,7 +458,7 @@ static struct packed_git *find_base_packs(struct odb_source_files *files,
if (e->pack->is_cruft)
continue;
if (limit) {
- if (e->pack->pack_size >= limit)
+ if ((uintmax_t) e->pack->pack_size >= limit)
string_list_append(packs, e->pack->pack_name);
} else if (!base || base->pack_size < e->pack->pack_size) {
base = e->pack;
@@ -946,7 +948,7 @@ static int odb_optimize(struct object_database *odb,
if (big_pack_threshold) {
find_base_packs(files, &keep_pack, big_pack_threshold);
- if (keep_pack.nr >= gc_auto_pack_limit) {
+ if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
string_list_clear(&keep_pack, 0);
find_base_packs(files, &keep_pack, 0);
}
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 09/11] builtin/gc: refactor ODB optimizations to operate on "files" source
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
We have a couple of functions that are implementation details of how the
"files" object database source performs optimizations. These functions
often use global state like `the_repository` and implicitly derive the
source they are supposed to optimize.
Refactor these interfaces to accept a "files" source directly. This will
make it easier to move around the whole logic into "odb/source-files.c"
in a subsequent step.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 79 +++++++++++++++++++++++++++++++-----------------------------
1 file changed, 41 insertions(+), 38 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index e119930adc..3207182488 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -428,9 +428,8 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
return should_gc;
}
-static int too_many_loose_objects(int limit)
+static int too_many_loose_objects(struct odb_source_files *files, int limit)
{
- struct odb_source_files *files = odb_source_files_downcast(the_repository->objects->sources);
/*
* This is weird, but stems from legacy behaviour: the GC auto
* threshold was always essentially interpreted as if it was rounded up
@@ -446,19 +445,21 @@ static int too_many_loose_objects(int limit)
return loose_count > auto_threshold;
}
-static struct packed_git *find_base_packs(struct string_list *packs,
+static struct packed_git *find_base_packs(struct odb_source_files *files,
+ struct string_list *packs,
unsigned long limit)
{
- struct packed_git *p, *base = NULL;
+ struct packfile_list_entry *e;
+ struct packed_git *base = NULL;
- repo_for_each_pack(the_repository, p) {
- if (!p->pack_local || p->is_cruft)
+ for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+ if (e->pack->is_cruft)
continue;
if (limit) {
- if (p->pack_size >= limit)
- string_list_append(packs, p->pack_name);
- } else if (!base || base->pack_size < p->pack_size) {
- base = p;
+ if (e->pack->pack_size >= limit)
+ string_list_append(packs, e->pack->pack_name);
+ } else if (!base || base->pack_size < e->pack->pack_size) {
+ base = e->pack;
}
}
@@ -468,18 +469,16 @@ static struct packed_git *find_base_packs(struct string_list *packs,
return base;
}
-static int too_many_packs(int gc_auto_pack_limit)
+static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
{
- struct packed_git *p;
+ struct packfile_list_entry *e;
int cnt = 0;
if (gc_auto_pack_limit <= 0)
return 0;
- repo_for_each_pack(the_repository, p) {
- if (!p->pack_local)
- continue;
- if (p->pack_keep)
+ for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+ if (e->pack->pack_keep)
continue;
/*
* Perhaps check the size of the pack and count only
@@ -535,15 +534,16 @@ static uint64_t total_ram(void)
return 0;
}
-static uint64_t estimate_repack_memory(struct packed_git *pack)
+static uint64_t estimate_repack_memory(struct odb_source_files *files,
+ struct packed_git *pack)
{
unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
unsigned long nr_objects;
size_t os_cache, heap;
- if (odb_count_objects(the_repository->objects,
- ODB_COUNT_OBJECTS_APPROXIMATE, &nr_objects) < 0)
+ if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+ &nr_objects) < 0)
return 0;
if (!pack || !nr_objects)
@@ -679,6 +679,8 @@ static void add_repack_incremental_option(struct strvec *args)
static bool odb_optimize_required(struct object_database *odb,
const struct odb_optimize_options *opts)
{
+ struct odb_source_files *files = odb_source_files_downcast(odb->sources);
+
switch (opts->strategy) {
case ODB_OPTIMIZE_INCREMENTAL: {
int gc_auto_threshold = 6700;
@@ -693,8 +695,8 @@ static bool odb_optimize_required(struct object_database *odb,
*/
if (gc_auto_threshold <= 0)
return false;
- if (!too_many_packs(gc_auto_pack_limit) &&
- !too_many_loose_objects(gc_auto_threshold))
+ if (!too_many_packs(files, gc_auto_pack_limit) &&
+ !too_many_loose_objects(files, gc_auto_threshold))
return false;
return true;
@@ -739,7 +741,7 @@ static bool odb_optimize_required(struct object_database *odb,
* Otherwise, we estimate the number of loose objects to determine
* whether we want to create a new packfile or not.
*/
- if (too_many_loose_objects(auto_value)) {
+ if (too_many_loose_objects(files, auto_value)) {
ret = true;
goto out;
}
@@ -886,21 +888,22 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
static int odb_optimize(struct object_database *odb,
const struct odb_optimize_options *opts)
{
+ struct odb_source_files *files = odb_source_files_downcast(odb->sources);
struct child_process repack_cmd = CHILD_PROCESS_INIT;
unsigned long big_pack_threshold = 0;
int gc_auto_threshold = 6700;
int gc_auto_pack_limit = 50;
int ret;
- repo_config_get_int(the_repository, "gc.auto", &gc_auto_threshold);
- repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
- repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
+ repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
+ repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
+ repo_config_get_ulong(odb->repo, "gc.bigpackthreshold", &big_pack_threshold);
if (odb->repo->repository_format_precious_objects)
return 0;
repack_cmd.git_cmd = 1;
- repack_cmd.odb_to_close = the_repository->objects;
+ repack_cmd.odb_to_close = odb->repo->objects;
strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
@@ -930,29 +933,29 @@ static int odb_optimize(struct object_database *odb,
if (opts->keep_largest_pack != -1) {
if (opts->keep_largest_pack)
- find_base_packs(&keep_pack, 0);
+ find_base_packs(files, &keep_pack, 0);
} else if (big_pack_threshold) {
- find_base_packs(&keep_pack, big_pack_threshold);
+ find_base_packs(files, &keep_pack, big_pack_threshold);
}
add_repack_all_option(opts, &keep_pack, &repack_cmd.args);
string_list_clear(&keep_pack, 0);
} else {
- if (too_many_packs(gc_auto_pack_limit)) {
+ if (too_many_packs(files, gc_auto_pack_limit)) {
struct string_list keep_pack = STRING_LIST_INIT_NODUP;
if (big_pack_threshold) {
- find_base_packs(&keep_pack, big_pack_threshold);
+ find_base_packs(files, &keep_pack, big_pack_threshold);
if (keep_pack.nr >= gc_auto_pack_limit) {
string_list_clear(&keep_pack, 0);
- find_base_packs(&keep_pack, 0);
+ find_base_packs(files, &keep_pack, 0);
}
} else {
- struct packed_git *p = find_base_packs(&keep_pack, 0);
+ struct packed_git *p = find_base_packs(files, &keep_pack, 0);
uint64_t mem_have, mem_want;
mem_have = total_ram();
- mem_want = estimate_repack_memory(p);
+ mem_want = estimate_repack_memory(files, p);
/*
* Only allow 1/2 of memory for pack-objects, leave
@@ -981,10 +984,10 @@ static int odb_optimize(struct object_database *odb,
struct existing_packs existing_packs = EXISTING_PACKS_INIT;
struct string_list kept_packs = STRING_LIST_INIT_DUP;
- repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor",
+ repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
&geometry.split_factor);
- existing_packs.repo = the_repository;
+ existing_packs.repo = odb->repo;
existing_packs_collect(&existing_packs, &kept_packs);
pack_geometry_init(&geometry, &existing_packs, &po_args);
pack_geometry_split(&geometry);
@@ -995,7 +998,7 @@ static int odb_optimize(struct object_database *odb,
} else {
add_repack_all_option(opts, NULL, &repack_cmd.args);
}
- if (the_repository->settings.core_multi_pack_index)
+ if (odb->repo->settings.core_multi_pack_index)
strvec_push(&repack_cmd.args, "--write-midx");
existing_packs_release(&existing_packs);
@@ -1020,7 +1023,7 @@ static int odb_optimize(struct object_database *odb,
strvec_push(&prune_cmd.args, opts->prune_expire);
if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
strvec_push(&prune_cmd.args, "--no-progress");
- if (repo_has_promisor_remote(the_repository))
+ if (repo_has_promisor_remote(odb->repo))
strvec_push(&prune_cmd.args,
"--exclude-promisor-objects");
prune_cmd.git_cmd = 1;
@@ -1031,7 +1034,7 @@ static int odb_optimize(struct object_database *odb,
}
}
- if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(gc_auto_threshold))
+ if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
warning(_("There are too many unreachable loose objects; "
"run 'git prune' to remove them."));
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 08/11] builtin/gc: introduce `odb_optimize_required()`
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
When invoking either git-gc(1) or git-maintenance(1) with the "--auto"
flag then we only perform those maintenance tasks that are actually
required. This logic is inherently an implementation detail of the
object database backend that's in use. But the logic is scattered around
multiple different functions, which makes it hard to make the logic
pluggable.
Introduce a new `odb_optimize_required()` function that allows us to
check these conditions in a generic way.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 160 ++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 92 insertions(+), 68 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index c8504f4456..e119930adc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -676,25 +676,84 @@ static void add_repack_incremental_option(struct strvec *args)
strvec_push(args, "--no-write-bitmap-index");
}
-static int need_to_gc(struct repository *repo)
+static bool odb_optimize_required(struct object_database *odb,
+ const struct odb_optimize_options *opts)
{
- int gc_auto_threshold = 6700;
- int gc_auto_pack_limit = 50;
+ switch (opts->strategy) {
+ case ODB_OPTIMIZE_INCREMENTAL: {
+ int gc_auto_threshold = 6700;
+ int gc_auto_pack_limit = 50;
- repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
- repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+ repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
+ repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
- /*
- * Setting gc.auto to 0 or negative can disable the
- * automatic gc.
- */
- if (gc_auto_threshold <= 0)
- return 0;
- if (!too_many_packs(gc_auto_pack_limit) &&
- !too_many_loose_objects(gc_auto_threshold))
- return 0;
+ /*
+ * Setting gc.auto to 0 or negative can disable the
+ * automatic gc.
+ */
+ if (gc_auto_threshold <= 0)
+ return false;
+ if (!too_many_packs(gc_auto_pack_limit) &&
+ !too_many_loose_objects(gc_auto_threshold))
+ return false;
- return 1;
+ return true;
+ }
+ case ODB_OPTIMIZE_GEOMETRIC: {
+ struct pack_geometry geometry = {
+ .split_factor = 2,
+ };
+ struct pack_objects_args po_args = {
+ .local = 1,
+ };
+ struct existing_packs existing_packs = EXISTING_PACKS_INIT;
+ struct string_list kept_packs = STRING_LIST_INIT_DUP;
+ int auto_value = 100;
+ bool ret;
+
+ repo_config_get_int(odb->repo, "maintenance.geometric-repack.auto",
+ &auto_value);
+ if (!auto_value)
+ return false;
+ if (auto_value < 0)
+ return true;
+
+ repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
+ &geometry.split_factor);
+
+ existing_packs.repo = odb->repo;
+ existing_packs_collect(&existing_packs, &kept_packs);
+ pack_geometry_init(&geometry, &existing_packs, &po_args);
+ pack_geometry_split(&geometry);
+
+ /*
+ * When we'd merge at least two packs with one another we always
+ * perform the repack.
+ */
+ if (geometry.split) {
+ ret = true;
+ goto out;
+ }
+
+ /*
+ * Otherwise, we estimate the number of loose objects to determine
+ * whether we want to create a new packfile or not.
+ */
+ if (too_many_loose_objects(auto_value)) {
+ ret = true;
+ goto out;
+ }
+
+ ret = false;
+
+ out:
+ existing_packs_release(&existing_packs);
+ pack_geometry_release(&geometry);
+ return ret;
+ }
+ default:
+ BUG("unknown maintenance strategy '%d'", opts->strategy);
+ }
}
/* return NULL on success, else hostname running the gc */
@@ -1076,13 +1135,19 @@ int cmd_gc(int argc,
die(_("failed to parse prune expiry value %s"), cfg.prune_expire);
if (opts.auto_flag) {
+ struct odb_optimize_options optimize_opts = {
+ .strategy = ODB_OPTIMIZE_INCREMENTAL,
+ OPTIMIZE_FIELDS_FROM_GC_CONFIG(&cfg, 0),
+ };
+
if (cfg.detach_auto && opts.detach < 0)
opts.detach = 1;
/*
* Auto-gc should be least intrusive as possible.
*/
- if (!need_to_gc(the_repository) || run_hooks(the_repository, "pre-auto-gc")) {
+ if (!odb_optimize_required(the_repository->objects, &optimize_opts) ||
+ run_hooks(the_repository, "pre-auto-gc")) {
ret = 0;
goto out;
}
@@ -1379,9 +1444,13 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
return run_command(&child);
}
-static int gc_condition(struct gc_config *cfg UNUSED)
+static int gc_condition(struct gc_config *cfg)
{
- return need_to_gc(the_repository);
+ struct odb_optimize_options opts = {
+ .strategy = ODB_OPTIMIZE_INCREMENTAL,
+ OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0),
+ };
+ return odb_optimize_required(the_repository->objects, &opts);
}
static int prune_packed(struct maintenance_run_opts *opts)
@@ -1681,58 +1750,13 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
return odb_optimize(the_repository->objects, &odb_opts);
}
-static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED)
+static int geometric_repack_auto_condition(struct gc_config *cfg)
{
- struct pack_geometry geometry = {
- .split_factor = 2,
- };
- struct pack_objects_args po_args = {
- .local = 1,
+ struct odb_optimize_options opts = {
+ .strategy = ODB_OPTIMIZE_GEOMETRIC,
+ OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0),
};
- struct existing_packs existing_packs = EXISTING_PACKS_INIT;
- struct string_list kept_packs = STRING_LIST_INIT_DUP;
- int auto_value = 100;
- int ret;
-
- repo_config_get_int(the_repository, "maintenance.geometric-repack.auto",
- &auto_value);
- if (!auto_value)
- return 0;
- if (auto_value < 0)
- return 1;
-
- repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor",
- &geometry.split_factor);
-
- existing_packs.repo = the_repository;
- existing_packs_collect(&existing_packs, &kept_packs);
- pack_geometry_init(&geometry, &existing_packs, &po_args);
- pack_geometry_split(&geometry);
-
- /*
- * When we'd merge at least two packs with one another we always
- * perform the repack.
- */
- if (geometry.split) {
- ret = 1;
- goto out;
- }
-
- /*
- * Otherwise, we estimate the number of loose objects to determine
- * whether we want to create a new packfile or not.
- */
- if (too_many_loose_objects(auto_value)) {
- ret = 1;
- goto out;
- }
-
- ret = 0;
-
-out:
- existing_packs_release(&existing_packs);
- pack_geometry_release(&geometry);
- return ret;
+ return odb_optimize_required(the_repository->objects, &opts);
}
typedef int (*maintenance_task_fn)(struct maintenance_run_opts *opts,
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 07/11] builtin/gc: move geometric repacking into `odb_optimize()`
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
We have two major object database optimization strategies:
- The legacy strategy used by git-gc(1), which absorbs loose objects
into packfiles, and eventually merges all packfiles once we have too
many of them.
- The more recent "geometric" strategy used by git-maintenance(1),
which merges packfiles using a geometric sequence.
These two strategies are still using completely separate code paths. In
a subsequent commit we'll want to make both strategies pluggable though.
Prepare for this change by merging the "geometric" strategy into
`odb_optimize()`. This also allows us to reuse some of the logic we have
in that function.
Note that this change requires us to adapt tests because we're now using
"-q" instead of "--quiet". Naturally though, these invocations are of
course equivalent to one another.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 171 +++++++++++++++++++++++++------------------------
t/t7900-maintenance.sh | 22 +++----
2 files changed, 98 insertions(+), 95 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 17490106fc..c8504f4456 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -593,6 +593,11 @@ static int keep_one_pack(struct string_list_item *item, void *data)
return 0;
}
+enum odb_optimize_strategy {
+ ODB_OPTIMIZE_INCREMENTAL,
+ ODB_OPTIMIZE_GEOMETRIC,
+};
+
enum odb_optimize_flags {
/* Enable verbose logging and progress reporting. */
ODB_OPTIMIZE_VERBOSE = (1 << 0),
@@ -605,6 +610,7 @@ enum odb_optimize_flags {
};
struct odb_optimize_options {
+ enum odb_optimize_strategy strategy;
enum odb_optimize_flags flags;
const char *prune_expire;
const char *expire_to;
@@ -858,49 +864,87 @@ static int odb_optimize(struct object_database *odb,
*
* - Otherwise we perform an incremental repack.
*/
- if (!(opts->flags & ODB_OPTIMIZE_AUTO)) {
- struct string_list keep_pack = STRING_LIST_INIT_NODUP;
-
- if (opts->keep_largest_pack != -1) {
- if (opts->keep_largest_pack)
- find_base_packs(&keep_pack, 0);
- } else if (big_pack_threshold) {
- find_base_packs(&keep_pack, big_pack_threshold);
- }
-
- add_repack_all_option(opts, &keep_pack, &repack_cmd.args);
- string_list_clear(&keep_pack, 0);
- } else {
- if (too_many_packs(gc_auto_pack_limit)) {
+ switch (opts->strategy) {
+ case ODB_OPTIMIZE_INCREMENTAL:
+ if (!(opts->flags & ODB_OPTIMIZE_AUTO)) {
struct string_list keep_pack = STRING_LIST_INIT_NODUP;
- if (big_pack_threshold) {
- find_base_packs(&keep_pack, big_pack_threshold);
- if (keep_pack.nr >= gc_auto_pack_limit) {
- string_list_clear(&keep_pack, 0);
+ if (opts->keep_largest_pack != -1) {
+ if (opts->keep_largest_pack)
find_base_packs(&keep_pack, 0);
- }
- } else {
- struct packed_git *p = find_base_packs(&keep_pack, 0);
- uint64_t mem_have, mem_want;
-
- mem_have = total_ram();
- mem_want = estimate_repack_memory(p);
-
- /*
- * Only allow 1/2 of memory for pack-objects, leave
- * the rest for the OS and other processes in the
- * system.
- */
- if (!mem_have || mem_want < mem_have / 2)
- string_list_clear(&keep_pack, 0);
+ } else if (big_pack_threshold) {
+ find_base_packs(&keep_pack, big_pack_threshold);
}
add_repack_all_option(opts, &keep_pack, &repack_cmd.args);
string_list_clear(&keep_pack, 0);
} else {
- add_repack_incremental_option(&repack_cmd.args);
+ if (too_many_packs(gc_auto_pack_limit)) {
+ struct string_list keep_pack = STRING_LIST_INIT_NODUP;
+
+ if (big_pack_threshold) {
+ find_base_packs(&keep_pack, big_pack_threshold);
+ if (keep_pack.nr >= gc_auto_pack_limit) {
+ string_list_clear(&keep_pack, 0);
+ find_base_packs(&keep_pack, 0);
+ }
+ } else {
+ struct packed_git *p = find_base_packs(&keep_pack, 0);
+ uint64_t mem_have, mem_want;
+
+ mem_have = total_ram();
+ mem_want = estimate_repack_memory(p);
+
+ /*
+ * Only allow 1/2 of memory for pack-objects, leave
+ * the rest for the OS and other processes in the
+ * system.
+ */
+ if (!mem_have || mem_want < mem_have / 2)
+ string_list_clear(&keep_pack, 0);
+ }
+
+ add_repack_all_option(opts, &keep_pack, &repack_cmd.args);
+ string_list_clear(&keep_pack, 0);
+ } else {
+ add_repack_incremental_option(&repack_cmd.args);
+ }
}
+
+ break;
+ case ODB_OPTIMIZE_GEOMETRIC: {
+ struct pack_geometry geometry = {
+ .split_factor = 2,
+ };
+ struct pack_objects_args po_args = {
+ .local = 1,
+ };
+ struct existing_packs existing_packs = EXISTING_PACKS_INIT;
+ struct string_list kept_packs = STRING_LIST_INIT_DUP;
+
+ repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor",
+ &geometry.split_factor);
+
+ existing_packs.repo = the_repository;
+ existing_packs_collect(&existing_packs, &kept_packs);
+ pack_geometry_init(&geometry, &existing_packs, &po_args);
+ pack_geometry_split(&geometry);
+
+ if (geometry.split < geometry.pack_nr) {
+ strvec_pushf(&repack_cmd.args, "--geometric=%d",
+ geometry.split_factor);
+ } else {
+ add_repack_all_option(opts, NULL, &repack_cmd.args);
+ }
+ if (the_repository->settings.core_multi_pack_index)
+ strvec_push(&repack_cmd.args, "--write-midx");
+
+ existing_packs_release(&existing_packs);
+ pack_geometry_release(&geometry);
+ break;
+ }
+ default:
+ die("unknown maintenance strategy '%d'", opts->strategy);
}
if (run_command(&repack_cmd)) {
@@ -908,7 +952,8 @@ static int odb_optimize(struct object_database *odb,
goto out;
}
- if (opts->prune_expire) {
+ /* Geometric repacking uses cruft packs, so we don't have to prune separately. */
+ if (opts->strategy != ODB_OPTIMIZE_GEOMETRIC && opts->prune_expire) {
struct child_process prune_cmd = CHILD_PROCESS_INIT;
strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
@@ -943,6 +988,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
int aggressive)
{
struct odb_optimize_options odb_opts = {
+ .strategy = ODB_OPTIMIZE_INCREMENTAL,
.keep_largest_pack = keep_largest_pack,
OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive),
};
@@ -1624,58 +1670,15 @@ static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts
static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
struct gc_config *cfg)
{
- struct pack_geometry geometry = {
- .split_factor = 2,
- };
- struct pack_objects_args po_args = {
- .local = 1,
+ struct odb_optimize_options odb_opts = {
+ .strategy = ODB_OPTIMIZE_GEOMETRIC,
+ OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0),
};
- struct existing_packs existing_packs = EXISTING_PACKS_INIT;
- struct string_list kept_packs = STRING_LIST_INIT_DUP;
- struct child_process child = CHILD_PROCESS_INIT;
- int ret;
-
- repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor",
- &geometry.split_factor);
-
- existing_packs.repo = the_repository;
- existing_packs_collect(&existing_packs, &kept_packs);
- pack_geometry_init(&geometry, &existing_packs, &po_args);
- pack_geometry_split(&geometry);
-
- child.git_cmd = 1;
- child.odb_to_close = the_repository->objects;
-
- strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
- if (geometry.split < geometry.pack_nr) {
- strvec_pushf(&child.args, "--geometric=%d",
- geometry.split_factor);
- } else {
- struct odb_optimize_options odb_opts = {
- OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0),
- };
- if (!opts->quiet)
- odb_opts.flags |= ODB_OPTIMIZE_VERBOSE;
-
- add_repack_all_option(&odb_opts, NULL, &child.args);
- }
- if (opts->quiet)
- strvec_push(&child.args, "--quiet");
- if (the_repository->settings.core_multi_pack_index)
- strvec_push(&child.args, "--write-midx");
-
- if (run_command(&child)) {
- ret = error(_("failed to perform geometric repack"));
- goto out;
- }
-
- ret = 0;
+ if (!opts->quiet)
+ odb_opts.flags |= ODB_OPTIMIZE_VERBOSE;
-out:
- existing_packs_release(&existing_packs);
- pack_geometry_release(&geometry);
- return ret;
+ return odb_optimize(the_repository->objects, &odb_opts);
}
static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 1212b306b6..e0251064c7 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -556,8 +556,8 @@ run_and_verify_geometric_pack () {
rm -f "trace2.txt" &&
GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
git maintenance run --task=geometric-repack 2>/dev/null &&
- test_subcommand git repack -d -l --geometric=2 \
- --quiet --write-midx <trace2.txt &&
+ test_subcommand git repack -d -l -q --geometric=2 \
+ --write-midx <trace2.txt &&
# Verify that the number of packfiles matches our expectation.
ls -l .git/objects/pack/*.pack >packfiles &&
@@ -588,8 +588,8 @@ test_expect_success 'geometric repacking task' '
# The initial repack causes an all-into-one repack.
GIT_TRACE2_EVENT="$(pwd)/initial-repack.txt" \
git maintenance run --task=geometric-repack 2>/dev/null &&
- test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \
- --quiet --write-midx <initial-repack.txt &&
+ test_subcommand git repack -d -l -q --cruft --cruft-expiration=2.weeks.ago \
+ --write-midx <initial-repack.txt &&
# Repacking should now cause a no-op geometric repack because
# no packfiles need to be combined.
@@ -609,8 +609,8 @@ test_expect_success 'geometric repacking task' '
# an all-into-one-repack.
GIT_TRACE2_EVENT="$(pwd)/all-into-one-repack.txt" \
git maintenance run --task=geometric-repack 2>/dev/null &&
- test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \
- --quiet --write-midx <all-into-one-repack.txt &&
+ test_subcommand git repack -d -l -q --cruft --cruft-expiration=2.weeks.ago \
+ --write-midx <all-into-one-repack.txt &&
# The geometric repack soaks up unreachable objects.
echo blob-1 | git hash-object -w --stdin -t blob &&
@@ -644,8 +644,8 @@ test_expect_success 'geometric repacking task' '
run_and_verify_geometric_pack 3 &&
GIT_TRACE2_EVENT="$(pwd)/cruft-repack.txt" \
git maintenance run --task=geometric-repack 2>/dev/null &&
- test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \
- --quiet --write-midx <cruft-repack.txt &&
+ test_subcommand git repack -d -l -q --cruft --cruft-expiration=2.weeks.ago \
+ --write-midx <cruft-repack.txt &&
ls .git/objects/pack/*.pack >packs &&
test_line_count = 2 packs &&
ls .git/objects/pack/*.mtimes >cruft &&
@@ -736,7 +736,7 @@ test_expect_success 'geometric repacking honors configured split factor' '
test_geometric_repack_needed false splitFactor=2 &&
test_geometric_repack_needed true splitFactor=3 &&
- test_subcommand git repack -d -l --geometric=3 --quiet --write-midx <trace2.txt
+ test_subcommand git repack -d -l -q --geometric=3 --write-midx <trace2.txt
)
'
@@ -1167,7 +1167,7 @@ test_expect_success 'maintenance.strategy is respected' '
test_strategy geometric <<-\EOF &&
git pack-refs --all --prune
git reflog expire --all
- git repack -d -l --geometric=2 --quiet --write-midx
+ git repack -d -l -q --geometric=2 --write-midx
git commit-graph write --split --reachable --no-progress
git worktree prune --expire 3.months.ago
git rerere gc
@@ -1176,7 +1176,7 @@ test_expect_success 'maintenance.strategy is respected' '
test_strategy geometric --schedule=weekly <<-\EOF
git pack-refs --all --prune
git reflog expire --all
- git repack -d -l --geometric=2 --quiet --write-midx
+ git repack -d -l -q --geometric=2 --write-midx
git commit-graph write --split --reachable --no-progress
git worktree prune --expire 3.months.ago
git rerere gc
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 06/11] builtin/gc: introduce object database optimization options
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
Introduce `struct odb_optimize_options` to decouple the options that are
specific to optimizing the object database from `struct gc_config`. This
structure will be moved into the object database layer in a subsequent
commit.
Note that there are a small set of backend-specific options in this
structure. In an ideal world those of course wouldn't exist, but as
we're introducing the object database abstractions retroactively we are
somewhat forced to keep them.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 181 +++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 120 insertions(+), 61 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 5d445edaa0..17490106fc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -593,7 +593,39 @@ static int keep_one_pack(struct string_list_item *item, void *data)
return 0;
}
-static void add_repack_all_option(struct gc_config *cfg,
+enum odb_optimize_flags {
+ /* Enable verbose logging and progress reporting. */
+ ODB_OPTIMIZE_VERBOSE = (1 << 0),
+
+ /* Perform auto-maintenance, only optimizing objects as required. */
+ ODB_OPTIMIZE_AUTO = (1 << 1),
+
+ /* Recompute existing deltas. */
+ ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
+};
+
+struct odb_optimize_options {
+ enum odb_optimize_flags flags;
+ const char *prune_expire;
+ const char *expire_to;
+ int depth;
+ int window;
+
+ /* Backend-specific options. */
+ int keep_largest_pack;
+ int cruft_packs;
+ unsigned long max_cruft_size;
+};
+
+#define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
+ .prune_expire = (cfg)->prune_expire, \
+ .expire_to = (cfg)->repack_expire_to, \
+ .cruft_packs = (cfg)->cruft_packs, \
+ .max_cruft_size = (cfg)->max_cruft_size, \
+ .window = (aggressive) ? (cfg)->aggressive_window : 0, \
+ .depth = (aggressive) ? (cfg)->aggressive_depth : 0
+
+static void add_repack_all_option(const struct odb_optimize_options *opts,
struct string_list *keep_pack,
struct strvec *args)
{
@@ -603,22 +635,22 @@ static void add_repack_all_option(struct gc_config *cfg,
repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
- if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now")
- && !(cfg->cruft_packs && cfg->repack_expire_to))
+ if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
+ !(opts->cruft_packs && opts->expire_to))
strvec_push(args, "-a");
- else if (cfg->cruft_packs) {
+ else if (opts->cruft_packs) {
strvec_push(args, "--cruft");
- if (cfg->prune_expire)
- strvec_pushf(args, "--cruft-expiration=%s", cfg->prune_expire);
- if (cfg->max_cruft_size)
+ if (opts->prune_expire)
+ strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
+ if (opts->max_cruft_size)
strvec_pushf(args, "--max-cruft-size=%lu",
- cfg->max_cruft_size);
- if (cfg->repack_expire_to)
- strvec_pushf(args, "--expire-to=%s", cfg->repack_expire_to);
+ opts->max_cruft_size);
+ if (opts->expire_to)
+ strvec_pushf(args, "--expire-to=%s", opts->expire_to);
} else {
strvec_push(args, "-A");
- if (cfg->prune_expire)
- strvec_pushf(args, "--unpack-unreachable=%s", cfg->prune_expire);
+ if (opts->prune_expire)
+ strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
}
if (keep_pack)
@@ -786,10 +818,8 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
return 0;
}
-static int maintenance_task_odb(struct maintenance_run_opts *opts,
- struct gc_config *cfg,
- int keep_largest_pack,
- int aggressive)
+static int odb_optimize(struct object_database *odb,
+ const struct odb_optimize_options *opts)
{
struct child_process repack_cmd = CHILD_PROCESS_INIT;
unsigned long big_pack_threshold = 0;
@@ -801,21 +831,20 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
- if (the_repository->repository_format_precious_objects)
+ if (odb->repo->repository_format_precious_objects)
return 0;
repack_cmd.git_cmd = 1;
repack_cmd.odb_to_close = the_repository->objects;
strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
- if (aggressive) {
+ if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
strvec_push(&repack_cmd.args, "-f");
- if (cfg->aggressive_depth > 0)
- strvec_pushf(&repack_cmd.args, "--depth=%d", cfg->aggressive_depth);
- if (cfg->aggressive_window > 0)
- strvec_pushf(&repack_cmd.args, "--window=%d", cfg->aggressive_window);
- }
- if (opts->quiet)
+ if (opts->depth > 0)
+ strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
+ if (opts->window > 0)
+ strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
+ if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
strvec_push(&repack_cmd.args, "-q");
/*
@@ -829,47 +858,49 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
*
* - Otherwise we perform an incremental repack.
*/
- if (!opts->auto_flag) {
+ if (!(opts->flags & ODB_OPTIMIZE_AUTO)) {
struct string_list keep_pack = STRING_LIST_INIT_NODUP;
- if (keep_largest_pack != -1) {
- if (keep_largest_pack)
+ if (opts->keep_largest_pack != -1) {
+ if (opts->keep_largest_pack)
find_base_packs(&keep_pack, 0);
} else if (big_pack_threshold) {
find_base_packs(&keep_pack, big_pack_threshold);
}
- add_repack_all_option(cfg, &keep_pack, &repack_cmd.args);
+ add_repack_all_option(opts, &keep_pack, &repack_cmd.args);
string_list_clear(&keep_pack, 0);
- } else if (too_many_packs(gc_auto_pack_limit)) {
- struct string_list keep_pack = STRING_LIST_INIT_NODUP;
-
- if (big_pack_threshold) {
- find_base_packs(&keep_pack, big_pack_threshold);
- if (keep_pack.nr >= gc_auto_pack_limit) {
- string_list_clear(&keep_pack, 0);
- find_base_packs(&keep_pack, 0);
+ } else {
+ if (too_many_packs(gc_auto_pack_limit)) {
+ struct string_list keep_pack = STRING_LIST_INIT_NODUP;
+
+ if (big_pack_threshold) {
+ find_base_packs(&keep_pack, big_pack_threshold);
+ if (keep_pack.nr >= gc_auto_pack_limit) {
+ string_list_clear(&keep_pack, 0);
+ find_base_packs(&keep_pack, 0);
+ }
+ } else {
+ struct packed_git *p = find_base_packs(&keep_pack, 0);
+ uint64_t mem_have, mem_want;
+
+ mem_have = total_ram();
+ mem_want = estimate_repack_memory(p);
+
+ /*
+ * Only allow 1/2 of memory for pack-objects, leave
+ * the rest for the OS and other processes in the
+ * system.
+ */
+ if (!mem_have || mem_want < mem_have / 2)
+ string_list_clear(&keep_pack, 0);
}
- } else {
- struct packed_git *p = find_base_packs(&keep_pack, 0);
- uint64_t mem_have, mem_want;
-
- mem_have = total_ram();
- mem_want = estimate_repack_memory(p);
- /*
- * Only allow 1/2 of memory for pack-objects, leave
- * the rest for the OS and other processes in the
- * system.
- */
- if (!mem_have || mem_want < mem_have / 2)
- string_list_clear(&keep_pack, 0);
+ add_repack_all_option(opts, &keep_pack, &repack_cmd.args);
+ string_list_clear(&keep_pack, 0);
+ } else {
+ add_repack_incremental_option(&repack_cmd.args);
}
-
- add_repack_all_option(cfg, &keep_pack, &repack_cmd.args);
- string_list_clear(&keep_pack, 0);
- } else {
- add_repack_incremental_option(&repack_cmd.args);
}
if (run_command(&repack_cmd)) {
@@ -877,13 +908,13 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
goto out;
}
- if (cfg->prune_expire) {
+ if (opts->prune_expire) {
struct child_process prune_cmd = CHILD_PROCESS_INIT;
strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
/* run `git prune` even if using cruft packs */
- strvec_push(&prune_cmd.args, cfg->prune_expire);
- if (opts->quiet)
+ strvec_push(&prune_cmd.args, opts->prune_expire);
+ if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
strvec_push(&prune_cmd.args, "--no-progress");
if (repo_has_promisor_remote(the_repository))
strvec_push(&prune_cmd.args,
@@ -896,7 +927,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
}
}
- if (opts->auto_flag && too_many_loose_objects(gc_auto_threshold))
+ if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(gc_auto_threshold))
warning(_("There are too many unreachable loose objects; "
"run 'git prune' to remove them."));
@@ -906,6 +937,26 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
return ret;
}
+static int maintenance_task_odb(struct maintenance_run_opts *opts,
+ struct gc_config *cfg,
+ int keep_largest_pack,
+ int aggressive)
+{
+ struct odb_optimize_options odb_opts = {
+ .keep_largest_pack = keep_largest_pack,
+ OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive),
+ };
+
+ if (opts->auto_flag)
+ odb_opts.flags |= ODB_OPTIMIZE_AUTO;
+ if (!opts->quiet)
+ odb_opts.flags |= ODB_OPTIMIZE_VERBOSE;
+ if (aggressive)
+ odb_opts.flags |= ODB_OPTIMIZE_NO_REUSE_DELTAS;
+
+ return odb_optimize(the_repository->objects, &odb_opts);
+}
+
int cmd_gc(int argc,
const char **argv,
const char *prefix,
@@ -1596,11 +1647,19 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
child.odb_to_close = the_repository->objects;
strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
- if (geometry.split < geometry.pack_nr)
+ if (geometry.split < geometry.pack_nr) {
strvec_pushf(&child.args, "--geometric=%d",
geometry.split_factor);
- else
- add_repack_all_option(cfg, NULL, &child.args);
+ } else {
+ struct odb_optimize_options odb_opts = {
+ OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0),
+ };
+
+ if (!opts->quiet)
+ odb_opts.flags |= ODB_OPTIMIZE_VERBOSE;
+
+ add_repack_all_option(&odb_opts, NULL, &child.args);
+ }
if (opts->quiet)
strvec_push(&child.args, "--quiet");
if (the_repository->settings.core_multi_pack_index)
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 05/11] builtin/gc: inline config values specific to the "files" backend
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
The `struct gc_config` contains a set of values that we read via the Git
repository's configuration. Several of those values that are consumed by
the object database optimization logic are inherently specific to the
"files" config.
In a later commit we'll make the logic to optimize object databases
pluggable. So by carrying these "files"-backend specific values in the
generic config struct means that other backends would have to worry
about these values, too. This feels somewhat dirty, as implementation-
specific details should live with the backends themselves.
Inline these values directly at the call sites that need them instead.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 115 +++++++++++++++++++++++++++--------------------------------
1 file changed, 53 insertions(+), 62 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 25a59caea6..5d445edaa0 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -130,22 +130,11 @@ struct gc_config {
unsigned long max_cruft_size;
int aggressive_depth;
int aggressive_window;
- int gc_auto_threshold;
- int gc_auto_pack_limit;
int detach_auto;
char *gc_log_expire;
char *prune_expire;
char *prune_worktrees_expire;
- char *repack_filter;
- char *repack_filter_to;
char *repack_expire_to;
- unsigned long big_pack_threshold;
- unsigned long max_delta_cache_size;
- /*
- * Remove this member from gc_config once repo_settings is passed
- * through the callchain.
- */
- size_t delta_base_cache_limit;
};
#define GC_CONFIG_INIT { \
@@ -154,14 +143,10 @@ struct gc_config {
.cruft_packs = 1, \
.aggressive_depth = 50, \
.aggressive_window = 250, \
- .gc_auto_threshold = 6700, \
- .gc_auto_pack_limit = 50, \
.detach_auto = 1, \
.gc_log_expire = xstrdup("1.day.ago"), \
.prune_expire = xstrdup("2.weeks.ago"), \
.prune_worktrees_expire = xstrdup("3.months.ago"), \
- .max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \
- .delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \
}
static void gc_config_release(struct gc_config *cfg)
@@ -169,15 +154,12 @@ static void gc_config_release(struct gc_config *cfg)
free(cfg->gc_log_expire);
free(cfg->prune_expire);
free(cfg->prune_worktrees_expire);
- free(cfg->repack_filter);
- free(cfg->repack_filter_to);
}
static void gc_config(struct gc_config *cfg)
{
const char *value;
char *owned = NULL;
- unsigned long ulongval;
if (!repo_config_get_value(the_repository, "gc.packrefs", &value)) {
if (value && !strcmp(value, "notbare"))
@@ -192,8 +174,6 @@ static void gc_config(struct gc_config *cfg)
repo_config_get_int(the_repository, "gc.aggressivewindow", &cfg->aggressive_window);
repo_config_get_int(the_repository, "gc.aggressivedepth", &cfg->aggressive_depth);
- repo_config_get_int(the_repository, "gc.auto", &cfg->gc_auto_threshold);
- repo_config_get_int(the_repository, "gc.autopacklimit", &cfg->gc_auto_pack_limit);
repo_config_get_bool(the_repository, "gc.autodetach", &cfg->detach_auto);
repo_config_get_bool(the_repository, "gc.cruftpacks", &cfg->cruft_packs);
repo_config_get_ulong(the_repository, "gc.maxcruftsize", &cfg->max_cruft_size);
@@ -213,22 +193,6 @@ static void gc_config(struct gc_config *cfg)
cfg->gc_log_expire = owned;
}
- repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &cfg->big_pack_threshold);
- repo_config_get_ulong(the_repository, "pack.deltacachesize", &cfg->max_delta_cache_size);
-
- if (!repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &ulongval))
- cfg->delta_base_cache_limit = ulongval;
-
- if (!repo_config_get_string(the_repository, "gc.repackfilter", &owned)) {
- free(cfg->repack_filter);
- cfg->repack_filter = owned;
- }
-
- if (!repo_config_get_string(the_repository, "gc.repackfilterto", &owned)) {
- free(cfg->repack_filter_to);
- cfg->repack_filter_to = owned;
- }
-
repo_config(the_repository, git_default_config, NULL);
}
@@ -504,12 +468,12 @@ static struct packed_git *find_base_packs(struct string_list *packs,
return base;
}
-static int too_many_packs(struct gc_config *cfg)
+static int too_many_packs(int gc_auto_pack_limit)
{
struct packed_git *p;
int cnt = 0;
- if (cfg->gc_auto_pack_limit <= 0)
+ if (gc_auto_pack_limit <= 0)
return 0;
repo_for_each_pack(the_repository, p) {
@@ -523,7 +487,7 @@ static int too_many_packs(struct gc_config *cfg)
*/
cnt++;
}
- return cfg->gc_auto_pack_limit < cnt;
+ return gc_auto_pack_limit < cnt;
}
static uint64_t total_ram(void)
@@ -571,9 +535,10 @@ static uint64_t total_ram(void)
return 0;
}
-static uint64_t estimate_repack_memory(struct gc_config *cfg,
- struct packed_git *pack)
+static uint64_t estimate_repack_memory(struct packed_git *pack)
{
+ unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
+ unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
unsigned long nr_objects;
size_t os_cache, heap;
@@ -584,6 +549,9 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg,
if (!pack || !nr_objects)
return 0;
+ repo_config_get_ulong(the_repository, "pack.deltacachesize", &max_delta_cache_size);
+ repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &delta_base_cache_limit);
+
/*
* First we have to scan through at least one pack.
* Assume enough room in OS file cache to keep the entire pack
@@ -611,9 +579,9 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg,
* read_sha1_file() (either at delta calculation phase, or
* writing phase) also fills up the delta base cache
*/
- heap += cfg->delta_base_cache_limit;
+ heap += delta_base_cache_limit;
/* and of course pack-objects has its own delta cache */
- heap += cfg->max_delta_cache_size;
+ heap += max_delta_cache_size;
return os_cache + heap;
}
@@ -629,6 +597,12 @@ static void add_repack_all_option(struct gc_config *cfg,
struct string_list *keep_pack,
struct strvec *args)
{
+ char *repack_filter = NULL;
+ char *repack_filter_to = NULL;
+
+ repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
+ repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
+
if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now")
&& !(cfg->cruft_packs && cfg->repack_expire_to))
strvec_push(args, "-a");
@@ -650,10 +624,13 @@ static void add_repack_all_option(struct gc_config *cfg,
if (keep_pack)
for_each_string_list(keep_pack, keep_one_pack, args);
- if (cfg->repack_filter && *cfg->repack_filter)
- strvec_pushf(args, "--filter=%s", cfg->repack_filter);
- if (cfg->repack_filter_to && *cfg->repack_filter_to)
- strvec_pushf(args, "--filter-to=%s", cfg->repack_filter_to);
+ if (repack_filter && *repack_filter)
+ strvec_pushf(args, "--filter=%s", repack_filter);
+ if (repack_filter_to && *repack_filter_to)
+ strvec_pushf(args, "--filter-to=%s", repack_filter_to);
+
+ free(repack_filter);
+ free(repack_filter_to);
}
static void add_repack_incremental_option(struct strvec *args)
@@ -661,16 +638,24 @@ static void add_repack_incremental_option(struct strvec *args)
strvec_push(args, "--no-write-bitmap-index");
}
-static int need_to_gc(struct gc_config *cfg)
+static int need_to_gc(struct repository *repo)
{
+ int gc_auto_threshold = 6700;
+ int gc_auto_pack_limit = 50;
+
+ repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+ repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+
/*
* Setting gc.auto to 0 or negative can disable the
* automatic gc.
*/
- if (cfg->gc_auto_threshold <= 0)
+ if (gc_auto_threshold <= 0)
return 0;
- if (!too_many_packs(cfg) && !too_many_loose_objects(cfg->gc_auto_threshold))
+ if (!too_many_packs(gc_auto_pack_limit) &&
+ !too_many_loose_objects(gc_auto_threshold))
return 0;
+
return 1;
}
@@ -807,8 +792,15 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
int aggressive)
{
struct child_process repack_cmd = CHILD_PROCESS_INIT;
+ unsigned long big_pack_threshold = 0;
+ int gc_auto_threshold = 6700;
+ int gc_auto_pack_limit = 50;
int ret;
+ repo_config_get_int(the_repository, "gc.auto", &gc_auto_threshold);
+ repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
+ repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
+
if (the_repository->repository_format_precious_objects)
return 0;
@@ -843,19 +835,18 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
if (keep_largest_pack != -1) {
if (keep_largest_pack)
find_base_packs(&keep_pack, 0);
- } else if (cfg->big_pack_threshold) {
- find_base_packs(&keep_pack, cfg->big_pack_threshold);
+ } else if (big_pack_threshold) {
+ find_base_packs(&keep_pack, big_pack_threshold);
}
add_repack_all_option(cfg, &keep_pack, &repack_cmd.args);
string_list_clear(&keep_pack, 0);
- } else if (too_many_packs(cfg)) {
+ } else if (too_many_packs(gc_auto_pack_limit)) {
struct string_list keep_pack = STRING_LIST_INIT_NODUP;
- if (cfg->big_pack_threshold) {
- find_base_packs(&keep_pack, cfg->big_pack_threshold);
- if (keep_pack.nr >= cfg->gc_auto_pack_limit) {
- cfg->big_pack_threshold = 0;
+ if (big_pack_threshold) {
+ find_base_packs(&keep_pack, big_pack_threshold);
+ if (keep_pack.nr >= gc_auto_pack_limit) {
string_list_clear(&keep_pack, 0);
find_base_packs(&keep_pack, 0);
}
@@ -864,7 +855,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
uint64_t mem_have, mem_want;
mem_have = total_ram();
- mem_want = estimate_repack_memory(cfg, p);
+ mem_want = estimate_repack_memory(p);
/*
* Only allow 1/2 of memory for pack-objects, leave
@@ -905,7 +896,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
}
}
- if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold))
+ if (opts->auto_flag && too_many_loose_objects(gc_auto_threshold))
warning(_("There are too many unreachable loose objects; "
"run 'git prune' to remove them."));
@@ -994,7 +985,7 @@ int cmd_gc(int argc,
/*
* Auto-gc should be least intrusive as possible.
*/
- if (!need_to_gc(&cfg) || run_hooks(the_repository, "pre-auto-gc")) {
+ if (!need_to_gc(the_repository) || run_hooks(the_repository, "pre-auto-gc")) {
ret = 0;
goto out;
}
@@ -1291,9 +1282,9 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
return run_command(&child);
}
-static int gc_condition(struct gc_config *cfg)
+static int gc_condition(struct gc_config *cfg UNUSED)
{
- return need_to_gc(cfg);
+ return need_to_gc(the_repository);
}
static int prune_packed(struct maintenance_run_opts *opts)
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 04/11] builtin/gc: make repack arguments self-contained
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
When optimizing the object database most of the heavy-lifting is done by
git-repack(1). The arguments we pass to this function are assembled in
global scope, which is hard to follow.
Refactor the logic by moving the vector into `maintenance_task_odb()`.
While that means we have to pass more arguments to this function, it has
the upside that the logic becomes self-contained without any kind of
global interdependencies.
This is a pure refactoring with no intended functional change.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 156 ++++++++++++++++++++++++++++-------------------------------
1 file changed, 75 insertions(+), 81 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 2ff98fa727..25a59caea6 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -661,7 +661,7 @@ static void add_repack_incremental_option(struct strvec *args)
strvec_push(args, "--no-write-bitmap-index");
}
-static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args)
+static int need_to_gc(struct gc_config *cfg)
{
/*
* Setting gc.auto to 0 or negative can disable the
@@ -669,46 +669,8 @@ static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args)
*/
if (cfg->gc_auto_threshold <= 0)
return 0;
-
- /*
- * If there are too many loose objects, but not too many
- * packs, we run "repack -d -l". If there are too many packs,
- * we run "repack -A -d -l". Otherwise we tell the caller
- * there is no need.
- */
- if (too_many_packs(cfg)) {
- struct string_list keep_pack = STRING_LIST_INIT_NODUP;
-
- if (cfg->big_pack_threshold) {
- find_base_packs(&keep_pack, cfg->big_pack_threshold);
- if (keep_pack.nr >= cfg->gc_auto_pack_limit) {
- cfg->big_pack_threshold = 0;
- string_list_clear(&keep_pack, 0);
- find_base_packs(&keep_pack, 0);
- }
- } else {
- struct packed_git *p = find_base_packs(&keep_pack, 0);
- uint64_t mem_have, mem_want;
-
- mem_have = total_ram();
- mem_want = estimate_repack_memory(cfg, p);
-
- /*
- * Only allow 1/2 of memory for pack-objects, leave
- * the rest for the OS and other processes in the
- * system.
- */
- if (!mem_have || mem_want < mem_have / 2)
- string_list_clear(&keep_pack, 0);
- }
-
- add_repack_all_option(cfg, &keep_pack, repack_args);
- string_list_clear(&keep_pack, 0);
- } else if (too_many_loose_objects(cfg->gc_auto_threshold))
- add_repack_incremental_option(repack_args);
- else
+ if (!too_many_packs(cfg) && !too_many_loose_objects(cfg->gc_auto_threshold))
return 0;
-
return 1;
}
@@ -841,7 +803,8 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
static int maintenance_task_odb(struct maintenance_run_opts *opts,
struct gc_config *cfg,
- struct strvec *repack_args)
+ int keep_largest_pack,
+ int aggressive)
{
struct child_process repack_cmd = CHILD_PROCESS_INIT;
int ret;
@@ -851,9 +814,75 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
repack_cmd.git_cmd = 1;
repack_cmd.odb_to_close = the_repository->objects;
- strvec_pushv(&repack_cmd.args, repack_args->v);
+
+ strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
+ if (aggressive) {
+ strvec_push(&repack_cmd.args, "-f");
+ if (cfg->aggressive_depth > 0)
+ strvec_pushf(&repack_cmd.args, "--depth=%d", cfg->aggressive_depth);
+ if (cfg->aggressive_window > 0)
+ strvec_pushf(&repack_cmd.args, "--window=%d", cfg->aggressive_window);
+ }
+ if (opts->quiet)
+ strvec_push(&repack_cmd.args, "-q");
+
+ /*
+ * There's three cases we need to consider:
+ *
+ * - If we're invoked without `--auto` we'll need to perform a full
+ * repack.
+ *
+ * - If we're invoked with `--auto` and there's too many packs, then
+ * we perform a full repack, as well.
+ *
+ * - Otherwise we perform an incremental repack.
+ */
+ if (!opts->auto_flag) {
+ struct string_list keep_pack = STRING_LIST_INIT_NODUP;
+
+ if (keep_largest_pack != -1) {
+ if (keep_largest_pack)
+ find_base_packs(&keep_pack, 0);
+ } else if (cfg->big_pack_threshold) {
+ find_base_packs(&keep_pack, cfg->big_pack_threshold);
+ }
+
+ add_repack_all_option(cfg, &keep_pack, &repack_cmd.args);
+ string_list_clear(&keep_pack, 0);
+ } else if (too_many_packs(cfg)) {
+ struct string_list keep_pack = STRING_LIST_INIT_NODUP;
+
+ if (cfg->big_pack_threshold) {
+ find_base_packs(&keep_pack, cfg->big_pack_threshold);
+ if (keep_pack.nr >= cfg->gc_auto_pack_limit) {
+ cfg->big_pack_threshold = 0;
+ string_list_clear(&keep_pack, 0);
+ find_base_packs(&keep_pack, 0);
+ }
+ } else {
+ struct packed_git *p = find_base_packs(&keep_pack, 0);
+ uint64_t mem_have, mem_want;
+
+ mem_have = total_ram();
+ mem_want = estimate_repack_memory(cfg, p);
+
+ /*
+ * Only allow 1/2 of memory for pack-objects, leave
+ * the rest for the OS and other processes in the
+ * system.
+ */
+ if (!mem_have || mem_want < mem_have / 2)
+ string_list_clear(&keep_pack, 0);
+ }
+
+ add_repack_all_option(cfg, &keep_pack, &repack_cmd.args);
+ string_list_clear(&keep_pack, 0);
+ } else {
+ add_repack_incremental_option(&repack_cmd.args);
+ }
+
if (run_command(&repack_cmd)) {
- ret = error(FAILED_RUN, repack_args->v[0]);
+ ret = error(FAILED_RUN, repack_cmd.args.v[0]);
goto out;
}
@@ -899,7 +928,6 @@ int cmd_gc(int argc,
int keep_largest_pack = -1;
int skip_foreground_tasks = 0;
timestamp_t dummy;
- struct strvec repack_args = STRVEC_INIT;
struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
struct gc_config cfg = GC_CONFIG_INIT;
const char *prune_expire_sentinel = "sentinel";
@@ -939,8 +967,6 @@ int cmd_gc(int argc,
show_usage_with_options_if_asked(argc, argv,
builtin_gc_usage, builtin_gc_options);
- strvec_pushl(&repack_args, "repack", "-d", "-l", NULL);
-
gc_config(&cfg);
if (parse_expiry_date(cfg.gc_log_expire, &gc_log_expire_time))
@@ -961,16 +987,6 @@ int cmd_gc(int argc,
if (cfg.prune_expire && parse_expiry_date(cfg.prune_expire, &dummy))
die(_("failed to parse prune expiry value %s"), cfg.prune_expire);
- if (aggressive) {
- strvec_push(&repack_args, "-f");
- if (cfg.aggressive_depth > 0)
- strvec_pushf(&repack_args, "--depth=%d", cfg.aggressive_depth);
- if (cfg.aggressive_window > 0)
- strvec_pushf(&repack_args, "--window=%d", cfg.aggressive_window);
- }
- if (opts.quiet)
- strvec_push(&repack_args, "-q");
-
if (opts.auto_flag) {
if (cfg.detach_auto && opts.detach < 0)
opts.detach = 1;
@@ -978,8 +994,7 @@ int cmd_gc(int argc,
/*
* Auto-gc should be least intrusive as possible.
*/
- if (!need_to_gc(&cfg, &repack_args) ||
- run_hooks(the_repository, "pre-auto-gc")) {
+ if (!need_to_gc(&cfg) || run_hooks(the_repository, "pre-auto-gc")) {
ret = 0;
goto out;
}
@@ -991,18 +1006,6 @@ int cmd_gc(int argc,
fprintf(stderr, _("Auto packing the repository for optimum performance.\n"));
fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
}
- } else {
- struct string_list keep_pack = STRING_LIST_INIT_NODUP;
-
- if (keep_largest_pack != -1) {
- if (keep_largest_pack)
- find_base_packs(&keep_pack, 0);
- } else if (cfg.big_pack_threshold) {
- find_base_packs(&keep_pack, cfg.big_pack_threshold);
- }
-
- add_repack_all_option(&cfg, &keep_pack, &repack_args);
- string_list_clear(&keep_pack, 0);
}
if (opts.detach > 0) {
@@ -1065,7 +1068,7 @@ int cmd_gc(int argc,
if (maintenance_task_rerere_gc(&opts, &cfg))
die(FAILED_RUN, "rerere");
- if (maintenance_task_odb(&opts, &cfg, &repack_args))
+ if (maintenance_task_odb(&opts, &cfg, keep_largest_pack, aggressive))
die(NULL);
report_garbage = report_pack_garbage;
@@ -1088,7 +1091,6 @@ int cmd_gc(int argc,
out:
maintenance_run_opts_release(&opts);
- strvec_clear(&repack_args);
gc_config_release(&cfg);
return 0;
}
@@ -1291,15 +1293,7 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
static int gc_condition(struct gc_config *cfg)
{
- /*
- * Note that it's fine to drop the repack arguments here, as we execute
- * git-gc(1) as a separate child process anyway. So it knows to compute
- * these arguments again.
- */
- struct strvec repack_args = STRVEC_INIT;
- int ret = need_to_gc(cfg, &repack_args);
- strvec_clear(&repack_args);
- return ret;
+ return need_to_gc(cfg);
}
static int prune_packed(struct maintenance_run_opts *opts)
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 03/11] builtin/gc: extract object database optimizations into separate function
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
Extract the object database optimization logic from `cmd_gc()` into a
new `maintenance_task_odb()` helper function. This is a pure refactoring
with no intended functional change.
Note that the message that notifies the user about too many loose
objects is moved into the new function, as well. It is inherently an
implementation detail of how the "files" source works, and as a
consequence we'll move it around in a later commit, as well. This
reordering means that the warning may now be printed at a different
point in time, but it's not expected that this will have any practical
implications.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 79 +++++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 49 insertions(+), 30 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 8f568003ee..2ff98fa727 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -839,6 +839,53 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
return 0;
}
+static int maintenance_task_odb(struct maintenance_run_opts *opts,
+ struct gc_config *cfg,
+ struct strvec *repack_args)
+{
+ struct child_process repack_cmd = CHILD_PROCESS_INIT;
+ int ret;
+
+ if (the_repository->repository_format_precious_objects)
+ return 0;
+
+ repack_cmd.git_cmd = 1;
+ repack_cmd.odb_to_close = the_repository->objects;
+ strvec_pushv(&repack_cmd.args, repack_args->v);
+ if (run_command(&repack_cmd)) {
+ ret = error(FAILED_RUN, repack_args->v[0]);
+ goto out;
+ }
+
+ if (cfg->prune_expire) {
+ struct child_process prune_cmd = CHILD_PROCESS_INIT;
+
+ strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
+ /* run `git prune` even if using cruft packs */
+ strvec_push(&prune_cmd.args, cfg->prune_expire);
+ if (opts->quiet)
+ strvec_push(&prune_cmd.args, "--no-progress");
+ if (repo_has_promisor_remote(the_repository))
+ strvec_push(&prune_cmd.args,
+ "--exclude-promisor-objects");
+ prune_cmd.git_cmd = 1;
+
+ if (run_command(&prune_cmd)) {
+ ret = error(FAILED_RUN, prune_cmd.args.v[0]);
+ goto out;
+ }
+ }
+
+ if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold))
+ warning(_("There are too many unreachable loose objects; "
+ "run 'git prune' to remove them."));
+
+ ret = 0;
+
+out:
+ return ret;
+}
+
int cmd_gc(int argc,
const char **argv,
const char *prefix,
@@ -1018,32 +1065,8 @@ int cmd_gc(int argc,
if (maintenance_task_rerere_gc(&opts, &cfg))
die(FAILED_RUN, "rerere");
- if (!the_repository->repository_format_precious_objects) {
- struct child_process repack_cmd = CHILD_PROCESS_INIT;
-
- repack_cmd.git_cmd = 1;
- repack_cmd.odb_to_close = the_repository->objects;
- strvec_pushv(&repack_cmd.args, repack_args.v);
- if (run_command(&repack_cmd))
- die(FAILED_RUN, repack_args.v[0]);
-
- if (cfg.prune_expire) {
- struct child_process prune_cmd = CHILD_PROCESS_INIT;
-
- strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
- /* run `git prune` even if using cruft packs */
- strvec_push(&prune_cmd.args, cfg.prune_expire);
- if (opts.quiet)
- strvec_push(&prune_cmd.args, "--no-progress");
- if (repo_has_promisor_remote(the_repository))
- strvec_push(&prune_cmd.args,
- "--exclude-promisor-objects");
- prune_cmd.git_cmd = 1;
-
- if (run_command(&prune_cmd))
- die(FAILED_RUN, prune_cmd.args.v[0]);
- }
- }
+ if (maintenance_task_odb(&opts, &cfg, &repack_args))
+ die(NULL);
report_garbage = report_pack_garbage;
odb_reprepare(the_repository->objects);
@@ -1057,10 +1080,6 @@ int cmd_gc(int argc,
!opts.quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,
NULL);
- if (opts.auto_flag && too_many_loose_objects(cfg.gc_auto_threshold))
- warning(_("There are too many unreachable loose objects; "
- "run 'git prune' to remove them."));
-
if (!daemonized) {
char *path = repo_git_path(the_repository, "gc.log");
unlink(path);
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 02/11] builtin/gc: move worktree and rerere tasks before object optimizations
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
In subsequent patches we'll consolidate all tasks that relate to
maintenance of the object database and move it into the "files" backend.
The relevant code is somewhat scattered though, as several other tasks
are interspersed between.
Refactor the code so that all object database optimizations are grouped
together, which requires us to move worktree pruning and rerere garbage
collection around. In theory, rearranging this code can have an effect
on the object database optimizations:
- Rerere entries really shouldn't impact garbage collection at all, as
these entries are not stored in the object database.
- The index and HEAD reference of pruned worktrees may reference
objects that become unreachable.
That being said, the impact should be overall rather negligible. If the
user was asking us to prune objects with immediate expiration time then
we might now prune objects that were previously still kept alive by the
worktree. But besides being a very specific edge case, it's arguably not
even the wrong thing to also prune any potentially-unreachable objects
immediately.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 77d0a5c948..8f568003ee 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1011,6 +1011,13 @@ int cmd_gc(int argc,
if (opts.detach <= 0 && !skip_foreground_tasks)
gc_foreground_tasks(&opts, &cfg);
+ if (cfg.prune_worktrees_expire &&
+ maintenance_task_worktree_prune(&opts, &cfg))
+ die(FAILED_RUN, "worktree");
+
+ if (maintenance_task_rerere_gc(&opts, &cfg))
+ die(FAILED_RUN, "rerere");
+
if (!the_repository->repository_format_precious_objects) {
struct child_process repack_cmd = CHILD_PROCESS_INIT;
@@ -1038,13 +1045,6 @@ int cmd_gc(int argc,
}
}
- if (cfg.prune_worktrees_expire &&
- maintenance_task_worktree_prune(&opts, &cfg))
- die(FAILED_RUN, "worktree");
-
- if (maintenance_task_rerere_gc(&opts, &cfg))
- die(FAILED_RUN, "rerere");
-
report_garbage = report_pack_garbage;
odb_reprepare(the_repository->objects);
if (pack_garbage.nr > 0) {
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 01/11] odb: run "pre-auto-gc" hook for all maintenance tasks
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
In-Reply-To: <20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im>
The "pre-auto-gc" hook is supposed to run before auto-maintenance
starts. The intent of this is to give users the ability to intercept
running maintenance in case there's for example an event that is not
supposed to run in parallel with repository maintenance.
This hook runs via `need_to_gc()`, which is invoked via two paths:
- It is called directly by git-gc(1).
- It is called indirectly by git-maintenance(1) via the "gc" task.
While the former makes sense, the latter is somewhat off. While the hook
is indeed strongly tied to gc'ing a repository, the original intent of
the hook is rather to inhibit any kind of automated garbage collection.
That noticeably also includes all the other maintenance tasks that our
new infrastructure may run, but those aren't getting intercepted at all.
The move towards our new maintenance strategy has thus somewhat neutered
the effectiveness of the hook.
Fix this issue by running the hook before the first auto-maintenance
task that would run as determined by the tasks's auto condition. Note
that this requires us to lift the call to `run_hooks()` out of
`needs_to_gc()`, as the hook would otherwise potentially run multiple
times.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/gc.c | 35 ++++++++++----
t/t7900-maintenance.sh | 121 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 147 insertions(+), 9 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index d32af422af..77d0a5c948 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -709,8 +709,6 @@ static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args)
else
return 0;
- if (run_hooks(the_repository, "pre-auto-gc"))
- return 0;
return 1;
}
@@ -933,7 +931,8 @@ int cmd_gc(int argc,
/*
* Auto-gc should be least intrusive as possible.
*/
- if (!need_to_gc(&cfg, &repack_args)) {
+ if (!need_to_gc(&cfg, &repack_args) ||
+ run_hooks(the_repository, "pre-auto-gc")) {
ret = 0;
goto out;
}
@@ -1755,11 +1754,18 @@ enum task_phase {
TASK_PHASE_BACKGROUND,
};
+enum auto_gc_hook_result {
+ AUTO_GC_HOOK_UNDECIDED = 0,
+ AUTO_GC_HOOK_RUN = 1,
+ AUTO_GC_HOOK_SKIP = 2,
+};
+
static int maybe_run_task(const struct maintenance_task *task,
struct repository *repo,
struct maintenance_run_opts *opts,
struct gc_config *cfg,
- enum task_phase phase)
+ enum task_phase phase,
+ enum auto_gc_hook_result *auto_gc_hook_result)
{
int foreground = (phase == TASK_PHASE_FOREGROUND);
maintenance_task_fn fn = foreground ? task->foreground : task->background;
@@ -1768,9 +1774,19 @@ static int maybe_run_task(const struct maintenance_task *task,
if (!fn)
return 0;
- if (opts->auto_flag &&
- (!task->auto_condition || !task->auto_condition(cfg)))
- return 0;
+ if (opts->auto_flag) {
+ if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP)
+ return 0;
+
+ if (!task->auto_condition || !task->auto_condition(cfg))
+ return 0;
+
+ if (*auto_gc_hook_result == AUTO_GC_HOOK_UNDECIDED)
+ *auto_gc_hook_result = run_hooks(repo, "pre-auto-gc") ?
+ AUTO_GC_HOOK_SKIP : AUTO_GC_HOOK_RUN;
+ if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP)
+ return 0;
+ }
trace2_region_enter(region, task->name, repo);
if (fn(opts, cfg)) {
@@ -1789,6 +1805,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
struct lock_file lk;
struct repository *r = the_repository;
char *lock_path = xstrfmt("%s/maintenance", r->objects->sources->path);
+ enum auto_gc_hook_result auto_gc_hook_result = AUTO_GC_HOOK_UNDECIDED;
if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) {
/*
@@ -1808,7 +1825,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
for (size_t i = 0; i < opts->tasks_nr; i++)
if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg,
- TASK_PHASE_FOREGROUND))
+ TASK_PHASE_FOREGROUND, &auto_gc_hook_result))
result = 1;
/* Failure to daemonize is ok, we'll continue in foreground. */
@@ -1820,7 +1837,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
for (size_t i = 0; i < opts->tasks_nr; i++)
if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg,
- TASK_PHASE_BACKGROUND))
+ TASK_PHASE_BACKGROUND, &auto_gc_hook_result))
result = 1;
rollback_lock_file(&lk);
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index d7f82e1bec..1212b306b6 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -740,6 +740,127 @@ test_expect_success 'geometric repacking honors configured split factor' '
)
'
+test_expect_success 'pre-auto-gc hook runs exactly once' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ write_script .git/hooks/pre-auto-gc <<-\EOF &&
+ echo hook >>hook.log
+ EOF
+
+ # Satisfy the auto condition for multiple tasks, both in the
+ # foreground and in the background phase.
+ git config set maintenance.reflog-expire.auto -1 &&
+ git config set maintenance.geometric-repack.auto -1 &&
+ git config set maintenance.rerere-gc.auto -1 &&
+
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+ git maintenance run --auto 2>/dev/null &&
+
+ # The successful hook does not inhibit any of the tasks...
+ test_subcommand git reflog expire --all <trace2.txt &&
+ test_subcommand_flex git repack <trace2.txt &&
+ test_subcommand git rerere gc <trace2.txt &&
+ # ... but it must only have been executed a single time.
+ test_line_count = 1 hook.log
+ )
+'
+
+test_expect_success 'pre-auto-gc hook can inhibit geometric strategy' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ write_script .git/hooks/pre-auto-gc <<-\EOF &&
+ echo hook >>hook.log
+ exit 1
+ EOF
+
+ git config set maintenance.reflog-expire.auto -1 &&
+ git config set maintenance.geometric-repack.auto -1 &&
+ git config set maintenance.rerere-gc.auto -1 &&
+
+ # Maintenance would be required...
+ git maintenance is-needed --auto &&
+
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+ git maintenance run --auto 2>/dev/null &&
+
+ # ... but the failing hook inhibits all tasks. The hook itself
+ # is expected to be the only child process being spawned, and
+ # it must only run a single time.
+ test_grep "child_start.*pre-auto-gc" trace2.txt &&
+ test_subcommand_flex ! git trace2 &&
+ test_line_count = 1 hook.log
+ )
+'
+
+test_expect_success 'pre-auto-gc hook can inhibit gc strategy' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ write_script .git/hooks/pre-auto-gc <<-\EOF &&
+ echo hook >>hook.log
+ exit 1
+ EOF
+
+ git config set maintenance.strategy gc &&
+ git config set maintenance.auto false &&
+ git config set gc.auto 3 &&
+
+ test_oid_init &&
+
+ # We need to create two objects whose hashes start with 17
+ # since this is what the gc task counts.
+ test_commit "$(test_oid blob17_1)" &&
+ test_commit "$(test_oid blob17_2)" &&
+
+ # Maintenance would be required...
+ git maintenance is-needed --auto &&
+
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+ git maintenance run --auto 2>/dev/null &&
+
+ # ... but the failing hook inhibits all tasks. The hook itself
+ # is expected to be the only child process being spawned, and
+ # it must only run a single time.
+ test_grep "child_start.*pre-auto-gc" trace2.txt &&
+ test_subcommand_flex ! git trace2 &&
+ test_line_count = 1 hook.log
+ )
+'
+
+test_expect_success 'pre-auto-gc hook does not run when no maintenance is needed' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ write_script .git/hooks/pre-auto-gc <<-\EOF &&
+ echo hook >>hook.log
+ EOF
+ test_must_fail git maintenance is-needed --auto &&
+ git maintenance run --auto 2>/dev/null &&
+ test_path_is_missing hook.log
+ )
+'
+
+test_expect_success 'pre-auto-gc hook does not run without --auto' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_hook -C repo pre-auto-gc <<-\EOF &&
+ echo hook >>hook.log
+ EOF
+ (
+ cd repo &&
+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+ git maintenance run 2>/dev/null &&
+ test_grep "\[\"git\",\"repack\"," trace2.txt &&
+ test_path_is_missing hook.log
+ )
+'
+
test_expect_success 'pack-refs task' '
for n in $(test_seq 1 5)
do
--
2.55.0.141.g00534a21ce.dirty
^ permalink raw reply related
* [PATCH 00/11] odb: make optimizations pluggable
From: Patrick Steinhardt @ 2026-07-07 15:32 UTC (permalink / raw)
To: git
Hi,
this patch series converts object housekeeping to become pluggable.
There isn't really anything else to say about this.
The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
2026-07-06).
Thanks!
Patrick
---
Patrick Steinhardt (11):
odb: run "pre-auto-gc" hook for all maintenance tasks
builtin/gc: move worktree and rerere tasks before object optimizations
builtin/gc: extract object database optimizations into separate function
builtin/gc: make repack arguments self-contained
builtin/gc: inline config values specific to the "files" backend
builtin/gc: introduce object database optimization options
builtin/gc: move geometric repacking into `odb_optimize()`
builtin/gc: introduce `odb_optimize_required()`
builtin/gc: refactor ODB optimizations to operate on "files" source
builtin/gc: fix signedness issues in ODB-related functionality
odb: make optimizations pluggable
builtin/gc.c | 534 ++++++++-----------------------------------------
odb.c | 12 ++
odb.h | 45 +++++
odb/source-files.c | 470 +++++++++++++++++++++++++++++++++++++++++++
odb/source-files.h | 15 ++
odb/source.h | 36 ++++
t/t7900-maintenance.sh | 143 ++++++++++++-
7 files changed, 789 insertions(+), 466 deletions(-)
---
base-commit: f85a7e662054a7b0d9070e432508831afa214b47
change-id: 20260612-b4-pks-odb-optimize-3426c57e5c30
^ permalink raw reply
* Re: [PATCH v6 2/2] config: add "worktree" and "worktree/i" includeIf conditions
From: Patrick Steinhardt @ 2026-07-07 15:26 UTC (permalink / raw)
To: Chen Linxuan; +Cc: git, Kristoffer Haugsbakk, Junio C Hamano, Phillip Wood
In-Reply-To: <CAC1kPDNBecLbmZwjfR5-CsNheF3rcbZ5=SQ+cwjzpFMjFr9KGQ@mail.gmail.com>
On Mon, Jul 06, 2026 at 08:18:39PM +0800, Chen Linxuan wrote:
> On Fri, Jul 3, 2026 at 7:03 PM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Fri, Jul 03, 2026 at 11:13:18AM +0800, Chen Linxuan via B4 Relay wrote:
> > > diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> > > index f3892578e4ff..4e840dfdb35b 100755
> > > --- a/t/t1305-config-include.sh
> > > +++ b/t/t1305-config-include.sh
> > > @@ -396,4 +396,132 @@ test_expect_success 'onbranch without repository but explicit nonexistent Git di
> > [snip]
> > > +test_expect_success SYMLINKS 'conditional include, worktree resolves symlinks' '
> > > + mkdir real-wt &&
> > > + ln -s real-wt link-wt &&
> > > + git init link-wt/repo &&
> > > + (
> > > + cd link-wt/repo &&
> > > + # repo->worktree resolves symlinks, so use real path in pattern
> > > + echo "[includeIf \"worktree:**/real-wt/repo\"]path=bar-link" >>.git/config &&
> > > + echo "[test]wtlink=2" >.git/bar-link &&
> > > + echo 2 >expect &&
> > > + git config test.wtlink >actual &&
> > > + test_cmp expect actual
> > > + )
> > > +'
> >
> > Okay, this covers one scenario. But with "gitdir:" we're actually able
> > to use both the symlinked and the real location:
> >
> > test_expect_success SYMLINKS 'conditional include, worktree matching symlink' '
> > mkdir sym-real &&
> > ln -s sym-real sym-link &&
> > git init sym-link/repo &&
> > (
> > cd sym-link/repo &&
> > link_path="$(pwd)" &&
> > real_path="$(test-tool path-utils real_path "$link_path")" &&
> > cat >>.git/config <<-EOF &&
> > [includeIf "gitdir:$link_path/.git"]
> > path = gitdir-link
> > [includeIf "gitdir:$real_path/.git"]
> > path = gitdir-real
> > [includeIf "worktree:$link_path"]
> > path = worktree-link
> > [includeIf "worktree:$real_path"]
> > path = worktree-real
> > EOF
> > echo "[test]gitdirlink=1" >.git/gitdir-link &&
> > echo "[test]gitdirreal=1" >.git/gitdir-real &&
> > echo "[test]worktreelink=1" >.git/worktree-link &&
> > echo "[test]worktreereal=1" >.git/worktree-real &&
> >
> > git config get test.gitdirlink &&
> > git config get test.gitdirreal &&
> > git config get test.worktreereal &&
> > test_must_fail git config test.worktreelink
> > )
> > '
> >
> > The last call to git-config(1) fails, which is inconsistent with how
> > resolve the path for "gitdir".
> >
>
> I investigated the symlink mismatch.
>
> `gitdir:` works because `opts->git_dir` still preserves the discovered or
> user-provided spelling, and `include_by_path()` matches both its realpath
> and its absolute non-realpath form.
>
> `worktree:` is different: `repo_get_work_tree()` returns
> `repo->worktree`, which is stored by `repo_set_worktree()` via
> `real_pathdup(path, 1)`. So the symlink spelling is already lost before
> we evaluate includeIf conditions.
>
> Changing `repo->worktree` itself to preserve the original spelling looks
> risky, because several users access `repo->worktree` directly, and setup
> code appears to rely on it being canonical.
>
> My current possible v7 approach is to keep `repo->worktree` canonical,
> but store an additional absolute, normalized, non-realpath worktree path
> for `includeIf.worktree`. For the ordinary discovered-repository case,
> this has to be derived in `setup_discovered_git_dir()` from physical
> `cwd`, the worktree-root offset, and a validated `$PWD`, because
> `set_git_work_tree()` is otherwise only called with `"."`.
>
> This makes your suggested test pass, but the plumbing is less trivial
> than the original patch. Does this approach sound reasonable, or would
> you prefer different semantics for symlinked worktree paths?
It certainly sounds a bit ugly, but I'd rather have something that's
ugly than something that's inconsistent for our users *shrug*
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH 2/2] reftable: fix quadratic behavior when re-creating deleted refs
From: Patrick Steinhardt @ 2026-07-07 15:24 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
In-Reply-To: <1459371d3ab2f237152e20040987b4cb6a5eca77.1783344957.git.gitgitgadget@gmail.com>
On Mon, Jul 06, 2026 at 01:35:56PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
>
> When many refs are deleted and then re-created, update-ref exhibits
> quadratic behavior. With 8000 refs deleted and re-created, the
> runtime is ~15s, quadrupling for each doubling of input size.
>
> The root cause is the merged iterator's suppress_deletions flag.
> When set, merged_iter_next_void() silently consumes tombstone records
> in a tight internal loop before returning to the caller. This
> prevents higher-level code from checking iteration bounds (such as
> prefix or refname comparisons) until after all tombstones have been
> scanned.
>
> This affects two code paths during ref creation:
>
> - refs_verify_refnames_available() seeks to "refs/tags/foo-1/" to
> check for D/F conflicts and must scan through all subsequent
> tombstones before the caller can see that they are past the prefix
> of interest.
>
> - reftable_backend_read_ref() seeks to a specific refname and must
> scan through all subsequent tombstones before returning "not
> found", because the merged iterator skips the matching tombstone
> and searches for the next live record.
It probably not only impacts reference creation, but also every reader
that wants to search for a specific reference that doesn't exist.
> Fix this by removing suppress_deletions from the merged iterator and
> instead handling deletion records at each call site in the reftable
> backend, where prefix and refname bounds are available. Tombstones
> are now returned to callers, which skip them after their existing
> bounds checks. This allows iteration to terminate as soon as a
> tombstone past the relevant bound is encountered.
This option is still used by downstream users of the reftable library,
like libgit2. So we shouldn't just delete it outright.
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 4ae22922de..8c4f119ff1 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -633,6 +633,9 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
> break;
> }
>
> + if (iter->ref.value_type == REFTABLE_REF_DELETION)
> + continue;
> +
> if (iter->exclude_patterns && should_exclude_current_ref(iter))
> continue;
>
Okay. I was first wondering whether we should move this call earlier.
But we actually don't want to, as this is the code that precedes the
above:
if (iter->prefix_len &&
strncmp(iter->prefix, iter->ref.refname, iter->prefix_len)) {
iter->err = 1;
break;
}
So this allows us to not only skip the current iteration, but completely
abort iteration by observing tombstones that sort after our prefix.
In any case, as far as I can see all sites where we iterate through
either ref or log records have been adapted to handle deletions.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] t: add tests for ref tombstone scenarios
From: Patrick Steinhardt @ 2026-07-07 15:24 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
In-Reply-To: <d8ffdcb4f8c1988c109761ddb9daff8c07caa2b1.1783344957.git.gitgitgadget@gmail.com>
On Mon, Jul 06, 2026 at 01:35:55PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> diff --git a/t/perf/p1401-ref-store-tombstones.sh b/t/perf/p1401-ref-store-tombstones.sh
> new file mode 100755
> index 0000000000..e40a6dcbf4
> --- /dev/null
> +++ b/t/perf/p1401-ref-store-tombstones.sh
> @@ -0,0 +1,44 @@
> +#!/bin/sh
> +
> +test_description="Tests performance of ref operations with many tombstones"
> +
> +. ./perf-lib.sh
> +
> +test_expect_success "setup" '
> + git init --ref-format=reftable repo &&
> + blob=$(echo foo | git -C repo hash-object -w --stdin) &&
> + for i in $(test_seq 8000)
> + do
> + printf "create refs/tags/tag-%d %s\n" "$i" "$blob" ||
> + return 1
> + done >repo/input &&
> + git -C repo update-ref --stdin <repo/input &&
> + git -C repo for-each-ref --format="delete %(refname)" |
> + git -C repo update-ref --stdin
> +'
> +
> +test_perf "recreate refs after mass delete" '
> + git -C repo update-ref --stdin <repo/input &&
> + git -C repo for-each-ref --format="delete %(refname)" |
> + git -C repo update-ref --stdin
> +'
You're not only benchmarking the reference recreation, but also their
deletion. If I'm not misreading things, then you can queue cleanups via
`test_when_finished`, and these calls will not be measured.
> +test_expect_success "setup asymmetric" '
> + for i in $(test_seq 8000)
> + do
> + printf "create refs/tags/old-%d %s\n" "$i" "$blob" ||
> + return 1
> + done >repo/input-old &&
> + sed "s/old-/new-/" <repo/input-old >repo/input-new &&
> + git -C repo update-ref --stdin <repo/input-old &&
> + git -C repo for-each-ref --format="delete %(refname)" |
> + git -C repo update-ref --stdin
> +'
Would it make sense to use separate repositories? Otherwise, state from
the preceding benchmark(s) will impact subsequent ones.
> diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
> index e19e036898..4b7cfe38e4 100755
> --- a/t/t0610-reftable-basics.sh
> +++ b/t/t0610-reftable-basics.sh
> @@ -1163,4 +1163,26 @@ test_expect_success 'writes do not persist peeled value for invalid tags' '
> )
> '
>
> +test_expect_success 'delete and re-create refs with tombstones' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + test_commit -C repo A &&
> + A=$(git -C repo rev-parse HEAD) &&
> + cat >input <<-EOF &&
> + create refs/tags/a $A
> + create refs/tags/b $A
> + create refs/tags/c $A
> + EOF
> + git -C repo update-ref --stdin <input &&
> +
> + # delete all tags, leaving tombstones
> + git -C repo for-each-ref --format="delete %(refname)" refs/tags/ |
> + git -C repo update-ref --stdin &&
> +
> + # re-create the same refs and verify they are visible
> + git -C repo update-ref --stdin <input &&
> + git -C repo tag -l >actual &&
> + test_line_count = 3 actual
> +'
I wonder whether this test really adds any value. We probably have lots
of tests already that test creation/deletion of references.
Patrick
^ permalink raw reply
* Re: [PATCH v6 3/3] replay: offer an option to linearize the commit topology
From: Toon Claes @ 2026-07-07 15:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Elijah Newren, Johannes Schindelin
In-Reply-To: <xmqqbjcnhjvk.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Toon Claes <toon@iotcl.com> writes:
>
>> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>> ...
>> Linearizing is a distinct operation, and flattening merge commits is
>> just one aspect of that. Recreating merges would be a separate mode, so
>> rather than mirror git-rebase(1)'s `--rebase-merges[=<mode>]` interface,
>> git-replay(1) uses its own `--linearize` option.
>>
>> Co-authored-by: Toon Claes <toon@iotcl.com>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> Signed-off-by: Toon Claes <toon@iotcl.com>
>> ---
>> Documentation/git-replay.adoc | 21 ++++++-
>> builtin/replay.c | 6 +-
>> replay.c | 54 ++++++++++------
>> replay.h | 5 ++
>> t/t3650-replay-basics.sh | 140 +++++++++++++++++++++++++++++++++++++++++-
>> 5 files changed, 203 insertions(+), 23 deletions(-)
>
> With such an extensive change in behaviour, I wonder if Dscho is
> still responsible for latent bugs in this round of implementation
> and documentation, or should you take the responsibility over?
I don't mind. But I don't want to steal credit.
Initially, when Dscho shared the patch with me, he already informed me
he doesn't really care about authorship. So in the next version I'll be
taking over authorship and add a Based-on-patches-by trailer.
@Dscho, let me know if you disagree?
>> +--linearize::
>> + In this mode, each replayed commit is stacked on top of the
>> + previously replayed one, so all replayed commits are flattened into
>> + a single linear history.
>> ++
>> +When a merge commit is encountered, the behavior of git-rebase(1)'s
>> +option `--no-rebase-merges` is imitated. All commits in the range
>> +reachable from the merge commit are replayed into a linear history, and
>> +the merge commit itself is dropped. A ref that pointed to a merge commit
>> +is updated to the merge's last replayed ancestor.
>> ++
>> +This flattens the `<revision-range>` as a whole. When multiple revision
>> +ranges are given they are stacked on top of each other into one linear
>> +history. Each of their refs is updated to point to its position in that
>> +history. To linearize ranges separately, replay them in separate `git
>> +replay` invocations.
>
> OK, very much understandable.
>
>> +This option is incompatible with `--revert`.
>
> Definitely it is OK to leave it outside the scope, but I am not sure
> if reverting a group of commits that happens to be "closed" and
> happens to contain merges, is inherently incompatible with
> flattening. If you have
>
> ----O--A
> \ \
> B--M--C
>
> and you want to revert what happened while the history advanced from
> O to M, I would naïvely expect that I can arrive at
>
> ----O--A
> \ \
> B--M--C-B'-A'
>
> by linearly applying the inverse of A and B (in either order).
You're absolutely right. Personally I'm not sure why the limitation was
introduced. I've done some testing and I cannot see why we wouldn't
allow --revert and --linearize to be combined. So I'll be submitting v7
without this restriction.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers
From: Kristofer Karlsson @ 2026-07-07 14:57 UTC (permalink / raw)
To: Taylor Blau; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <ak0D44nhSH/98WYD@nand.local>
On Tue, 7 Jul 2026 at 15:49, Taylor Blau <me@ttaylorr.com> wrote:
>
> I think that there is a more permanent fix, though, which would have not
> allowed this bug to evade both its author, and reviewer (me). I *think*
> that we may clear up some scoping issues if we removed g->topo_levels
> entirely, and instead stored it in the write_commit_graph_ctx struct.
>
> I haven't thought through the implications of doing so completely, so
> it's entirely possible that this idea is bunk for some other reason. But
> it was the first thing that came to mind, and so feels worth exploring
> to see if it might have prevented something like this from ever
> happening in the first place.
>
I looked into the structural change you suggested and I think
it's doable, though not quite as simple as just moving
it into ctx (since fill_commit_graph_info() doesn't have ctx).
I found three approaches:
(a) Thread topo_levels through the call chain. This would
affect:
- fill_commit_graph_info()
- fill_commit_in_graph()
- parse_commit_in_graph_one()
- parse_commit_in_graph()
- load_commit_graph_info()
- lookup_commit_in_graph().
This is the most direct approach, but it touches many functions
and some callers would need to pass in NULL which makes it a bit
noisy.
(b) Move topo_levels to struct object_database. Since
fill_commit_graph_info() can already reach the odb via
g->odb_source->odb, no signature changes are needed.
The write side becomes a single assignment:
ctx.r->objects->topo_levels = &topo_levels;
and cleanup becomes:
ctx.r->objects->topo_levels = NULL;
No chain walk needed and the diff is fairly small.
I am not sure about the semantics of it though -- should the odb
have a reference to topo_levels?
(c) Introduce a struct for the chain as a whole, separating it from
the per-layer struct commit_graph. Right now struct commit_graph
represents a single layer but also serves as the chain head, so
chain-wide state like topo_levels gets duplicated on every layer
(only logically -- the actual overhead is still small).
A dedicated chain struct could own topo_levels and the linked list
of layers. IMO this is the cleanest model but a larger refactoring.
I have a prototype of (b) that compiles and passes the test suite.
For now though, I think the minimal bugfix is the right thing to do.
Thanks,
Kristofer
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox