regexp: revert "use sync.Pool to cache regexp.machine objects"

Revert CL 101715.

The size of a sync.Pool scales linearly with GOMAXPROCS,
making it inappropriate to put a sync.Pool in any individually
allocated object, as the sync.Pool documentation explains.
The change also broke DeepEqual on regexps.

I have a cleaner way to do this with global sync.Pools but it's
too late in the cycle. Will revisit in Go 1.12. For now, revert.

Fixes #26219.

Change-Id: Ie632e709eb3caf489d85efceac0e4b130ec2019f
Reviewed-on: https://go-review.googlesource.com/122596
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/regexp/regexp.go b/src/regexp/regexp.go
index 0d10aa1e225fb3cd37a42b47242d06bfed791c32..811187175d9ff9e0aa4eb2b36e9909c66dfac4e1 100644 (file)
--- a/src/regexp/regexp.go
+++ b/src/regexp/regexp.go
@@ -79,13 +79,15 @@ import (
 // A Regexp is safe for concurrent use by multiple goroutines,
 // except for configuration methods, such as Longest.
 type Regexp struct {
-       // cache of machines for running regexp. This is a shared pointer across
-       // all copies of the original Regexp object to decrease the overall
-       // memory footprint of the regexps (since there will be one machine
-       // cached per thread instead of one per thread per copy).
-       machines *sync.Pool
+       // read-only after Compile
+       regexpRO
 
-       // everything below is read-only after Compile
+       // cache of machines for running regexp
+       mu      sync.Mutex
+       machine []*machine
+}
+
+type regexpRO struct {
        expr           string         // as passed to Compile
        prog           *syntax.Prog   // compiled program
        onepass        *onePassProg   // onepass program or nil
@@ -107,10 +109,14 @@ func (re *Regexp) String() string {
 
 // Copy returns a new Regexp object copied from re.
 //
-// Deprecated: This exists for historical reasons.
+// When using a Regexp in multiple goroutines, giving each goroutine
+// its own copy helps to avoid lock contention.
 func (re *Regexp) Copy() *Regexp {
-       re2 := *re
-       return &re2
+       // It is not safe to copy Regexp by value
+       // since it contains a sync.Mutex.
+       return &Regexp{
+               regexpRO: re.regexpRO,
+       }
 }
 
 // Compile parses a regular expression and returns, if successful,
@@ -173,21 +179,15 @@ func compile(expr string, mode syntax.Flags, longest bool) (*Regexp, error) {
        if err != nil {
                return nil, err
        }
-       onepass := compileOnePass(prog)
        regexp := &Regexp{
-               expr:        expr,
-               prog:        prog,
-               onepass:     onepass,
-               numSubexp:   maxCap,
-               subexpNames: capNames,
-               cond:        prog.StartCond(),
-               longest:     longest,
-       }
-       regexp.machines = &sync.Pool{
-               New: func() interface{} {
-                       z := progMachine(prog, onepass)
-                       z.re = regexp
-                       return z
+               regexpRO: regexpRO{
+                       expr:        expr,
+                       prog:        prog,
+                       onepass:     compileOnePass(prog),
+                       numSubexp:   maxCap,
+                       subexpNames: capNames,
+                       cond:        prog.StartCond(),
+                       longest:     longest,
                },
        }
        if regexp.onepass == notOnePass {
@@ -208,7 +208,17 @@ func compile(expr string, mode syntax.Flags, longest bool) (*Regexp, error) {
 // It uses the re's machine cache if possible, to avoid
 // unnecessary allocation.
 func (re *Regexp) get() *machine {
-       return re.machines.Get().(*machine)
+       re.mu.Lock()
+       if n := len(re.machine); n > 0 {
+               z := re.machine[n-1]
+               re.machine = re.machine[:n-1]
+               re.mu.Unlock()
+               return z
+       }
+       re.mu.Unlock()
+       z := progMachine(re.prog, re.onepass)
+       z.re = re
+       return z
 }
 
 // put returns a machine to the re's machine cache.
@@ -221,7 +231,9 @@ func (re *Regexp) put(z *machine) {
        z.inputString.str = ""
        z.inputReader.r = nil
 
-       re.machines.Put(z)
+       re.mu.Lock()
+       re.machine = append(re.machine, z)
+       re.mu.Unlock()
 }
 
 // MustCompile is like Compile but panics if the expression cannot be parsed.