Re: [PATCH v3 1/4] connected: do not sort input revisions

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

On Mon, Aug 02, 2021 at 12:00:02PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> > Introduce a new "--unsorted-input" flag to git-rev-list(1) which will
> > cause it to not sort the commits and adjust the connectivity check to
> > always pass the flag. This results in the following speedups, executed
> > in a clone of gitlab-org/gitlab [1]:
> > ...
> > [1]: https://gitlab.com/gitlab-org/gitlab.git. Note that not all refs
> >      are visible to clients.
> 
> So is this the 2.2 million refs thing?

Yeah, it is. The repo itself got 2.2 million refs, even though only 800k
are publicly visible. It got even more when one considers its alternate,
where it grows to 3.4 million in total.

> > @@ -3584,7 +3586,7 @@ int prepare_revision_walk(struct rev_info *revs)
> >  
> >  	if (!revs->reflog_info)
> >  		prepare_to_use_bloom_filter(revs);
> > -	if (revs->no_walk != REVISION_WALK_NO_WALK_UNSORTED)
> > +	if (revs->no_walk != REVISION_WALK_NO_WALK_UNSORTED && !revs->unsorted_input)
> >  		commit_list_sort_by_date(&revs->commits);
> 
> Looks quite straight-forward.
> 
> I however suspect that in the longer term it may be cleaner to get
> rid of REVSISION_WALK_NO_WALK_UNSORTED if we do this.  The knob that
> controls if we sort the initial traversal tips and the knob that
> controls if we walk from these tips used to be tied to --no-walk
> only because ca92e59e30b wanted to affect only no-walk case, but
> with your new finding, it clearly is not limited to the no-walk case
> to want to avoid sorting.

Right. The question also is what to do when the user calls `git rev-list
--no-walk=sorted --unsorted-input`. Do we sort? Don't we? Should we mark
these options as incompatible with each other and bail out? I guess just
bailing out would be the easiest solution for now.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]