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

Prevent drift in tick delivery (fix #406) #413

Closed
wants to merge 3 commits into from
Closed

Prevent drift in tick delivery (fix #406) #413

wants to merge 3 commits into from

Conversation

drrlvn
Copy link

@drrlvn drrlvn commented Sep 1, 2019

Calculate the next delivery time based on the previous one instead of the time the thread awoke, preventing a small but noticeable drift from accumulating.

Calculate the next delivery time based on the previous one instead of the time the thread awoke, preventing a small but noticeable drift from accumulating.
@drrlvn
Copy link
Author

drrlvn commented Sep 1, 2019

I fixed the tests so they now pass. I don't know why the CI fails on 1.26/1.28, maybe it's a dependency issue. This is the error:

error[E0432]: unresolved import `self::std::hint`

  --> /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/lazy_static-1.4.0/src/inline_lazy.rs:13:16

   |

13 | use self::std::hint::unreachable_unchecked;

   |                ^^^^ Could not find `hint` in `std`

error: aborting due to previous error

@drrlvn
Copy link
Author

drrlvn commented Sep 1, 2019

I don't know why the CI fails on 1.26/1.28, maybe it's a dependency issue.

Happens on master as well, see #412.

@ghost
Copy link

ghost commented Dec 24, 2019

Hey, thank you for submitting this PR and apologies for coming to it so late! :( I'm assuming you already forgot about this code so I submitted a new PR: #456

I think try_recv() should not change - the drift there is intended and this is exactly how tickers work in Go, too. However, we do need changes in recv() - instead of sleeping and then attempting to receive a message, we should receive it first and then sleep.

@drrlvn
Copy link
Author

drrlvn commented Dec 25, 2019

@stjepang OK, great, I'll close this PR then. Thanks!

@drrlvn drrlvn closed this Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant