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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: May 12, 2017, 18:17 UTC
Message-ID: <CACBZZX7x=9Pvuhy7Q9=ecnhvVmjt664enZOjk4B8LXErVL4oEg@mail.gmail.com>
In-Reply-To: <20170512173527.GD98586@google.com>

On Fri, May 12, 2017 at 7:35 PM, Brandon Williams <bmwill@google.com> wrote:

Show 9 quoted lines

> 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