Best accuracy and performance with timers in JS

Question

Regular timers in JavaScript (setInteval & setTimeout) have two problems:

1. They aren't as much accuracy as you can get in JS. If you set it like this:

   setInterval(fn, 1000);
   

   You can't be sure that it will be called every 1000 ms. It'll be called 1000 + 1-10ms or even more if some heavy tasks were before the tick.

   This is normal behaviour. The bad thing about it is that these delays will accumulate.

2. Throttling when tab is inactive. When you switch tabs for a while, timers that are in a invisible tab will tick at a higher frequency because of CPU saving. It's a reasonable thing, but killer for multiplayer gaming.

Both problems can be solved by webworker and some variables. That's why I've built a class for it and need a review.

My class also provide anti-blocking mechanism. When there was heavy tasks before timer tick and we need to deal with lag - timer will make up all lost ticks but it can be "forever" loop. To prevent this you can set the max ms for "making lost ticks" and after them will be a break for other code/events/etc and back to work.

module.exports = UberTimer = function UberTimer(callback, interval, blockThreadMaxMs) {
  this.callback = callback;
  this.interval = interval;

  this.lastTickTime = -1;

  this.blocking = {};
  this.blocking.maxTime = blockThreadMaxMs;
  this.blocking.startTime = -1;
}

////////////////////////////////////////////////////////////////////////

UberTimer.worker = null;
UberTimer.instances = [];

UberTimer.__startWorker = function() {
  if ( UberTimer.worker ) return;

  var blobURL = URL.createObjectURL( new Blob([ '(', (

    function() {
      setInterval(() => self.postMessage(0), 1);
    }

  ).toString(), ')()' ], { type: 'application/javascript' } ) );

  UberTimer.worker = new Worker(blobURL);
  UberTimer.worker.onmessage = UberTimer.__tick;
}

UberTimer.__stopWorker = function() {
  UberTimer.worker.terminate();
  UberTimer.worker = null;
}

UberTimer.__tick = function(timer) {
  for ( var timer of UberTimer.instances )
    timer.__workerTick();
}

UberTimer.create = function(callback, interval, blockThreadMaxMs=100) {
  var timer = new UberTimer(callback, interval, blockThreadMaxMs);

  UberTimer.instances.push(timer);
  UberTimer.__startWorker();

  return timer;
}

UberTimer.release = function(timer) {
  var i = UberTimer.instances.indexOf(timer)
  if ( i ) UberTimer.instances.splice(i, 1);

  if ( UberTimer.instances.length == 0 )
    UberTimer.__stopWorker();
}

////////////////////////////////////////////////////////////////////////

UberTimer.prototype.__makeCall = function() {
  this.callback();
  this.lastTickTime += this.interval;
}

UberTimer.prototype.__workerTick = function() {
  if ( this.lastTickTime == -1 ) return;

  var now = performance.now();

  if ( this.blocking.startTime == -1 )
    this.blocking.startTime = now;

  var elapsedTime = now - this.lastTickTime;

  if ( elapsedTime >= this.interval ) {
    this.__makeCall();

    var blockingTime = now - this.blocking.startTime;

    if ( blockingTime < this.blocking.maxTime )
      this.__workerTick();
    else
      this.blocking.startTime = -1;
  }
}

UberTimer.prototype.start = function() {
  this.__makeCall();
  this.lastTickTime = performance.now();
}

UberTimer.prototype.stop = function() {
  this.lastTickTime = -1;
}

Usage:

var timer = UberTimer.create(callback, intevalInMs, optional: maxBlockingTime);

timer.start();
timer.stop();

UberTimer.release(timer);

Answer 1

BUGs

First some logic and flow bugs.

1. The function release uses indexOf to get the timer index i then you use the statement if(i) { ... which fails to remove the first item in the instances array. The result is the worker is never shut down.

2. Calling UberTimer.stop(timer) from within the callback function timer.callback fails to stop the timer, the callback continues to be called. Because you set lastTickTime to -1 your code then tries to catch up by calling the callback at the full rate of the worker's messages.

And related to how you call the callback function.

3. If the callback function timer.callback fails to return (throws an error) on the function timer.start the timer.lastTickTime property is not set and remains the init value of -1. The callback is then call in rapid succession, and if the callback continues the throw it just continues tring to catch up every time the worker sends a message. The is in effect an infinite message incomplete loop

4. Because you call the callback function from the function timer.start any error in the callback will stop current execution context. This is not what one would expect from a timer.

5. When you have many timers. If any callback throws the remaining callbacks (higher on the instance array) will not get called as the UberTimer.__tick does not get to complete the timer iteration loop.

Danger potencial silent bug.

Because the property UberTimer.worker is exposed there is a danger that you loose the reference to the worker, if it is accidentally or deliberately overwritten. This could result in a silent bug that makes its way to release degrading performance. You should make it safer (protect your state).

Some style notes

• Underscore prefixes do not create private / protected variables. If you wish to protect your object's state don't expose functions and properties that you don't want messed with.

• C like languages allow the lazy programmers to save milliseconds typing by not including {...} for single line statement blocks. The irony is that the cost is hours of bug hunting. Your eyes don't look for { and } when you think you are looking for a logic bug.

  For your coding sanity ALWAYS delimit blocks with {...}

if(foo === blah) 
    foo = 1;
    
 // then later you make the change
 
 
 if(foo === blah)
    foo = 1;
    blah = 1;
    
// Yes obvious in isolation. But in 100's of lines of code with many unblocked statements
// the eye sees the indent and the mind puts in the {}

// never an issue if you always do
if (foo === blah) { foo = 1 }
// or 
if (foo === blah) { 
    foo = 1;
}

• Use === or !== never use == or !=

• For variables that never change declare them with const

• Don't define a variable in one type only to convert it to another when you use it. The worker function can be defined as a constant string.

• Don't add redundant or superfluous code it only reduces the quality of the code. module.exports = UberTimer = function UberTimer( reassigning the UberTimer serves no purpose in your code.

 this.blocking = {};
 this.blocking.maxTime = blockThreadMaxMs;
 this.blocking.startTime = -1;

 // should be

 this.blocking = {
     maxTime : blockThreadMaxMs,
     startTime : -1,
 };

• Use object function shorthand if you can.

UberTimer.prototype.__makeCall = function() { ... }

UberTimer.prototype.__workerTick = function() { ... }

UberTimer.prototype.start = function() { ... }

UberTimer.prototype.stop = function() { ... }

Is better as

UberTimer.prototype = {
     __makeCall() { ... },
    __workerTick() { ... },
    start() { ... },
    stop() { ... },
};