Commit a41d2169 authored by Russ Cox's avatar Russ Cox

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: default avatarBrad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 7254cfc3
...@@ -79,13 +79,15 @@ import ( ...@@ -79,13 +79,15 @@ import (
// A Regexp is safe for concurrent use by multiple goroutines, // A Regexp is safe for concurrent use by multiple goroutines,
// except for configuration methods, such as Longest. // except for configuration methods, such as Longest.
type Regexp struct { type Regexp struct {
// cache of machines for running regexp. This is a shared pointer across // read-only after Compile
// all copies of the original Regexp object to decrease the overall regexpRO
// memory footprint of the regexps (since there will be one machine
// cached per thread instead of one per thread per copy).
machines *sync.Pool
// 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 expr string // as passed to Compile
prog *syntax.Prog // compiled program prog *syntax.Prog // compiled program
onepass *onePassProg // onepass program or nil onepass *onePassProg // onepass program or nil
...@@ -107,10 +109,14 @@ func (re *Regexp) String() string { ...@@ -107,10 +109,14 @@ func (re *Regexp) String() string {
// Copy returns a new Regexp object copied from re. // 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 { func (re *Regexp) Copy() *Regexp {
re2 := *re // It is not safe to copy Regexp by value
return &re2 // since it contains a sync.Mutex.
return &Regexp{
regexpRO: re.regexpRO,
}
} }
// Compile parses a regular expression and returns, if successful, // Compile parses a regular expression and returns, if successful,
...@@ -173,21 +179,15 @@ func compile(expr string, mode syntax.Flags, longest bool) (*Regexp, error) { ...@@ -173,21 +179,15 @@ func compile(expr string, mode syntax.Flags, longest bool) (*Regexp, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
onepass := compileOnePass(prog)
regexp := &Regexp{ regexp := &Regexp{
expr: expr, regexpRO: regexpRO{
prog: prog, expr: expr,
onepass: onepass, prog: prog,
numSubexp: maxCap, onepass: compileOnePass(prog),
subexpNames: capNames, numSubexp: maxCap,
cond: prog.StartCond(), subexpNames: capNames,
longest: longest, cond: prog.StartCond(),
} longest: longest,
regexp.machines = &sync.Pool{
New: func() interface{} {
z := progMachine(prog, onepass)
z.re = regexp
return z
}, },
} }
if regexp.onepass == notOnePass { if regexp.onepass == notOnePass {
...@@ -208,7 +208,17 @@ func compile(expr string, mode syntax.Flags, longest bool) (*Regexp, error) { ...@@ -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 // It uses the re's machine cache if possible, to avoid
// unnecessary allocation. // unnecessary allocation.
func (re *Regexp) get() *machine { 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. // put returns a machine to the re's machine cache.
...@@ -221,7 +231,9 @@ func (re *Regexp) put(z *machine) { ...@@ -221,7 +231,9 @@ func (re *Regexp) put(z *machine) {
z.inputString.str = "" z.inputString.str = ""
z.inputReader.r = nil 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. // MustCompile is like Compile but panics if the expression cannot be parsed.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment