Subject: Re: [PATCH v2] grep: fall back to interpreter if JIT memory allocation fails

From: Junio C Hamano <[email-masked]>
Date: Mon, 30 Jan 2023 12:08:31 -0800

Junio C Hamano <[email-masked]> writes:

> If we were to keep that "die", it is absolutely required, I would
> think.  Users who got their Git with JIT-enabled pcre2 may be
> viewing JIT merely as "a clever optimization the implementation is
> allowed to use when able", without knowing and more importantly
> without wanting to know how to disable it from within their
> patterns.
>
> But can't we drop that die() if we took the v1 route?

Having said all that, I do not mind queuing v2 if the "use *NO_JIT
to disable" is added to the message to help users who are forced to
redo the query.

And in practice, it shouldn't make that much difference, because the
only scenario (other than the SELinux-like situation where JIT is
compiled in but does not work at all) that the difference may matter
would happen when a non-trivial portion of the patterns users use
are not workable with JIT, but if that were the case, we would have
written JIT off as not mature enough and not yet usable long time
ago.  So, in practice, patterns refused by JIT would be a very tiny
minority to matter in real life, and "failing fast to inconvenience
users" would not be too bad.

So while I still think v1's simplicity is the right thing to have
here, I think it is waste of our braincell to compare v1 vs v2.  As
v2 gives smaller incremental behaviour change perceived by end
users, if somebody really wanted to, I'd expect that a low-hanging
fruit #leftoverbit on top of such a patch, after the dust settles,
would be to

 (1) rename pcre2_jit_functional() to fall_back_to_interpreter() or
     something,

 (2) add a configuration variable to tell fall_back_to_interpreter()
     that any form of JIT error is allowed to fall back to
     interpreter().

and such a patch will essentially give back the simplicity of v1 to
folks who opt into the configuration.

Thanks.