git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/1] gc/repack: release packs when needed

From
Jeff King <peff@peff.net>
Date
Jan 11, 2019, 16:10 UTC
Message-ID
<20190111161021.GE16754@sigill.intra.peff.net>
In-Reply-To
<xmqqmuo81b2n.fsf@gitster-ct.c.googlers.com>
On Thu, Jan 10, 2019 at 01:01:36PM -0800, Junio C Hamano wrote:
Show 24 quoted lines
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index 871a56f1c5..df90fd7f51 100644
> > --- a/builtin/gc.c
> > +++ b/builtin/gc.c
> > @@ -659,8 +659,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
> >  
> >  	report_garbage = report_pack_garbage;
> >  	reprepare_packed_git(the_repository);
> > -	if (pack_garbage.nr > 0)
> > +	if (pack_garbage.nr > 0) {
> > +		close_all_packs(the_repository->objects);
> >  		clean_pack_garbage();
> > +	}
> 
> Closing before removing does make sense, but wouldn't we want to
> move reprepare_packed_git() after clean_pack_garbage() while at it?
> After all, the logical sequence is that we used the current set of
> packs to figure out whihch ones are garbage, then now we are about
> to discard.  We close the packs in the current set (i.e. the fix
> made in this patch), discard the garbage packs.  It would make sense
> to start using the new set (i.e. "reprepare") after all that is
> done, no?  Especially, given that the next step (write-commit-graph)
> still wants to read quite a lot of data from now the latest set of
> packfiles...

I agree that your suggested ordering makes more sense, but I don't think it matters in practice with the current code. reprepare_packed_git() never throws away old pack entries (and if they're mmap'd, we might even continue to use them). So the end result is the same either way.

-Peff
Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 5 in “gc/repack: release packs when needed”
  1. 0/1 gc/repack: release packs when neededJohannes Schindelin via GitGitGadget, Dec 15, 2018
  2. 1/1 gc/repack: release packs when neededJohannes Schindelin via GitGitGadget, Dec 15, 2018
  3. Junio C HamanoJan 10, 2019
  4. Jeff KingJan 11, 2019
  5. Junio C HamanoJan 11, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.