Re: [PATCH v2 7/8] grep: simplify config parsing, change grep.<rx config> interaction

On Fri, Nov 12 2021, Junio C Hamano wrote:

Ævar Arnfjörð Bjarmason  [off-list ref] writes:
[...]

I'll try to reply to all the rest of the feedback, just really quick on
this, because I think it might represent a bit of a gordian knot.

Another thing I have trouble with is your mention of "keep working".
Are you proposing to deliberately break what is working as users
correctly expect?  Why?

Yes, I'd like to change the behavior, because it makes the grep API much
easier to deal with, and beacuse I think it impacts nobody in practice.

The real goal for this series is that I've got pending patches to
speedup diffcore-pickaxe massively by moving it over to PCRE & drop the
kwset.c code. An old perf test I dug up for that is in [1].

To do that I needed to re-use the bits of grep.c machinery that deal
with setting up patterns, dealing with BRE,ERE,PCRE etc. elsewhere.

I *can* do that in a different way, but it's going to be much easier if
we can gradually evolve the already working grep API to become an
internal textual pattern matching API. Eventually I'd like to move all
of regcomp()/regexec() over to such a thing, because we can for any
other ranodm thing we use regexes for get speedups by using PCRE (and
optionally use its interface to understand BRE/ERE syntax).

The alternative is to split that part off from grep.c, which is a bit
more painful, or to have the init bits etc. take some "no config doesn't
go first", "no it goes first" flags just to support this one API user.

So it would be generally useful to know if you're at all open to
that. Reading between the lines in some other comments I fear that it
may be a "no" except if we mark it as deprecated, wait some years, maybe
remove/change it then etc.

1.

    GIT_TEST_LONG= GIT_PERF_REPEAT_COUNT=10 GIT_PERF_MAKE_OPTS='-j8 USE_LIBPCRE=1 CFLAGS=-O3 LIBPCREDIR=/home/avar/g/pcre2/inst' ./run origin/next HEAD -- p4209-pickaxe.sh
    Test                                                                      origin/next       HEAD
    ------------------------------------------------------------------------------------------------------------------
    4209.1: git log -S'int main' <limit-rev>..                                0.38(0.36+0.01)   0.37(0.33+0.04) -2.6%
    4209.2: git log -S'æ' <limit-rev>..                                       0.51(0.47+0.04)   0.32(0.27+0.05) -37.3%
    4209.3: git log --pickaxe-regex -S'(int|void|null)' <limit-rev>..         0.72(0.68+0.03)   0.57(0.54+0.03) -20.8%
    4209.4: git log --pickaxe-regex -S'if *\([^ ]+ & ' <limit-rev>..          0.60(0.55+0.02)   0.39(0.34+0.05) -35.0%
    4209.5: git log --pickaxe-regex -S'[àáâãäåæñøùúûüýþ]' <limit-rev>..       0.43(0.40+0.03)   0.50(0.44+0.06) +16.3%
    4209.6: git log -G'(int|void|null)' <limit-rev>..                         0.64(0.55+0.09)   0.63(0.56+0.05) -1.6%
    4209.7: git log -G'if *\([^ ]+ & ' <limit-rev>..                          0.64(0.59+0.05)   0.63(0.56+0.06) -1.6%
    4209.8: git log -G'[àáâãäåæñøùúûüýþ]' <limit-rev>..                       0.63(0.54+0.08)   0.62(0.55+0.06) -1.6%
    4209.9: git log -i -S'int main' <limit-rev>..                             0.39(0.35+0.03)   0.38(0.35+0.02) -2.6%
    4209.10: git log -i -S'æ' <limit-rev>..                                   0.39(0.33+0.06)   0.32(0.28+0.04) -17.9%
    4209.11: git log -i --pickaxe-regex -S'(int|void|null)' <limit-rev>..     0.90(0.84+0.05)   0.58(0.53+0.04) -35.6%
    4209.12: git log -i --pickaxe-regex -S'if *\([^ ]+ & ' <limit-rev>..      0.71(0.64+0.06)   0.40(0.37+0.03) -43.7%
    4209.13: git log -i --pickaxe-regex -S'[àáâãäåæñøùúûüýþ]' <limit-rev>..   0.43(0.40+0.03)   0.50(0.46+0.04) +16.3%
    4209.14: git log -i -G'(int|void|null)' <limit-rev>..                     0.64(0.57+0.06)   0.62(0.56+0.05) -3.1%
    4209.15: git log -i -G'if *\([^ ]+ & ' <limit-rev>..                      0.65(0.59+0.06)   0.63(0.54+0.08) -3.1%
    4209.16: git log -i -G'[àáâãäåæñøùúûüýþ]' <limit-rev>..                   0.63(0.55+0.08)   0.62(0.56+0.05) -1.6%