From a647d82f83ad5ddad5db7be2cc24c3d686121792 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Thu, 12 Jan 2017 20:03:29 +0800
Subject: [PATCH] process: improve process.hrtime

* Add benchmarks for diffing a previous result
* Improvements to the documentation, including type annotation
* Update the outdated comments in src/node.cc, improve comments
  in lib/internal/process.js
* Check the argument is an Array Tuple with length 2

PR-URL: https://github.com/nodejs/node/pull/10764
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Brian White <mscdex@mscdex.net>
---
 benchmark/process/bench-hrtime.js    | 28 +++++++++++++++++++-------
 doc/api/process.md                   | 30 ++++++++++++++++------------
 lib/internal/process.js              | 18 +++++++++--------
 src/node.cc                          | 12 +++++------
 test/parallel/test-process-hrtime.js | 15 ++++++++++----
 5 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/benchmark/process/bench-hrtime.js b/benchmark/process/bench-hrtime.js
index 661dff43b0..8a2920a238 100644
--- a/benchmark/process/bench-hrtime.js
+++ b/benchmark/process/bench-hrtime.js
@@ -1,18 +1,32 @@
 'use strict';
 
 const common = require('../common');
+const assert = require('assert');
 
 const bench = common.createBenchmark(main, {
-  n: [1e6]
+  n: [1e6],
+  type: ['raw', 'diff']
 });
 
-
 function main(conf) {
-  const n = conf.n >>> 0;
+  const n = conf.n | 0;
+  const hrtime = process.hrtime;
+  var noDead = hrtime();
+  var i;
 
-  bench.start();
-  for (var i = 0; i < n; i++) {
-    process.hrtime();
+  if (conf.type === 'raw') {
+    bench.start();
+    for (i = 0; i < n; i++) {
+      noDead = hrtime();
+    }
+    bench.end(n);
+  } else {
+    bench.start();
+    for (i = 0; i < n; i++) {
+      noDead = hrtime(noDead);
+    }
+    bench.end(n);
   }
-  bench.end(n);
+
+  assert.ok(Array.isArray(noDead));
 }
diff --git a/doc/api/process.md b/doc/api/process.md
index 4e20399d41..cd16dec540 100644
--- a/doc/api/process.md
+++ b/doc/api/process.md
@@ -1016,18 +1016,25 @@ Android)
 added: v0.7.6
 -->
 
-The `process.hrtime()` method returns the current high-resolution real time in a
-`[seconds, nanoseconds]` tuple Array. `time` is an optional parameter that must
-be the result of a previous `process.hrtime()` call (and therefore, a real time
-in a `[seconds, nanoseconds]` tuple Array containing a previous time) to diff
-with the current time. These times are relative to an arbitrary time in the
-past, and not related to the time of day and therefore not subject to clock
-drift. The primary use is for measuring performance between intervals.
+* `time` {Array} The result of a previous call to `process.hrtime()`
+* Returns: {Array}
+
+The `process.hrtime()` method returns the current high-resolution real time
+in a `[seconds, nanoseconds]` tuple Array, where `nanoseconds` is the
+remaining part of the real time that can't be represented in second precision.
 
-Passing in the result of a previous call to `process.hrtime()` is useful for
-calculating an amount of time passed between calls:
+`time` is an optional parameter that must be the result of a previous
+`process.hrtime()` call to diff with the current time. If the parameter
+passed in is not a tuple Array, a `TypeError` will be thrown. Passing in a
+user-defined array instead of the result of a previous call to
+`process.hrtime()` will lead to undefined behavior.
+
+These times are relative to an arbitrary time in the
+past, and not related to the time of day and therefore not subject to clock
+drift. The primary use is for measuring performance between intervals:
 
 ```js
+const NS_PER_SEC = 1e9;
 var time = process.hrtime();
 // [ 1800216, 25 ]
 
@@ -1035,14 +1042,11 @@ setTimeout(() => {
   var diff = process.hrtime(time);
   // [ 1, 552 ]
 
-  console.log(`Benchmark took ${diff[0] * 1e9 + diff[1]} nanoseconds`);
+  console.log(`Benchmark took ${diff[0] * NS_PER_SEC + diff[1]} nanoseconds`);
   // benchmark took 1000000527 nanoseconds
 }, 1000);
 ```
 
-Constructing an array by some method other than calling `process.hrtime()` and
-passing the result to process.hrtime() will result in undefined behavior.
-
 
 ## process.initgroups(user, extra_group)
 <!-- YAML
