Re: [PATCH 1/7] grep: don't redundantly compile throwaway patterns under threading

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, May 12, 2017 at 7:35 PM, Brandon Williams <bmwill@google.com> wrote:
> On 05/11, Ęvar Arnfjörš Bjarmason wrote:
>> Change the pattern compilation logic under threading so that grep
>> doesn't compile a pattern it never ends up using on the non-threaded
>> code path, only to compile it again N times for N threads which will
>> each use their own copy, ignoring the initially compiled pattern.
>
> Is there a reason each thread needs to compile the patterns as opposed
> to them being compiled a single time and being copies N time for N
> threads?

Not really, just simplicity.

I'll amend the commit message to mention that I did this not as an
optimization, but just so the code is easier to reason about and
debug, when debugging this I found that there were two different
stacktraces to get to where I was compiling the pattern, and e.g.
"give each compilation/execution an id" for ad-hoc debugging would
always end up with one unused pattern, confusingly.

The reason it's like this is because it's a side-effect of duplicating
the whole grep_opt structure, which is not thread safe, writable, and
munged during execution, that's where the pattern is stored.

We could re-use the compiled regexp itself for POSIX, for PCRE you can
pre-JIT, post-JIT you can only partially do it, i.e. the pattern is
const, thread safe and can be shared, but you need to also marshal
around mutable per-thread for the JIT stack etc.

I looked into e.g. splitting the API into some "do & alloc threadsafe
stuff", "spawn thread", "do and alloc non-threadsafe stuff", but the
execution time of grep_opt_dup() & pattern compilation is trivial
compared to actually executing the grep, so there was no point. Even
with the more expensive JIT some of the most expensive PCRE patterns
take something like 0.0X milliseconds to compile at most[1].

1. http://sljit.sourceforge.net/pcre.html