Re: Can we just get rid of kwset & obstack in favor of optimistically using PCRE v2 JIT?
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jun 15, 2019, 22:14 UTC
- Message-ID
- <87r27u8pie.fsf@evledraar.gmail.com>
- In-Reply-To
- <20190615191514.GD8616@genre.crustytoothpaste.net>
On Sat, Jun 15 2019, brian m. carlson wrote:
Show 16 quoted lines
> On 2019-06-14 at 22:55:17, Ævar Arnfjörð Bjarmason wrote: >> It seems most packagers build with PCRE v2 now (CC: >> git-packagers@). I.e. we'd keep something like compile_fixed_regexp() >> (and as an aside just use PCRE's \Q...\E instead of our own escaping). >> >> We'd have performance regression for platforms that use kwset() now but >> don't build pcre2, or where pcre2 jit doesn't work. Does anyone care? > > I know that there are people shipping newer versions of Git using CentOS > 6, which IIRC doesn't ship PCRE 2[0]. Since having to ship your own PCRE > is a security maintenance nightmare, it's probably best to leave this at > least compatible with non-PCRE 2 systems until November 2020. At that > point, I'm happy to drop support for it. > > If it would work but just be slower with PCRE 1, I'm not too terribly > concerned. Let that be an incentive to users to upgrade.
Not just PCRE, but if you don't have PCRE at all things would still work perfectly fine.
I.e. all we're talking about is how to treat this internal optimization. If we'd never imported kwset we'd still be perfectly capable of searching for fixed strings with grep/pickaxe, it would have just been slower.
So platforms that don't have PCRE at all would be slowed down by something like what the benchmark in my https://public-inbox.org/git/87v9x793qi.fsf@evledraar.gmail.com/ upthread shows.
Or not, maybe their C library POSIX regcomp()/regexec() is faster.
Platforms that do have PCRE would be faster than they are now, and we could stop shipping this kwset code.
The *only* case where what I've outlined above isn't true is cases where the pattern being matched has a \0. See my 966be95549 ("grep: add tests to fix blind spots with \0 patterns", 2017-05-20) for how that behaves.
There our current behavior is IMNSHO insane, and is certainly undocumented and unreliable (i.e. it behaves differently if you e.g. have non-ASCII along with \0 in the pattern, none of this is documented).
Having poked a bit at that I think the only sane thing there is to just outright die unless you have PCRE v2, which is the only backend we have that has any hope of handling that sanely.
Show 7 quoted lines
> Also, as Carlos pointed out, not all platforms will have the JIT support > functional, such as OpenBSD, NetBSD, and PaX Linux systems. That may be > more of a blocker than the CentOS issue, especially since people run PaX > kernels with standard distros. > > [0] I'm not certain because CentOS 6 Docker images segfault on newer > kernels and I'm too lazy to download a live CD image for testing.