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

channel::tick() behavior questions #406

Closed
theof opened this issue Jul 30, 2019 · 3 comments · Fixed by #456
Closed

channel::tick() behavior questions #406

theof opened this issue Jul 30, 2019 · 3 comments · Fixed by #456

Comments

@theof
Copy link

theof commented Jul 30, 2019

Hello !
When using channel::tick::recv() I expected it to compute its time to sleep with some kind of Instant saved at the time of the Channel instantiation.
Instead the Instant used is the one taken from the last message received, which has two effects :

  • it guarantees that the specified duration at least elapses between two messages received
  • it leads to a small shift from the initial Channel instantiation Instant, mostly due to the job scheduler not waking up the thread necessarily on time

If this was intended originally, then some documentation about the second effect is necessary.
If this is a bug, I would be happy to send a PR. However we must keep in mind that some users might rely on the first effect above.

Also, I think the channel::tick::recv should not have to loop around a thread::sleep as the latter already guarantees to sleep at least the provided duration, which leaves room for simplifying this file a lot.

@drrlvn
Copy link

drrlvn commented Aug 31, 2019

Until I read this issue I had the same expectation of tick() as you did but after reading the code I think you're right.

IMO The time it takes to handle a message should not cause the time the next one is delivered to be shifted.

@theof
Copy link
Author

theof commented Sep 23, 2019

Maybe we should bring that discussion to https://github.com/crossbeam-rs/rfcs ?

@ghost ghost mentioned this issue Dec 24, 2019
@ghost
Copy link

ghost commented Dec 24, 2019

The next message delivery time is always max(last_delivery, now) + duration.

You're right that there is some unnecessary drift due to the way recv() is implemented. I submitted a fix in #456.

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

Successfully merging a pull request may close this issue.

2 participants