threads / patch / 47949

patchcommit: run git gc --auto just before the pre-commit hook

Subject: [PATCH] commit: run git gc --auto just before the pre-commit hook

## tl;dr

4 messages between Feb 28, 2018 and Feb 28, 2018. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Ævar Arnfjörð Bjarmason· Feb 28, 2018, 22:13 UTC · lore

Change the behavior of git-commit back to what it was back in d4bb43ee27 ("Invoke "git gc --auto" from commit, merge, am and rebase.", 2007-09-05) when it was git-commit.sh.

Shortly afterwards in f5bbc3225c ("Port git commit to C.", 2007-11-08) when it was ported to C the "git gc --auto" invocation went away.

Before this git gc --auto only ran for git-{am,merge,fetch,receive-pack}, therefore it was possible to write a script that would "git commit" a lot of data locally, and gc would never run.

One such repository that was locally committing generated zone file changes had grown to a size of ~60GB before a daily cronjob was added to "git gc", bringing it down to less than 1GB. This will make such cases work transparently.

I think fixing such pathological cases where the repository will grow forever is a worthwhile trade-off for spending a couple of milliseconds calling "git gc --auto" (in the common cases where it doesn't do anything).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit.c | 2 ++
 1 file changed, 2 insertions(+)

Show changes to builtin/commit.c +2 −0

diff --git a/builtin/commit.c b/builtin/commit.c
index e8e8d13be4..b671367840 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1402,6 +1402,7 @@ int run_commit_hook(int editor_is_used, const char *index_file, const char *name
 
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
+	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	static struct wt_status s;
 	static struct option builtin_commit_options[] = {
 		OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
@@ -1608,6 +1609,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	rerere(0);
 	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
+	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
 	if (amend && !no_post_rewrite) {
 		commit_post_rewrite(current_head, &oid);
 	}

-- 
2.15.1.424.g9478a66081

Junio C Hamano· Feb 28, 2018, 22:29 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] commit: run git gc --auto just before the pre-commit hook

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Change the behavior of git-commit back to what it was back in
> d4bb43ee27 ("Invoke "git gc --auto" from commit, merge, am and
> rebase.", 2007-09-05) when it was git-commit.sh.

... which was to run it just before post-commit. Do I retitle this patch before queuing?

The code seems to run it "after" post-commit. We need to explain why we chose to run it differently from the old practice, when we claim we resurrect the old behaviour with this change.

> Shortly afterwards in f5bbc3225c ("Port git commit to C.", 2007-11-08)
> when it was ported to C the "git gc --auto" invocation went away.

So this is a decade late regression fix? As they say, better late than never, probably.

Show 8 quoted lines

> @@ -1608,6 +1609,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  
>  	rerere(0);
>  	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
> +	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
>  	if (amend && !no_post_rewrite) {
>  		commit_post_rewrite(current_head, &oid);
>  	}

Ævar Arnfjörð Bjarmason· Feb 28, 2018, 23:04 UTC · re: Junio C Hamano · lore

[PATCH v2] commit: run git gc --auto just before the pre-commit hook

Change the behavior of git-commit back to what it was back in d4bb43ee27 ("Invoke "git gc --auto" from commit, merge, am and rebase.", 2007-09-05) when it was git-commit.sh.

Shortly afterwards in f5bbc3225c ("Port git commit to C.", 2007-11-08) when it was ported to C, the "git gc --auto" invocation went away.

Before this, git gc --auto only ran for git-{am,merge,fetch,receive-pack}. Therefore it was possible to write a script that would "git commit" a lot of data locally, and gc would never run.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Wed, Feb 28 2018, Junio C. Hamano jotted:

Show 8 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Change the behavior of git-commit back to what it was back in
>> d4bb43ee27 ("Invoke "git gc --auto" from commit, merge, am and
>> rebase.", 2007-09-05) when it was git-commit.sh.
>
> ... which was to run it just before post-commit.  Do I retitle this
> patch before queuing?

Do'h. Of course I screw up something simple like that, sorry. This v2 fixes it, and I also rephrased the commit message a bit (more commas & full-stops).

> So this is a decade late regression fix?  As they say, better late
> than never, probably.

Yup.

I wonder if it would also be a good idea to run git gc --auto on "git push". It itself won't create any objects, but it would be a nice proxy in many cases for picking up anything else we missed due to various object writing commands that won't run --auto.

 builtin/commit.c | 2 ++
 1 file changed, 2 insertions(+)

Show changes to builtin/commit.c +2 −0

diff --git a/builtin/commit.c b/builtin/commit.c
index e8e8d13be4..a16a056f6a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1402,6 +1402,7 @@ int run_commit_hook(int editor_is_used, const char *index_file, const char *name
 
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
+	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	static struct wt_status s;
 	static struct option builtin_commit_options[] = {
 		OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
@@ -1607,6 +1608,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		     "not exceeded, and then \"git reset HEAD\" to recover."));
 
 	rerere(0);
+	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
 	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
 		commit_post_rewrite(current_head, &oid);

-- 
2.15.1.424.g9478a66081

Junio C Hamano· Feb 28, 2018, 23:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2] commit: run git gc --auto just before the pre-commit hook

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 12 quoted lines

>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>
>>> Change the behavior of git-commit back to what it was back in
>>> d4bb43ee27 ("Invoke "git gc --auto" from commit, merge, am and
>>> rebase.", 2007-09-05) when it was git-commit.sh.
>>
>> ... which was to run it just before post-commit.  Do I retitle this
>> patch before queuing?
>
> Do'h. Of course I screw up something simple like that, sorry. This v2
> fixes it, and I also rephrased the commit message a bit (more commas &
> full-stops).

I guess I still need to retitle it ;-) But that can happen tomorrow (I have the previous one with local fixes that pretty much matches v2 modulo the body of the log message on 'pu', ready to be pushed out).

> I wonder if it would also be a good idea to run git gc --auto on "git
> push". It itself won't create any objects, but it would be a nice
> proxy in many cases for picking up anything else we missed due to
> various object writing commands that won't run --auto.

Before "push" starts producing a pack might be a good optimization, as reading from a packed repository is often more performant than a repository full of loose objects.