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

Use PeriodicTimer in the async timer #4004

Closed
wants to merge 2 commits into from
Closed

Conversation

danielmarbach
Copy link
Contributor

This switches the internals of AsyncTimer to using PeriodicTimer which is built-in. To support the due time and to avoid having to change the period of the timer it first delays for the due time and then waits until the period fires. The previous implementation did essentially prolong the execution of the next interval by the time it took the callback to execute. This implementation changes the behavior slightly by checking if the next tick is available, which might already be. If multiple periods are over, those get coalesced into one execution.

The PeriodicTimer behaves like an auto-reset event, in that multiple ticks are coalesced into a single tick if they occur between calls to WaitForNextTickAsync(CancellationToken).

Do you see any issues with this potentially firing a bit earlier? It would be nice to be able to rely on the provided period timer.

@danielmarbach
Copy link
Contributor Author

Discussed it today during the sync and we have decided that the initial behavior of prolonging the execution by the time it takes to execute the callback is desirable because we are running potentially high intensity IO bound operations.

@danielmarbach danielmarbach deleted the periodic-timer branch March 13, 2024 15:08
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

1 participant