From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 857C3C433E0 for ; Tue, 19 Jan 2021 23:34:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5525122DD3 for ; Tue, 19 Jan 2021 23:34:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727956AbhASXeb (ORCPT ); Tue, 19 Jan 2021 18:34:31 -0500 Received: from cloud.peff.net ([104.130.231.41]:60568 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727918AbhASXe1 (ORCPT ); Tue, 19 Jan 2021 18:34:27 -0500 Received: (qmail 15894 invoked by uid 109); 19 Jan 2021 23:33:35 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 19 Jan 2021 23:33:35 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 14328 invoked by uid 111); 19 Jan 2021 23:33:35 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 19 Jan 2021 18:33:35 -0500 Authentication-Results: peff.net; auth=none Date: Tue, 19 Jan 2021 18:33:34 -0500 From: Jeff King To: Taylor Blau Cc: Jacob Vosmaer , git@vger.kernel.org Subject: Re: [PATCH 1/1] builtin/pack-objects.c: avoid iterating all refs Message-ID: References: <20210119143348.27535-1-jacob@gitlab.com> <20210119143348.27535-2-jacob@gitlab.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Tue, Jan 19, 2021 at 06:08:31PM -0500, Taylor Blau wrote: > > 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