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

Re: [PATCH 1/1] builtin/pack-objects.c: avoid iterating all refs

From
Jeff King <peff@peff.net>
Date
Jan 19, 2021, 23:33 UTC
Message-ID
<YAdsTjDXImYlfTn8@coredump.intra.peff.net>
In-Reply-To
<YAdmb3EfNVHYavNk@nand.local>
On Tue, Jan 19, 2021 at 06:08:31PM -0500, Taylor Blau wrote:
Show 24 quoted lines
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index 2a00358f34..2b32bc93bd 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
> > @@ -3740,7 +3740,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> >  	}
> >  	cleanup_preferred_base();
> >  	if (include_tag && nr_result)
> > -		for_each_ref(add_ref_tag, NULL);
> > +		for_each_fullref_in("refs/tags/", add_ref_tag, NULL, 0);
> 
> Makes sense. I was curious why it wasn't this way from the beginning in
> f0a24aa56e (git-pack-objects: Automatically pack annotated tags if
> object was packed, 2008-03-03).
> 
> The patch doesn't say, but presumably it was because there wasn't any
> speed-up to be had iterating only a subset of references back then if
> they were packed (did packed refs even exist in 2008? unsure). In any
> case, builtin/pack-objects.c:add_ref_tag() discards anything that
> doesn't start with "refs/tags/", so I think your change is doing the
> right thing here.
> 
> That said, you could shorten this to use 'for_each_tag_ref()' instead
> (which compiles to the exact same thing).

You'd end up with "v1.2.3" in the refname field of the callback then, rather than "refs/tags/v1.2.3". So we'd definitely need to drop the prefix check in add_ref_tag() then (though I think it is a good idea anyway). But I'm also not sure if it would interfere with peel_ref(), and its magic for using packed-refs peeled information.

-Peff
Previous: Taylor BlauNext: Taylor Blau
Message 4 of 13 in “builtin/pack-objects.c: avoid iterating all refs”
  1. 0/1 builtin/pack-objects.c: avoid iterating all refsJacob Vosmaer, Jan 19, 2021
  2. 1/1 builtin/pack-objects.c: avoid iterating all refsJacob Vosmaer, Jan 19, 2021
  3. Taylor BlauJan 19, 2021
  4. Jeff KingJan 19, 2021
  5. Taylor BlauJan 19, 2021
  6. Jeff KingJan 19, 2021
  7. Ævar Arnfjörð BjarmasonJan 20, 2021
  8. Taylor BlauJan 20, 2021
  9. Ævar Arnfjörð BjarmasonJan 20, 2021
  10. Jeff KingJan 20, 2021
  11. Ævar Arnfjörð BjarmasonJan 21, 2021
  12. Jeff KingJan 21, 2021
  13. Jeff KingJan 19, 2021

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.