Skip to content

console,process: refactoring stdout/stderr error handling #6174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jasnell wants to merge 4 commits into from
Closed

console,process: refactoring stdout/stderr error handling #6174

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94fc9b5
process: use proper getter property syntax
jasnell Apr 13, 2016
7916d60
process: on error in stdout/stderr, reopen as null
jasnell Apr 13, 2016
6d347ec
test: move known-issues/test-process-external-stdio-close to parallel
jasnell Apr 13, 2016
2ddd056
console: simplify console impl with ES6 mechanisms
jasnell Apr 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
console: simplify console impl with ES6 mechanisms
  • Loading branch information
@jasnell
jasnell committed Apr 13, 2016
commit 2ddd0562b0534519eac99f0917ebec9cb3b1fe7a
25 changes: 10 additions & 15 deletions lib/console.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,25 @@ function Console(stdout, stderr) {
}
}

Console.prototype.log = function() {
this._stdout.write(util.format.apply(null, arguments) + '\n');
Console.prototype.log = function(...args) {
this._stdout.write(`${util.format(...args)}\n`);
Copy link
Copy Markdown
Member

@benjamingr benjamingr Apr 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't ...args still slightly slower than .apply?

Not that it would probably be measurable given log.

Copy link
Copy Markdown
Contributor

@mscdex mscdex Apr 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW v8 5.x supposedly has improved this significantly (according to the v8 blog), but we will have to test and see how it compares there.

Also, the other day I happened to notice that backtick strings are much slower to create than typical concatenation. I'm not sure what, if any, impact it has on GC though.

Copy link
Copy Markdown
Member Author

@jasnell jasnell Apr 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, as indicated, much of this change is speculative. The code cleanups
make a few assumptions about what v8 v5 should be doing as far as
optimizations but those assumptions have yet to be tested. Those specific
changes can be pulled back out of this if necessary tho.

Copy link
Copy Markdown
Member Author

@jasnell jasnell Apr 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've separated the more general modifications to console out to a separate PR #6233

};


Console.prototype.info = Console.prototype.log;


Console.prototype.warn = function() {
this._stderr.write(util.format.apply(null, arguments) + '\n');
Console.prototype.warn = function(...args) {
this._stderr.write(`${util.format(...args)}\n`);
};


Console.prototype.error = Console.prototype.warn;


Console.prototype.dir = function(object, options) {
this._stdout.write(util.inspect(object, util._extend({
customInspect: false
}, options)) + '\n');
options = Object.assign({customInspect: false}, options);
this._stdout.write(`${util.inspect(object, options)}\n`);
};


Expand All @@ -75,24 +74,20 @@ Console.prototype.timeEnd = function(label) {
};


Console.prototype.trace = function trace() {
Console.prototype.trace = function trace(...args) {
// TODO probably can to do this better with V8's debug object once that is
// exposed.
var err = new Error();
err.name = 'Trace';
err.message = util.format.apply(null, arguments);
err.message = util.format(...args);
Error.captureStackTrace(err, trace);
this.error(err.stack);
};


Console.prototype.assert = function(expression) {
Console.prototype.assert = function(expression, ...args) {
if (!expression) {
const argsLen = arguments.length || 1;
const arr = new Array(argsLen - 1);
for (var i = 1; i < argsLen; i++)
arr[i - 1] = arguments[i];
require('assert').ok(false, util.format.apply(null, arr));
require('assert').ok(false, util.format(...args));
}
};

Expand Down