amazon-chime-sdk-js icon indicating copy to clipboard operation
amazon-chime-sdk-js copied to clipboard

Reexamine use of setInterval and setTimeout

Open richnew10 opened this issue 4 years ago • 1 comments

setInterval queues a new work item each time the interval is hit, even if the first work item hasn't completed or took a large chunk of the time between intervals. That is usually not what you want to happen.

The SDK extensively uses setInterval via IntervalScheduler.

It also makes heavy use of setTimeout. This has its own issues: under load, the queued event can occur substantially later than you expect.

Browsers are also willing to heavily throttle timers and render clocks inaccurate by up to 100msec.

We should take three actions:

  1. Audit uses of IntervalTimer, probably switching them to use chained setTimeout instead of setInterval to achieve a fixed duration between tasks rather than a fixed duration between task starts.
  2. Audit all uses of setTimeout and AsyncScheduler to decide whether the task should abort if it has been extensively delayed before running, or whether the task needs to be rewritten to allow for coalescing of work items via a separate queue or chalkboard.
  3. Test with fingerprint resistance enabled, and fix as many issues as we can.

richnew10 avatar Aug 27 '21 21:08 richnew10

Some code samples that show over/under runs when putting tabs in the background:

  1. setInterval: https://codepen.io/schontz/pen/GREpgmb
  2. setTimeout chained to mimic setInterval: https://codepen.io/schontz/pen/eYRpmgR

When the tabs are in the background (or when the thread is busy), the deltas will grow. In the case of setTimeout they will only grow. But in the case of setInterval, you will see some intervals run considerably shorter as they are in some kind of queue. This results in something like the following when coming back to the setInterval tab:

     112 | Expected:   112000 | Actual:   186158 | Diff:    74158 | Delta:     1808
     113 | Expected:   113000 | Actual:   187936 | Diff:    74936 | Delta:     1778
     114 | Expected:   114000 | Actual:   188687 | Diff:    74687 | Delta:      751  <-- ouch
     115 | Expected:   115000 | Actual:   189006 | Diff:    74006 | Delta:      319  <-- I've seen as low as 2ms

As Richard noted, you can update IntervalTimer to internally use a chained setTimeout, which will fix your under-run issues.

Any code that is based on timeouts and/or ticks may do well to pass the delta to the callbacks so the callbacks can monitor their "FPS" and adapt/abort accordingly.

schontz avatar Aug 27 '21 22:08 schontz