console: implement timeLog method #21312

targos · 2018-06-13T13:41:06Z

Marking blocked until the spec PR lands.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2018-06-13T13:41:07Z

@targos build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/50/pipeline

cjihrig · 2018-06-13T14:11:37Z

lib/console.js

@@ -234,6 +234,19 @@ Console.prototype.time = function time(label = 'default') {
  this._times.set(label, process.hrtime());
 };

+Console.prototype.timeLog = function timeLog(label = 'default', ...data) {


Other than the function name printed in the process warning, couldn't Console.prototype.timeEnd() be updated to call this?

apapirovski

Barring spec changes, this looks great.

vsemozhetbyt · 2018-06-13T20:10:09Z

doc/api/console.md

+* `label` {string} **Default:** `'default'`
+* `...data` {any}
+
+For a timer that was previously started by calling [`console.time()`)][], prints


Extra parenthesis in [`console.time()`)][].

vsemozhetbyt · 2018-06-13T20:11:33Z

doc/api/console.md

+
+```js
+console.time('process');
+const value = expensiveProcess1();


Maybe add a comment that value is 42 here? Otherwise, it is a bit unexpected below.

vsemozhetbyt · 2018-06-13T20:13:25Z

doc/api/console.md

+console.time('process');
+const value = expensiveProcess1();
+console.timeLog('process', value);
+// Prints process: 365.227ms 42


Seems a bit confusing. Maybe something like // Prints "process: 365.227ms 42".?

vsemozhetbyt · 2018-06-13T20:15:50Z

Should we document the separator that joins several `...data` values?

vsemozhetbyt · 2018-06-13T20:20:39Z

doc/api/console.md

@@ -392,6 +392,25 @@ are identified by a unique `label`. Use the same `label` when calling
 [`console.timeEnd()`][] to stop the timer and output the elapsed time in
 milliseconds to `stdout`. Timer durations are accurate to the sub-millisecond.

+### console.timeLog([label][, ...data])


It seems this should go after ### console.timeEnd(label) section ABC-wise.

No strong opinion but this document doesn't seem to follow the ABC rule and I went for the spec order. It's also the order in which you call the methods

All console. methods seem to be in ABC order according to the TOC.

Right. I didn't see that inspector methods were in another section

targos · 2018-06-16T17:14:35Z

@cjihrig I refactored the common code in a separate function. PTAL.

@vsemozhetbyt

Should we document the separator that joins several ...data values?

I don't know. It's just using console.log()'s mechanics. Perhaps we should document that? I don't know how to write it though.

vsemozhetbyt · 2018-06-16T17:48:17Z

@targos Then we can land as is and if anybody is sure how to document this (if needed), this can be done in another PR)

BridgeAR · 2018-06-18T11:11:47Z

doc/api/console.md

+console.timeLog('process', value);
+// Prints "process: 365.227ms 42".
+doExpensiveProcess2(value);
+console.timeEnd('process');


This will print a warning about no process label, right?

It's not supposed to. timeLog shouldn't clear the timer.

BridgeAR · 2018-06-18T11:18:36Z

lib/console.js

-  this.log('%s: %sms', label, ms.toFixed(3));
-  this._times.delete(label);
-};
+  this.log('%s: %sms', label, ms.toFixed(3), ...data);


Nit: I personally would prefer to just check if data is undefined by adding a if / else but as the function is not hot, that is just a non-blocking recommendation.

I can do that 👍

apapirovski · 2018-06-25T07:00:19Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15599/

jasnell · 2018-06-29T20:39:44Z

Looks like this failed across the board.

Refs: whatwg/console#138

targos · 2018-06-30T12:59:04Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15686/

targos · 2018-07-04T06:26:24Z

Upstream PR landed: https://console.spec.whatwg.org/#timelog

targos · 2018-07-04T19:16:22Z

CI before landing: https://ci.nodejs.org/job/node-test-pull-request/15728/

targos · 2018-07-04T20:14:51Z

Landed in b016745

Refs: whatwg/console#138 PR-URL: #21312 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Weijia Wang <starkwang@126.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477)

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477) * Added new collaborators * codebytere - Shelley Vohr PR-URL: #21851

Notable changes: * console: * The `console.timeLog()` method has been implemented. (#21312) * deps: * Upgrade to libuv 1.22.0. (#21731) * Upgrade to ICU 62.1 (Unicode 11, CLDR 33.1). (#21728) * http: * Added support for passing both `timeout` and `agent` options to `http.request`. (#21204) * inspector: * Expose the original console API in `require('inspector').console`. (#21659) * napi: * Added experimental support for functions dealing with bigint numbers. (#21226) * process: * The `process.hrtime.bigint()` method has been implemented. (#21256) * Added the `--title` command line argument to set the process title on startup. (#21477) * trace_events: * Added process_name metadata. (#21477) * Added new collaborators * codebytere - Shelley Vohr PR-URL: #21851

targos added wip Issues and PRs that are still a work in progress. semver-minor PRs that contain new features and should be released in the next minor version. console Issues and PRs related to the console subsystem. labels Jun 13, 2018

nodejs-github-bot added the console Issues and PRs related to the console subsystem. label Jun 13, 2018

targos added blocked PRs that are blocked by other issues or PRs. and removed wip Issues and PRs that are still a work in progress. labels Jun 13, 2018

cjihrig approved these changes Jun 13, 2018

View reviewed changes

devsnek approved these changes Jun 13, 2018

View reviewed changes

apapirovski approved these changes Jun 13, 2018

View reviewed changes

vsemozhetbyt reviewed Jun 13, 2018

View reviewed changes

starkwang approved these changes Jun 14, 2018

View reviewed changes

targos force-pushed the console-timelog branch from e6f1714 to 73c32ca Compare June 16, 2018 16:52

BridgeAR approved these changes Jun 18, 2018

View reviewed changes

jasnell approved these changes Jun 21, 2018

View reviewed changes

targos force-pushed the console-timelog branch from 63bd821 to 67279d0 Compare June 30, 2018 12:41

console: implement timeLog method

32bae6b

Refs: whatwg/console#138

targos force-pushed the console-timelog branch from 67279d0 to 32bae6b Compare June 30, 2018 12:50

domfarolino mentioned this pull request Jul 4, 2018

console: console.countReset() should emit warning #21649

Closed

4 tasks

targos removed the blocked PRs that are blocked by other issues or PRs. label Jul 4, 2018

targos closed this Jul 4, 2018

targos deleted the console-timelog branch July 4, 2018 20:14

targos mentioned this pull request Jul 17, 2018

v10.7.0 proposal #21851

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

console: implement timeLog method #21312

console: implement timeLog method #21312

targos commented Jun 13, 2018 •

edited

Loading

nodejs-github-bot commented Jun 13, 2018

cjihrig Jun 13, 2018

apapirovski left a comment

vsemozhetbyt Jun 13, 2018

vsemozhetbyt Jun 13, 2018

vsemozhetbyt Jun 13, 2018

vsemozhetbyt commented Jun 13, 2018 •

edited

Loading

vsemozhetbyt Jun 13, 2018

targos Jun 13, 2018

vsemozhetbyt Jun 13, 2018

targos Jun 14, 2018

targos commented Jun 16, 2018

vsemozhetbyt commented Jun 16, 2018

BridgeAR Jun 18, 2018

targos Jun 18, 2018 •

edited

Loading

BridgeAR Jun 18, 2018

targos Jun 18, 2018

apapirovski commented Jun 25, 2018

jasnell commented Jun 29, 2018

targos commented Jun 30, 2018

targos commented Jul 4, 2018

targos commented Jul 4, 2018

targos commented Jul 4, 2018

console: implement timeLog method #21312

console: implement timeLog method #21312

Conversation

targos commented Jun 13, 2018 • edited Loading

Checklist

nodejs-github-bot commented Jun 13, 2018

Choose a reason for hiding this comment

apapirovski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsemozhetbyt commented Jun 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

targos commented Jun 16, 2018

vsemozhetbyt commented Jun 16, 2018

Choose a reason for hiding this comment

targos Jun 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apapirovski commented Jun 25, 2018

jasnell commented Jun 29, 2018

targos commented Jun 30, 2018

targos commented Jul 4, 2018

targos commented Jul 4, 2018

targos commented Jul 4, 2018

targos commented Jun 13, 2018 •

edited

Loading

vsemozhetbyt commented Jun 13, 2018 •

edited

Loading

targos Jun 18, 2018 •

edited

Loading