diff --git a/lib/internal/process.js b/lib/internal/process.js
index 441fb722ed..44f1370459 100644
--- a/lib/internal/process.js
+++ b/lib/internal/process.js
@@ -73,19 +73,22 @@ function setup_cpuUsage() {
   };
 }
 
-
+// The 3 entries filled in by the original process.hrtime contains
+// the upper/lower 32 bits of the second part of the value,
+// and the renamining nanoseconds of the value.
 function setup_hrtime() {
   const _hrtime = process.hrtime;
   const hrValues = new Uint32Array(3);
 
-  process.hrtime = function hrtime(ar) {
+  process.hrtime = function hrtime(time) {
     _hrtime(hrValues);
 
-    if (typeof ar !== 'undefined') {
-      if (Array.isArray(ar)) {
-        const sec = (hrValues[0] * 0x100000000 + hrValues[1]) - ar[0];
-        const nsec = hrValues[2] - ar[1];
-        return [nsec < 0 ? sec - 1 : sec, nsec < 0 ? nsec + 1e9 : nsec];
+    if (time !== undefined) {
+      if (Array.isArray(time) && time.length === 2) {
+        const sec = (hrValues[0] * 0x100000000 + hrValues[1]) - time[0];
+        const nsec = hrValues[2] - time[1];
+        const needsBorrow = nsec < 0;
+        return [needsBorrow ? sec - 1 : sec, needsBorrow ? nsec + 1e9 : nsec];
       }
 
       throw new TypeError('process.hrtime() only accepts an Array tuple');
@@ -98,7 +101,6 @@ function setup_hrtime() {
   };
 }
 
-
 function setupConfig(_source) {
   // NativeModule._source
   // used for `process.config`, but not a real module
diff --git a/src/node.cc b/src/node.cc
index b6dd9b59d5..721c4d097a 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -2274,18 +2274,18 @@ void Kill(const FunctionCallbackInfo<Value>& args) {
 
 // Hrtime exposes libuv's uv_hrtime() high-resolution timer.
 // The value returned by uv_hrtime() is a 64-bit int representing nanoseconds,
-// so this function instead returns an Array with 2 entries representing seconds
-// and nanoseconds, to avoid any integer overflow possibility.
-// Pass in an Array from a previous hrtime() call to instead get a time diff.
+// so this function instead fills in an Uint32Array with 3 entries,
+// to avoid any integer overflow possibility.
+// The first two entries contain the second part of the value
+// broken into the upper/lower 32 bits to be converted back in JS,
+// because there is no Uint64Array in JS.
+// The third entry contains the remaining nanosecond part of the value.
 void Hrtime(const FunctionCallbackInfo<Value>& args) {
   uint64_t t = uv_hrtime();
 
   Local<ArrayBuffer> ab = args[0].As<Uint32Array>()->Buffer();
   uint32_t* fields = static_cast<uint32_t*>(ab->GetContents().Data());
 
-  // These three indices will contain the values for the hrtime tuple. The
-  // seconds value is broken into the upper/lower 32 bits and stored in two
-  // uint32 fields to be converted back in JS.
   fields[0] = (t / NANOS_PER_SEC) >> 32;
   fields[1] = (t / NANOS_PER_SEC) & 0xffffffff;
   fields[2] = t % NANOS_PER_SEC;
diff --git a/test/parallel/test-process-hrtime.js b/test/parallel/test-process-hrtime.js
index faa598d0a5..db8700d3e6 100644
--- a/test/parallel/test-process-hrtime.js
+++ b/test/parallel/test-process-hrtime.js
@@ -15,14 +15,21 @@ validateTuple(process.hrtime(tuple));
 assert.throws(() => {
   process.hrtime(1);
 }, /^TypeError: process.hrtime\(\) only accepts an Array tuple$/);
+assert.throws(() => {
+  process.hrtime([]);
+}, /^TypeError: process.hrtime\(\) only accepts an Array tuple$/);
+assert.throws(() => {
+  process.hrtime([1]);
+}, /^TypeError: process.hrtime\(\) only accepts an Array tuple$/);
+assert.throws(() => {
+  process.hrtime([1, 2, 3]);
+}, /^TypeError: process.hrtime\(\) only accepts an Array tuple$/);
 
 function validateTuple(tuple) {
   assert(Array.isArray(tuple));
   assert.strictEqual(tuple.length, 2);
-  tuple.forEach((v) => {
-    assert.strictEqual(typeof v, 'number');
-    assert.strictEqual(isFinite(v), true);
-  });
+  assert(Number.isInteger(tuple[0]));
+  assert(Number.isInteger(tuple[1]));
 }
 
 const diff = process.hrtime([0, 1e9 - 1]);