code review 110020045: regexp: use sync.Pool for machine cache

regexp: use sync.Pool for machine cache Controversial. For discussion. benchmark old ns/op new ns/op delta BenchmarkMachineCacheParallel 1514 1514 +0.00% BenchmarkMachineCacheParallel-2 907 825 -9.04% BenchmarkMachineCacheParallel-4 595 433 -27.23% BenchmarkMachineCacheParallel-8 591 364 -38.41% Fixes Issue 8232

[bradfitz, 2014-06-18 18:06:03]

Hello rsc@golang.org, r@golang.org (cc: golang-codereviews@googlegroups.com, jrockway@google.com), I'd like you to review this change to https://go.googlecode.com/hg/

[r, 2014-06-18 18:11:41]

LGTM but wait for rsc I was sitting at my desk waiting for you to send this CL....

[bradfitz, 2014-06-18 18:13:20]

On Wed, Jun 18, 2014 at 11:11 AM, <r@golang.org> wrote: > LGTM but wait for rsc > > I was sitting at my desk waiting for you to send this CL.... Well, I wanted to see the numbers. If there's an alternative without sync.Pool, I'm curious. 

[dvyukov, 2014-06-18 19:43:36]

AFAIR the discussion last time was along the lines of: It is a degradation for local regexps (e.g. when you receive the regexp in a request, compile it and use several times during request processing). First sync.Pool is larger; second it will do per-P caching, so even if you use it in a single goroutine, it can allocate several objects. The optimal solution that satisfies both global and local regexps would be to have a pointer to a single object and a sync.Pool. A goroutine first tries to acquire the object from the pointer using atomic.Exchange and release it using atomic.Store. If that single object pool even underflows (a goroutine finds that the pointer is nil when it tries to acquire), all goroutines switch to sync.Pool. I am not sure we want this complexity. On Wed, Jun 18, 2014 at 11:13 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > On Wed, Jun 18, 2014 at 11:11 AM, <r@golang.org> wrote: >> >> LGTM but wait for rsc >> >> I was sitting at my desk waiting for you to send this CL.... > > > Well, I wanted to see the numbers. > > If there's an alternative without sync.Pool, I'm curious. > > -- > You received this message because you are subscribed to the Google Groups > "golang-codereviews" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-codereviews+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. 

[rsc, 2014-06-18 19:44:10]

not lgtm. We rolled this back once already. 

[r, 2014-06-19 05:39:16]

Apologies, I misread this CL and stupidly saw the cache as a global, not per-Regexp. I shouldn't have signed off.

[gobot, 2014-07-07 21:58:49]

R=close (assigned by r@golang.org)