A similar change was made in https://golang.org/cl/44150043, by [@bradfitz]. It was rolled back in https://golang.org/cl/48190043. [@dvyukov] said: "The remaining concern is: Are there cases where a program creates lots of local short-lived regexps (one-shot), and it's not possible to replace them with global ones? For this case we've introduced a global contended mutex."
I'm guessing that comment refers to allPoolsMu here:
It strikes me as odd that sync.Pool should have a global Mutex at all. After all, sync.Pool explicitly cooperates with the garbage collector (and will likely cooperate even more after #22950), and the garbage collector will trace all of the live sync.Pool instances in the course of a normal garbage collection.
The allPools slice appears to exist so that the garbage collector can clear all of the sync.Pool instances before tracing them. We have to identify Pools before tracing them so that we don't over-retain pooled objects, but it seems like we could do that without acquiring global locks beyond the usual GC safepoints.
For example, we could add newly-allocated pools to a per-G list, and move that list to the global list only when the thread running that G acquires the scheduler lock.
If we happen to miss a new Pool on the current GC cycle (for example, because its goroutine wasn't descheduled until near the end of the cycle), that's ok: we'll just wait until the next GC cycle to clear it.
That could make sync.Pool substantially more efficient for objects that tend to be long-lived but are sometimes short-lived too (such as compiled regexps).
The text was updated successfully, but these errors were encountered:
In the review for https://golang.org/cl/101715 (“regexp: use sync.Pool to cache regexp.machine objects”), @ianlancetaylor notes:
I'm guessing that comment refers to
allPoolsMu
here:go/src/sync/pool.go
Line 242 in 2e84dc2
It strikes me as odd that
sync.Pool
should have a globalMutex
at all. After all,sync.Pool
explicitly cooperates with the garbage collector (and will likely cooperate even more after #22950), and the garbage collector will trace all of the livesync.Pool
instances in the course of a normal garbage collection.The
allPools
slice appears to exist so that the garbage collector can clear all of thesync.Pool
instances before tracing them. We have to identifyPool
s before tracing them so that we don't over-retain pooled objects, but it seems like we could do that without acquiring global locks beyond the usual GC safepoints.For example, we could add newly-allocated pools to a per-G list, and move that list to the global list only when the thread running that G acquires the scheduler lock.
If we happen to miss a new
Pool
on the current GC cycle (for example, because its goroutine wasn't descheduled until near the end of the cycle), that's ok: we'll just wait until the next GC cycle to clear it.That could make
sync.Pool
substantially more efficient for objects that tend to be long-lived but are sometimes short-lived too (such as compiled regexps).The text was updated successfully, but these errors were encountered: