Re: [PATCH 00/12] PCREv2 & more

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

Jeff King <peff@peff.net> writes:

> On Sat, Apr 08, 2017 at 01:24:54PM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> This adds PCRE v2 support, but as I was adding that I kept noticing
>> other related problems to fix. It's all bundled up into the same
>> series because much of it conflicts because it modifies the same test
>> or other code. Notes on each patch below.
>
> Overall, the series looks OK to me.
>
> I'm not sure if it is worth all the complexity to carry pcre1/pcre2 as
> run-time options. That does make it easier to do back-to-back
> comparisons, but it makes the code a lot more complicated. In particular
> I'm worried about subtle cases where we pcre1 turns into pcre2 (or vice
> versa) by use of the aliases. That shouldn't matter to a user for
> correctness, but it would throw off the benchmarking.
>
> If we literally just added USE_LIBPCRE2 and built against one or the
> other, then all the complexity would be limited to a few #ifdefs. The
> big drawback AFAICT is that anybody doing timing tests would have to
> recompile in between.

Yeah, having to dl two libs at runtime, even when you would ever use
just one in a single run, is less than ideal.  A small downside
inflicted on everybody will add up to million times more than a
larger downside only suffered by developers, so I tend to agree with
you that we probably should simplify to choose just one (or zero) at
compile time.

> So I dunno. I had hoped libpcre2 would be a hands-down win on timing,
> but it sounds like there's a little back-and-forth depending on the
> context. Is there a ticking clock on pcre1 going away? I suspect it will
> be around for many years to come, but if they'll continue optimizing
> pcre2 but not pcre1, then it may make sense to hitch our wagon to the
> one that upstream is actually working on.