High resolution timer in Stopwatch #5

Closed
seriousManual opened this Issue Jul 15, 2012 · 6 comments

Projects

None yet

2 participants

@seriousManual

It would be a quite nice feature to use process.hrtime() to have a even more precise time.

Than it would be:

{  
  histogram:
  { min: 0.45434,
  ...
  }
}

instead of:

{  
  histogram:
  { min: 0,
  ...
  }
}

Unfortunately sinon does not support the mocking of process.hrtime, so all tests would fail...

@felixge
Owner

Unfortunately sinon does not support the mocking of process.hrtime, so all tests would fail...

Well, then let's change that. Change the Stopwatch constructor like this:

function Stopwatch(options) {
  options = options || {};

  EventEmitter.call(this);

  this._getTime = options.getTime || process.hrtime || Date.now;
  this._start = this._getTime();
  this._ended = false;
}

This way you can easily pass in a fake getTime() function for testing.

Let me know if you can make the patch for this!

@seriousManual

Ah, I did not think about that solution, I will take care about it!

@seriousManual

hi,
due to the nature of process.hrtime i had to solve the measurement of the elapsed time a bit differently...
(process.hrtime does not provide an absolute time value but a relative one)

i made two different solutions:
https://github.com/zaphod1984/node-measured/blob/highResolution/lib/util/Stopwatch.js
https://github.com/zaphod1984/node-measured/blob/highResolutionAlternative/lib/util/Stopwatch.js

second version should be a little more efficient as getTime doesn't have to be assigned on every invocation.

to me both variations feel a little clumsy though they're working fine, i wouldn't mind if you don't like them. :)
if you do I'll gladly will do a pull request.

@felixge
Owner

Hm, seems overkill. I'm also not a fan of the way you use the ternary operator. What about my initial suggestion, but except for process.hrtime use hrtime() which is then defined as an inline function at the bottom of the module like this:

function hrtime() {
  var hrtime = process.hrtime();
  return hrtime[0] / 1000 + hrtime[1] / (1000 * 1000);
}

Something like that? Should be simpler.

@felixge
Owner

Nevermind, this should be done:

Stopwatch.prototype._getTime = function() {
  if (!process.hrtime) return Date.now();

  var hrtime = process.hrtime();
  return hrtime[0] / 1000 + hrtime[1] / (1000 * 1000);
}

And in the constructor:

function Stopwatch(options) {
  options = options || {};

  EventEmitter.call(this);

  if (options.getTime) this._getTime = options.getTime;
  this._start = this._getTime();
  this._ended = false;
}
@seriousManual

Uh, I obviously made it too complicated...
I did a pull request, I hope it's fine now... :)
#6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment