git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Carlo Arenas <carenas@gmail.com>
To: avarab@gmail.com
Cc: sandals@crustytoothpaste.net, git@vger.kernel.org, pcre-dev@exim.org
Subject: Re: [RFC PATCH 1/2] grep: fallback to interpreter if JIT fails with pcre1
Date: Tue, 11 Dec 2018 12:11:36 -0800	[thread overview]
Message-ID: <CAPUEspjHs7+G+FHXjxb4rCcNLqaybbhZESik=14_9Q5h2HMzMA@mail.gmail.com> (raw)
In-Reply-To: <87pnu9yekk.fsf@evledraar.gmail.com>

On Mon, Dec 10, 2018 at 12:24 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, Dec 10 2018, brian m. carlson wrote:
> > Considering that some Linux users use PaX kernels with standard
> > distributions and that most BSD kernels can be custom-compiled with a
> > variety of options enabled or disabled, I think this is something we
> > should detect dynamically.
>
> Right. I'm asking whether we're mixing up cases where it can always be
> detected at compile-time on some systems v.s. cases where it'll
> potentially change at runtime.

the closer we come to a system specific issues is with macOS where the
compiler (in some newer versions) is allocating the memory using the
MAP_JIT flag, which seems was originally meant to be only used in iOS
and has the strange characteristic of failing the mmap for versions
older than 10.14 if it was called more than once.

IMHO as brian pointed out, this is better done at runtime.

> >> I'm inclined to suggest that we should have another ifdef here for "if
> >> JIT fails I'd like it to die", so that e.g. packages I build (for
> >> internal use) don't silently slow down in the future, only for me to
> >> find some months later that someone enabled an overzealous SELinux
> >> policy and we swept this under the rug.
> >
> > My view is that JIT is a nice performance optimization, but it's
> > optional. I honestly don't think it should even be exposed through the
> > API: if it works, then things are faster, and if it doesn't, then
> > they're not. I don't see the value in an option for causing things to be
> > broken if someone improves the security of the system.
>
> For many users that's definitely the case, but for others that's like
> saying a RDBMS is still going to be functional if the "ORDER BY"
> function degrades to bubblesort. The JIT improves performance my
> multi-hundred percents sometimes, so some users (e.g. me) rely on that
> not being silently degraded.

the opposite is also true, specially considering that some old
versions of pcre result in a segfault instead of an error message and
therefore since there is no way to disable JIT, the only option left
is not to use `git grep -P` (or the equivalent git log call)

> So I'm wondering if we can have something like:
>
>     if (!jit)
>         if (must_have_jit)
>             BUG(...); // Like currently
>         else
>             fallback(); // new behavior

I am wondering if something like a `git doctor` command might be an
interesting alternative to this.

This way we could (for ex: in NetBSD) give the user a hint of what to
do to make their git grep -P faster when we detect we are running the
fallback, and might be useful as well to provide hints for
optimizations that could be used in other cases (probably even
depending on the size of the git repository)

For your use case, you just need to add a crontab that will trigger an
alarm if this command ever mentions PCRE

Carlo

  reply	other threads:[~2018-12-11 20:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-09 23:00 [RFC PATCH 0/2] fallback to interpreter if JIT fails with pcre Carlo Marcelo Arenas Belón
2018-12-09 23:00 ` [RFC PATCH 1/2] grep: fallback to interpreter if JIT fails with pcre1 Carlo Marcelo Arenas Belón
2018-12-09 23:51   ` Ævar Arnfjörð Bjarmason
2018-12-10  0:42     ` brian m. carlson
2018-12-10  1:25       ` Carlo Arenas
2018-12-10  6:42       ` Junio C Hamano
2018-12-10  8:24       ` Ævar Arnfjörð Bjarmason
2018-12-11 20:11         ` Carlo Arenas [this message]
2018-12-11 20:51           ` Ævar Arnfjörð Bjarmason
2018-12-10  6:54     ` Junio C Hamano
2018-12-10  6:48   ` Junio C Hamano
2018-12-09 23:00 ` [RFC PATCH 2/2] grep: fallback to interpreter if JIT fails with pcre2 Carlo Marcelo Arenas Belón
2018-12-10  7:00   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPUEspjHs7+G+FHXjxb4rCcNLqaybbhZESik=14_9Q5h2HMzMA@mail.gmail.com' \
    --to=carenas@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=pcre-dev@exim.org \
    --cc=sandals@crustytoothpaste.net \
    --subject='Re: [RFC PATCH 1/2] grep: fallback to interpreter if JIT fails with pcre1' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).