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

Reset on checkin #8

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

tsnoam
Copy link

@tsnoam tsnoam commented Mar 31, 2019

On certain usage patterns it might be more suitable to reset the values of the encapsulated data during checkin (When Checkout::drop() is executed) instead of during checkout.

@carllerche
Copy link
Owner

Thanks for the proposal. Could you describe the use case that requires reset on check-in? Before adding two options, I would like to evaluate whether reset on check-in should just be the only behavior.

@tsnoam
Copy link
Author

tsnoam commented Apr 3, 2019

The use case goes like this:

Thread 1:

A tight loop handling a massive stream of incoming data. The incoming data is stored on an object taken from the pool.
Only a fraction of the incoming data is sent to an handler thread.

Handle Thread (1 or more):

Performs additional computation on the data. Once its done, the object is dropped (and returned to the pool).

Problem:

Resetting the object in the thread1 (i.e. during checkout) takes time and stalls the tight loop.

Suggested Solution:

Reset the object in the handler thread (i.e. during checkin).

@carllerche
Copy link
Owner

reset logic could also be handled outside of the crate... I wonder if pool should get rid of the Reset trait all together. wdyt?

@tsnoam
Copy link
Author

tsnoam commented Apr 4, 2019

Actually, I find the Reset logic as a really important feature of this library.
It saves a lot of boilerplate code and more importantly avoids "accidents" where the developer forgets to explicitly call reset before a Checkout object is dropped.

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

Successfully merging this pull request may close these issues.

None yet

2 participants