On Thu, Nov 17, 2022 at 02:22:20PM +0100, Ævar Arnfjörð Bjarmason wrote: > > On Thu, Nov 17 2022, Teng Long wrote: > > > From: Teng Long <dyroneteng@xxxxxxxxx> > > > > 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. I think this falls into the same trap as the series on 'git show-ref --count' that I worked on earlier this year [1]. At the time, it seemed useful to me (since I was working in an environment where counting the number of lines from 'show-ref' was more cumbersome than teaching 'show-ref' how to perform the same task itself). And I stand by that value judgement, but sharing the patches with the Git mailing list under the intent to merge it in was wrong. Those patches were too niche to be more generally useful, and would only serve to clutter up the UI of show-ref for everybody else. So I was glad to drop that topic. Now, I'd be curious to hear from Teng whether or not there *is* something that we're missing, since if so, I definitely want to know what it is. But absent of that, I tend to agree with Ævar that I'm not compelled by replacing 'ls-tree | grep <pattern>' with 'ls-tree --pattern=<pattern>', especially if the latter is slower than the former. Thanks, Taylor [1]: https://lore.kernel.org/git/cover.1654552560.git.me@xxxxxxxxxxxx/
