From: Jeff King <peff@peff.net>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
gitgitgadget@gmail.com, Git Mailing List <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 2/2] repack -ad: prune the list of shallow commits
Date: Wed, 18 Jul 2018 13:45:05 -0400 [thread overview]
Message-ID: <20180718174505.GA3084@sigill.intra.peff.net> (raw)
In-Reply-To: <CACsJy8DW8=FoRyEwAy48S76q0gxQbrS3emHou7QDhHqzwJRu+g@mail.gmail.com>
On Wed, Jul 18, 2018 at 07:31:40PM +0200, Duy Nguyen wrote:
> > Sort of an aside to the patch under discussion, but I think it may make
> > sense for prune_shallow() to take a callback function for determining
> > whether a commit is reachable.
> >
> > I have an old patch that teaches git-prune to lazily do the reachability
> > check, since in many cases "git repack" will have just packed all of the
> > loose objects. But it just occurred to me that this patch is totally
> > broken with respect to prune_shallow(), because it would not set the
> > SEEN flag (I've literally been running with it for years, which goes to
> > show how often I use the shallow feature).
> >
> > And if we were to have repack do a prune_shallow(), it may want to use a
> > different method than traversing and marking each object SEEN.
>
> All of this sounds good. The only thing I'd like to do a bit
> differently is to pass the reachable commits in prune_shallow() as a
> commit-slab instead of taking a callback function. I'll refactor this
> code, move prune_shallow() to a separate command prune-shallow and do
> the locking thing.
I think using a slab is much nicer than the current global-struct flags.
But it's not as flexible as a callback, for two reasons:
- in the lazy case, the caller might not even have loaded the slab
yet. On the other hand, it might be sufficient to just be broad
there, and just always pre-populate the slab when
is_repository_shallow(), before we even call into prune_shallow().
If we have _any_ entries in the shallow file, we'd need to compute
reachability.
- it precludes any optimizations that compute partial reachability.
Asking "is XYZ reachable" is a much easier question to answer than
"show me the full reachability graph." At the least, it lets you
stop the graph traversal early. And with generation numbers, it can
even avoid traversing down unproductive segments of the graph.
I think it's OK to switch to a slab for now if you don't want to do the
callback thing. But I think a callback is probably long-term the right
thing (and I actually think it may be _less_ work to implement, since
then prune would probably be OK sticking with the global struct flags).
-Peff
next prev parent reply other threads:[~2018-07-18 17:45 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-13 20:18 [PATCH 0/2] repack -ad: fix after `fetch --prune` in a shallow repository Johannes Schindelin via GitGitGadget
2018-07-11 22:17 ` [PATCH 1/2] repack: point out a bug handling stale shallow info Johannes Schindelin via GitGitGadget
2018-07-11 22:23 ` [PATCH 2/2] repack -ad: prune the list of shallow commits Johannes Schindelin via GitGitGadget
2018-07-13 20:31 ` Jeff King
2018-07-14 21:56 ` Johannes Schindelin
2018-07-16 17:36 ` Jeff King
2018-07-17 16:25 ` Junio C Hamano
2018-07-19 16:42 ` Johannes Schindelin
2018-07-19 20:49 ` Junio C Hamano
2018-07-20 9:30 ` Junio C Hamano
2018-07-20 19:31 ` Jeff King
2018-07-17 17:28 ` Duy Nguyen
2018-07-17 19:41 ` Jeff King
2018-07-18 17:31 ` Duy Nguyen
2018-07-18 17:45 ` Jeff King [this message]
2018-07-18 17:48 ` Duy Nguyen
2018-07-17 16:39 ` Duy Nguyen
2018-07-17 16:48 ` Duy Nguyen
2018-07-19 17:50 ` Johannes Schindelin
2018-07-17 13:51 ` [PATCH v2 0/2] repack -ad: fix after `fetch --prune` in a shallow repository Johannes Schindelin via GitGitGadget
2018-07-17 13:51 ` [PATCH v2 1/2] repack: point out a bug handling stale shallow info Johannes Schindelin via GitGitGadget
2018-07-17 13:51 ` [PATCH v2 2/2] repack -ad: prune the list of shallow commits Johannes Schindelin via GitGitGadget
2018-07-17 17:45 ` Eric Sunshine
2018-07-17 19:15 ` [PATCH v2 0/2] repack -ad: fix after `fetch --prune` in a shallow repository Jeff King
2018-07-17 19:20 ` Jeff King
2018-07-19 17:48 ` Johannes Schindelin
2018-10-22 22:05 ` [PATCH v3 0/3] repack -ad: fix after fetch --prune " Johannes Schindelin via GitGitGadget
2018-10-22 22:05 ` [PATCH v3 1/3] repack: point out a bug handling stale shallow info Johannes Schindelin via GitGitGadget
2018-10-24 3:39 ` Junio C Hamano
2018-10-24 8:12 ` Johannes Schindelin
2018-10-24 8:38 ` Johannes Schindelin
2018-10-22 22:05 ` [PATCH v3 2/3] shallow: offer to prune only non-existing entries Johannes Schindelin via GitGitGadget
2018-10-24 3:47 ` Junio C Hamano
2018-10-24 8:01 ` Johannes Schindelin
2018-10-24 15:56 ` Johannes Schindelin
2018-10-25 18:54 ` Jonathan Tan
2018-10-26 7:59 ` Johannes Schindelin
2018-10-26 20:49 ` Jonathan Tan
2018-10-29 20:45 ` Johannes Schindelin
2018-10-22 22:05 ` [PATCH v3 3/3] repack -ad: prune the list of shallow commits Johannes Schindelin via GitGitGadget
2018-10-24 3:56 ` Junio C Hamano
2018-10-24 8:02 ` Johannes Schindelin
2018-10-23 10:15 ` [PATCH v3 0/3] repack -ad: fix after fetch --prune in a shallow repository Johannes Schindelin
2018-10-24 15:56 ` [PATCH v4 " Johannes Schindelin via GitGitGadget
2018-10-24 15:56 ` [PATCH v4 1/3] repack: point out a bug handling stale shallow info Johannes Schindelin via GitGitGadget
2018-10-24 15:56 ` [PATCH v4 2/3] shallow: offer to prune only non-existing entries Johannes Schindelin via GitGitGadget
2018-10-24 15:56 ` [PATCH v4 3/3] repack -ad: prune the list of shallow commits Johannes Schindelin via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180718174505.GA3084@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).