Re: [PATCH 0/9] for-each-ref optimizations & usability improvements

[Junio C Hamano <gitster@pobox.com>]

"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This series is a bit of an informal follow-up to [1], adding some more
> substantial optimizations and usability fixes around ref
> filtering/formatting. Some of the changes here affect user-facing behavior,
> some are internal-only, but they're all interdependent enough to warrant
> putting them together in one series.
>
> [1]
> https://lore.kernel.org/git/pull.1594.v2.git.1696888736.gitgitgadget@gmail.com/
>
> Patch 1 changes the behavior of the '--no-sort' option in 'for-each-ref',
> 'tag', and 'branch'. Currently, it just removes previous sort keys and, if
> no further keys are specified, falls back on ascending refname sort (which,
> IMO, makes the name '--no-sort' somewhat misleading).

We can read it changes the behaviour and what the current behaviour
is, but I presume that the untold new behaviour with --no-sort is to
show the output in an unspecified order of implementation's
convenience?  I think it makes quite a lot of sense if that is what
is done.

> Patch 2 updates the 'for-each-ref' docs to clearly state what happens if you
> use '--omit-empty' and '--count' together. I based the explanation on what
> the current behavior is (i.e., refs omitted with '--omit-empty' do count
> towards the total limited by '--count').

OK.

> Patches 3-7 incrementally refactor various parts of the ref
> filtering/formatting workflows in order to create a
> 'filter_and_format_refs()' function. If certain conditions are met (sorting
> disabled, no reachability filtering or ahead-behind formatting), ref
> filtering & formatting is done within a single 'for_each_fullref_in'
> callback. Especially in large repositories, this makes a huge difference in
> memory usage & runtime for certain usages of 'for-each-ref', since it's no
> longer writing everything to a 'struct ref_array' then repeatedly whittling
> down/updating its contents.

OK.  I was wondering if you are going threaded implementation, until
I read into 6th line ;-)

> Patch 8 introduces a new option to 'for-each-ref' called '--full-deref'.
> When provided, any format fields for the dereferenced value of a tag (e.g.
> "%(*objectname)") will be populated with the fully peeled target of the tag;
> right now, those fields are populated with the immediate target of a tag
> (which can be another tag). This avoids the need to pipe 'for-each-ref'
> results to 'cat-file --batch-check' to get fully-peeled tag information. It
> also benefits from the 'filter_and_format_refs()' single-iteration
> optimization, since 'peel_iterated_oid()' may be able to read the
> pre-computed peeled OID from a packed ref. A couple notes on this one:
>
>  * I went with a command line option for '--full-deref' rather than another
>    format specifier (like ** instead of *) because it seems unlikely that a
>    user is going to want to perform a shallow dereference and a full
>    dereference in the same 'for-each-ref'. There's also a NEEDSWORK going
>    all the way back to the introduction of 'for-each-ref' in 9f613ddd21c
>    (Add git-for-each-ref: helper for language bindings, 2006-09-15) that (to
>    me) implies different dereferencing behavior corresponds to different use
>    cases/user needs.

Makes quite a lot of sense.

>  * I'm not attached to '--full-deref' as a name - if someone has an idea for
>    a more descriptive name, please suggest it!

Another candidate verb may be "to peel", and I have no strong
opinion between it and "to dereference".  But I have a mild aversion
to an abbreviation that is not strongly established.

> Finally, patch 9 adds performance tests for 'for-each-ref', showing the
> effects of optimizations made throughout the series. Here are some sample
> results from my Ubuntu VM (test names shortened for space):

Nice.