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

Add correctDrift option to Stream.periodic (and Timer.periodic?) #18642

Open
DartBot opened this issue May 5, 2014 · 5 comments
Open

Add correctDrift option to Stream.periodic (and Timer.periodic?) #18642

DartBot opened this issue May 5, 2014 · 5 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2 library-async P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented May 5, 2014

This issue was originally filed by sophielight...@gmail.com


Periodic timers are subject to compounding deviation from the originally scheduled intervals over time. It would be useful to have a correctDrift or similar option to Stream.periodic which compensates for drift when scheduling the next timer. Thus it would not use Timer.periodic when correctDrift is true. If the scheduled time is already past, then the timer is scheduled immediately (using Duration.ZERO or Timer.run).

It would be useful for Timer.periodic too, but the createTimerPeriodic Zone callback signature would need to be updated, which would be a breaking change unless it just accepts callbacks with or without the correctDrift option.

@sgjesse
Copy link
Contributor

sgjesse commented May 6, 2014

Added Area-Library, Library-Async, Triaged labels.

@sgjesse
Copy link
Contributor

sgjesse commented May 6, 2014

Removed Type-Defect, Priority-Unassigned labels.
Added Type-Enhancement, Priority-Medium labels.

@DartBot
Copy link
Author

DartBot commented May 9, 2014

This comment was originally written by @seaneagan


Looks like there is a more canonical terminology for this:

http://en.wikipedia.org/wiki/Isochronous

i.e. correctDrift -> isochronous

or could add a Stream.isochronous constructor

@DartBot
Copy link
Author

DartBot commented Nov 22, 2014

This comment was originally written by @jtmcdole


I used Isochronous when adding to Quiver, but no one liked it, hence Metronome:
https://github.com/google/quiver-dart/blob/master/lib/src/async/metronome.dart

@lrhn
Copy link
Member

lrhn commented Nov 22, 2014

The VM periodic timer is actually attempting to be isochronous.

A problem with timers in any single-threaded system is that the execution can be delayed if something else is already running or scheduled in the event queue when the timer triggers.
The timer event is usually just run as soon as possible.

A problem with an isochronous timer is what it should do if the previous task plus its delay takes more time than the interval. Or if accumulated delay adds up to more than one time the interval: should two events then be run immediately after each other.
It means that the delay between two consecutive timer triggers can can be anything from 0ms to infinity.

The JavaScript setInterval timers are defined to only start a new interval when the previous one is done executing. That means that the time between two timer triggers is always greater than or equal to the interval - which is actually more promise than the isochronous timer can give. However, it will almost certainly accumulate clock skew, even if it has perfect timers, because it's delayed by its own execution time. Any further delay from already executing events will just add to that.
That's the timer that dart2js code is using, since it's what they have.

Timer.periodic is described only loosely because it has to cover both behaviors.
All we promise is that you get at most n events in n*duration time. You can get less.

@DartBot DartBot added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async labels Nov 22, 2014
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed triaged labels Mar 1, 2016
@lrhn lrhn added the core-m label Aug 11, 2017
@floitschG floitschG added core-2 and removed core-m labels Aug 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2 library-async P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants