Skip to content
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

Recycler: better lifecycle control for pooled instances #5214

Closed
hhoffstaette opened this issue Feb 21, 2014 · 2 comments
Closed

Recycler: better lifecycle control for pooled instances #5214

hhoffstaette opened this issue Feb 21, 2014 · 2 comments

Comments

@hhoffstaette
Copy link

The default DequeueRecycler.close() currently simply clears its internal queue and drops all instances on the floor for GC. During work for issue #5159 I found that this can be improved:

  • closing a recycler should properly dispose of the pooled instances via the factory
  • the factory currently conflates the functionality of per-instance "clean up & return to pool" and "destroy instance" behind the (according to Adrien badly named) cleanup() callback method.

So I suggest that we:

  • rename Recycler.C.clear(T) to recycle(T)
  • add a Recycler.C.destroy(T) to indicate that this particular instance needs to die
  • destroy() can then also be used when the recycler wants to kill off excess instances (on return).

While we're renaming things I'd also like to vote to rename the Recycler.C and Recycler.V interfaces to something more descriptive. :)

@jpountz
Copy link
Contributor

jpountz commented Feb 21, 2014

+1

@hhoffstaette hhoffstaette self-assigned this Feb 21, 2014
@dadoonet dadoonet assigned uboness and unassigned hhoffstaette Feb 24, 2014
@jpountz jpountz assigned hhoffstaette and unassigned uboness Feb 24, 2014
hhoffstaette pushed a commit that referenced this issue Feb 25, 2014
- introduce additional destroy() callback that allows better control
over internals of recycled data
- introduced AbstractRecyclerC as superclass for Recycler factories
(Adrien) with empty destroy() by default
- added tests for destroy()
- cleaned up Recycler tests (reduce copy&paste)
hhoffstaette pushed a commit that referenced this issue Feb 25, 2014
- introduce additional destroy() callback that allows better control
over internals of recycled data
- introduced AbstractRecyclerC as superclass for Recycler factories
(Adrien) with empty destroy() by default
- added tests for destroy()
- cleaned up Recycler tests (reduce copy&paste)
@hhoffstaette
Copy link
Author

Fixed in bac09e2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants