-
Notifications
You must be signed in to change notification settings - Fork 17.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: sync: add function to free objects removed from a Pool #23216
Comments
Why put such objects in the pool in the first place? |
sync.Pool is not a cache, that was an explicit design decision. |
Why put any object in a pool?
Someone should update the documentation, then: "Pool's purpose is to cache allocated but unused items for later reuse..." |
In CL/41860043, Russ states the following:
|
@neild yup, that is really bad wording. I remember at the time the discussion went something along the lines of “it’s called sync.Pool, because it doesn’t have cache semantics, it can happily throw away anything you put in it” |
I don't want to quibble on the semantics of "cache" vs. "free list", but the question of whether objects require release is orthogonal to whether they are interchangeable. I believe Russ's comment in CL/41860043 is regarding eviction policy in the sense of deciding when and whether to remove objects from the pool. This feature request isn't about controlling when items are removed from the pool, but to have a way to clean them up when they are. |
Practically speaking, the implementation of |
If sync.Pool cleans up at GC then why not just use runtime.Setfinalizer? |
@neild I fully agree with @AlexRouSg, that's exactly what finalizers are for, so much so that they are used in the standard library exactly for the use cases you're describing (closing file descriptors associated with an object being GCed): Line 52 in 0f3ab14
Line 132 in 0f3ab14
|
Finalizers don't work for an object with an associated goroutine. |
They can, as long as they are designed correctly.
The generic way to do this is to have an outer object on which the finalizer is set, that contains a pointer to an inner object that is the one used by the goroutine. Since the goroutine only holds a reference to the inner object, the outer object can be collected when it's evicted from the pool. When the outer object is collected, its finalizer is called. The finalizer can then stop the goroutine, making the inner object eligible for GC.
The price for the the generic naive solution is double indirection. There is a similarly generic approach that does not require double indirection but that makes the outer object hold two pointers instead of one, thereby causing increased load on the GC. It's a tradeoff that has to be evaluated on a case-by-case basis.
|
If a goroutine holds a pointer to an object that is also held by a |
I'm sure you can make finalizers work for this, in the worst case by putting in the pool a wrapper that has the finalizer attached but is not referenced by any other goroutine. Please do that. I regret having one kind of finalizer already. I would regret having two kinds at least four times as much. (You can decide whether my regret is quadratic or exponential in the number of finalizer kinds.) /cc @aclements @RLH |
Also:
I can see why this wording would seem appropriate — in this case, what's being cached is unused items, not used items. But I suppose, given this thread, a question would be does this merit rewording? I'm not sure how much more obscure the term "free list" is. |
Items stored in a
sync.Pool
may be silently removed at any time. This makes a Pool unsuited for managing objects which require explicit deallocation; e.g., objects with an associated open file or active goroutine.Proposal: Add a
Release
field tosync.Pool
containing a function which is called with items automatically removed from the pool. Add aClose
method which removes all objects from the pool.The text was updated successfully, but these errors were encountered: