Subject: Re: [RFC PATCH 0/6] ls-tree: introduce '--pattern' option

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 17 Nov 2022 14:22:20 +0100

On Thu, Nov 17 2022, Teng Long wrote:

> From: Teng Long <[email-masked]>
>
> This RFC patch introduce a new "ls-tree" option "--pattern", aim to match the
> entries by regex then filter the output which we may want to achieve. It also
> contains some commit for preparation or cleanup.
>
> The idea may be not comprehensive and the tests for it might be insufficient
> too, but I'd like to listen the suggestion from the community to decide if it's
> worth going forward with.

I applied this series, compiled with CFLAGS=-O3, and:
	
	$ hyperfine './git ls-tree --pattern=[ab]c.*d -r HEAD' './git ls-tree -r HEAD | grep [ab]c.*d' -w 10 -r 20
	Benchmark 1: ./git ls-tree --pattern=[ab]c.*d -r HEAD
	  Time (mean ± σ):      14.8 ms ±   0.1 ms    [User: 12.0 ms, System: 2.8 ms]
	  Range (min … max):    14.6 ms …  15.0 ms    20 runs
	
	Benchmark 2: ./git ls-tree -r HEAD | grep [ab]c.*d
	  Time (mean ± σ):      12.5 ms ±   0.1 ms    [User: 10.0 ms, System: 4.0 ms]
	  Range (min … max):    12.4 ms …  12.8 ms    20 runs
	
	Summary
	  './git ls-tree -r HEAD | grep [ab]c.*d' ran
	    1.18 ± 0.01 times faster than './git ls-tree --pattern=[ab]c.*d -r HEAD'

So the value-proposition isn't really clear to me, and the included
docs, commit messages & this CL don't answer the "why not just 'grep'"
question?

That's faster even with another process for me, but likely that's
because you're doing the regex matching really inefficiently
(e.g. malloc-ing again for each line), which could be "fixed".

But in any setup which cares about the performance you're likely piping
to another process anyway (the thing using the data), which could do
that filtering without thep "grep" process.

So I don't see the value in doing this, but maybe I'm just missing
something.

And, in terms of the complexity for git's implementation it would be
really good to avoid the complexity of a "--pattern", "--sort-lines"
etc., if those use-cases can be satisfied by piping into "grep" or
"sort" instead.

Some of the pre-cleanup here looks good, but it's unrelated to the rest
of the series. I think in any case that it would be nice to see that as
another topic.