From 4c05b6d01adde1614c77071105a24feba8a7208e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 21 Nov 2018 07:31:25 +0800 Subject: [PATCH 1/3] console: lazy load process.stderr and process.stdout This patch: - Refactors the Console constructor: moves the property binding code into and the writable streams binding code into two methods defined on the Console.prototype with symbols. - Refactors the global console creation: we only need to share the property binding code from the Console constructor. To bind the streams we can lazy load `process.stdio` and `process.stderr` so that we don't create these streams when they are not used. This significantly reduces the number of modules loaded during bootstrap. Also, by calling the refactored-out method directly we can skip the unnecessary typechecks when creating the global console and there is no need to create a temporary Console anymore. - Refactors the error handler creation and the `write` method: use a `kUseStdout` symbol to tell the internals which stream should be loaded from the console instance. Also put the `write` method on the Console prototype so it just loads other properties directly off the console instance which simplifies the call sites. Also leaves a few TODOs for further refactoring of the console bootstrap. --- lib/console.js | 197 +++++++++++------- test/parallel/test-bootstrap-modules.js | 2 +- .../test-stderr-stdout-handle-sigwinch.out | 2 +- 3 files changed, 124 insertions(+), 77 deletions(-) diff --git a/lib/console.js b/lib/console.js index b0f3055e91fb62..ebd098e9affdff 100644 --- a/lib/console.js +++ b/lib/console.js @@ -65,7 +65,15 @@ const kFormatForStdout = Symbol('kFormatForStdout'); const kGetInspectOptions = Symbol('kGetInspectOptions'); const kColorMode = Symbol('kColorMode'); const kIsConsole = Symbol('kIsConsole'); - +const kWriteToConsole = Symbol('kWriteToConsole'); +const kBindProperties = Symbol('kBindProperties'); +const kBindStreamsEager = Symbol('kBindStreamsEager'); +const kBindStreamsLazy = Symbol('kBindStreamsLazy'); +const kUseStdout = Symbol('kUseStdout'); +const kUseStderr = Symbol('kUseStderr'); + +// This constructor is not used to construct the global console. +// It's exported for backwards compatibility. function Console(options /* or: stdout, stderr, ignoreErrors = true */) { // We have to test new.target here to see if this function is called // with new, because we need to define a custom instanceof to accommodate @@ -74,7 +82,6 @@ function Console(options /* or: stdout, stderr, ignoreErrors = true */) { return new Console(...arguments); } - this[kIsConsole] = true; if (!options || typeof options.write === 'function') { options = { stdout: options, @@ -97,37 +104,9 @@ function Console(options /* or: stdout, stderr, ignoreErrors = true */) { throw new ERR_CONSOLE_WRITABLE_STREAM('stderr'); } - const prop = { - writable: true, - enumerable: false, - configurable: true - }; - Object.defineProperty(this, '_stdout', { ...prop, value: stdout }); - Object.defineProperty(this, '_stderr', { ...prop, value: stderr }); - Object.defineProperty(this, '_ignoreErrors', { - ...prop, - value: Boolean(ignoreErrors), - }); - Object.defineProperty(this, '_times', { ...prop, value: new Map() }); - Object.defineProperty(this, '_stdoutErrorHandler', { - ...prop, - value: createWriteErrorHandler(stdout), - }); - Object.defineProperty(this, '_stderrErrorHandler', { - ...prop, - value: createWriteErrorHandler(stderr), - }); - if (typeof colorMode !== 'boolean' && colorMode !== 'auto') throw new ERR_INVALID_ARG_VALUE('colorMode', colorMode); - // Corresponds to https://console.spec.whatwg.org/#count-map - this[kCounts] = new Map(); - this[kColorMode] = colorMode; - - Object.defineProperty(this, kGroupIndent, { writable: true }); - this[kGroupIndent] = ''; - // bind the prototype functions to this Console instance var keys = Object.keys(Console.prototype); for (var v = 0; v < keys.length; v++) { @@ -137,14 +116,92 @@ function Console(options /* or: stdout, stderr, ignoreErrors = true */) { // from the prototype chain of the subclass. this[k] = this[k].bind(this); } + + this[kBindStreamsEager](stdout, stderr); + this[kBindProperties](ignoreErrors, colorMode); } +const consolePropAttributes = { + writable: true, + enumerable: false, + configurable: true +}; + +// Fixup global.console instanceof global.console.Console +Object.defineProperty(Console, Symbol.hasInstance, { + value(instance) { + return instance[kIsConsole]; + } +}); + +// Eager version for the Console constructor +Console.prototype[kBindStreamsEager] = function(stdout, stderr) { + Object.defineProperties(this, { + '_stdout': { ...consolePropAttributes, value: stdout }, + '_stderr': { ...consolePropAttributes, value: stderr } + }); +}; + +// Lazily load the stdout and stderr from an object so we don't +// create the stdio streams when they are not even accessed +Console.prototype[kBindStreamsLazy] = function(object) { + let stdout; + let stderr; + Object.defineProperties(this, { + '_stdout': { + enumerable: false, + configurable: true, + get() { + if (!stdout) stdout = object.stdout; + return stdout; + }, + set(value) { stdout = value; } + }, + '_stderr': { + enumerable: false, + configurable: true, + get() { + if (!stderr) { stderr = object.stderr; } + return stderr; + }, + set(value) { stderr = value; } + } + }); +}; + +Console.prototype[kBindProperties] = function(ignoreErrors, colorMode) { + Object.defineProperties(this, { + '_stdoutErrorHandler': { + ...consolePropAttributes, + value: createWriteErrorHandler(this, kUseStdout) + }, + '_stderrErrorHandler': { + ...consolePropAttributes, + value: createWriteErrorHandler(this, kUseStderr) + }, + '_ignoreErrors': { + ...consolePropAttributes, + value: Boolean(ignoreErrors) + }, + '_times': { ...consolePropAttributes, value: new Map() } + }); + + // TODO(joyeecheung): use consolePropAttributes for these + // Corresponds to https://console.spec.whatwg.org/#count-map + this[kCounts] = new Map(); + this[kColorMode] = colorMode; + this[kIsConsole] = true; + this[kGroupIndent] = ''; +}; + // Make a function that can serve as the callback passed to `stream.write()`. -function createWriteErrorHandler(stream) { +function createWriteErrorHandler(instance, streamSymbol) { return (err) => { // This conditional evaluates to true if and only if there was an error // that was not already emitted (which happens when the _write callback // is invoked asynchronously). + const stream = streamSymbol === kUseStdout ? + instance._stdout : instance._stderr; if (err !== null && !stream._writableState.errorEmitted) { // If there was an error, it will be emitted on `stream` as // an `error` event. Adding a `once` listener will keep that error @@ -158,7 +215,15 @@ function createWriteErrorHandler(stream) { }; } -function write(ignoreErrors, stream, string, errorhandler, groupIndent) { +Console.prototype[kWriteToConsole] = function(streamSymbol, string) { + const ignoreErrors = this._ignoreErrors; + const groupIndent = this[kGroupIndent]; + + const useStdout = streamSymbol === kUseStdout; + const stream = useStdout ? this._stdout : this._stderr; + const errorHandler = useStdout ? + this._stdoutErrorHandler : this._stderrErrorHandler; + if (groupIndent.length !== 0) { if (string.indexOf('\n') !== -1) { string = string.replace(/\n/g, `\n${groupIndent}`); @@ -176,7 +241,7 @@ function write(ignoreErrors, stream, string, errorhandler, groupIndent) { // Add and later remove a noop error handler to catch synchronous errors. stream.once('error', noop); - stream.write(string, errorhandler); + stream.write(string, errorHandler); } catch (e) { // console is a debugging utility, so it swallowing errors is not desirable // even in edge cases such as low stack space. @@ -186,7 +251,7 @@ function write(ignoreErrors, stream, string, errorhandler, groupIndent) { } finally { stream.removeListener('error', noop); } -} +}; const kColorInspectOptions = { colors: true }; const kNoColorInspectOptions = {}; @@ -212,34 +277,24 @@ Console.prototype[kFormatForStderr] = function(args) { }; Console.prototype.log = function log(...args) { - write(this._ignoreErrors, - this._stdout, - this[kFormatForStdout](args), - this._stdoutErrorHandler, - this[kGroupIndent]); + this[kWriteToConsole](kUseStdout, this[kFormatForStdout](args)); }; + Console.prototype.debug = Console.prototype.log; Console.prototype.info = Console.prototype.log; Console.prototype.dirxml = Console.prototype.log; Console.prototype.warn = function warn(...args) { - write(this._ignoreErrors, - this._stderr, - this[kFormatForStderr](args), - this._stderrErrorHandler, - this[kGroupIndent]); + this[kWriteToConsole](kUseStderr, this[kFormatForStderr](args)); }; + Console.prototype.error = Console.prototype.warn; Console.prototype.dir = function dir(object, options) { options = Object.assign({ customInspect: false }, this[kGetInspectOptions](this._stdout), options); - write(this._ignoreErrors, - this._stdout, - util.inspect(object, options), - this._stdoutErrorHandler, - this[kGroupIndent]); + this[kWriteToConsole](kUseStdout, util.inspect(object, options)); }; Console.prototype.time = function time(label = 'default') { @@ -299,7 +354,7 @@ Console.prototype.trace = function trace(...args) { Console.prototype.assert = function assert(expression, ...args) { if (!expression) { args[0] = `Assertion failed${args.length === 0 ? '' : `: ${args[0]}`}`; - this.warn(this[kFormatForStderr](args)); + this.warn(...args); // the arguments will be formatted in warn() again } }; @@ -361,7 +416,6 @@ const valuesKey = 'Values'; const indexKey = '(index)'; const iterKey = '(iteration index)'; - const isArray = (v) => ArrayIsArray(v) || isTypedArray(v) || isBuffer(v); // https://console.spec.whatwg.org/#table @@ -488,37 +542,30 @@ function noop() {} // we cannot actually use `new Console` to construct the global console. // Therefore, the console.Console.prototype is not // in the global console prototype chain anymore. + +// TODO(joyeecheung): +// - Move the Console constructor into internal/console.js +// - Move the global console creation code along with the inspector console +// wrapping code in internal/bootstrap/node.js into a separate file. +// - Make this file a simple re-export of those two files. const globalConsole = Object.create({}); -const tempConsole = new Console({ - stdout: process.stdout, - stderr: process.stderr -}); // Since Console is not on the prototype chain of the global console, // the symbol properties on Console.prototype have to be looked up from -// the global console itself. -for (const prop of Object.getOwnPropertySymbols(Console.prototype)) { - globalConsole[prop] = Console.prototype[prop]; -} - -// Reflect.ownKeys() is used here for retrieving Symbols -for (const prop of Reflect.ownKeys(tempConsole)) { - const desc = { ...(Reflect.getOwnPropertyDescriptor(tempConsole, prop)) }; - // Since Console would bind method calls onto the instance, - // make sure the methods are called on globalConsole instead of - // tempConsole. - if (typeof Console.prototype[prop] === 'function') { - desc.value = Console.prototype[prop].bind(globalConsole); +// the global console itself. In addition, we need to make the global +// console a namespace by binding the console methods directly onto +// the global console with the receiver fixed. +for (const prop of Reflect.ownKeys(Console.prototype)) { + if (prop === 'constructor') { continue; } + const desc = Reflect.getOwnPropertyDescriptor(Console.prototype, prop); + if (typeof desc.value === 'function') { // fix the receiver + desc.value = desc.value.bind(globalConsole); } Reflect.defineProperty(globalConsole, prop, desc); } -globalConsole.Console = Console; - -Object.defineProperty(Console, Symbol.hasInstance, { - value(instance) { - return instance[kIsConsole]; - } -}); +globalConsole[kBindStreamsLazy](process); +globalConsole[kBindProperties](true, 'auto'); module.exports = globalConsole; +module.exports.Console = Console; diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 70011637e08af4..00044ed78c289d 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -11,4 +11,4 @@ const list = process.moduleLoadList.slice(); const assert = require('assert'); -assert(list.length <= 78, list); +assert(list.length <= 56, list); diff --git a/test/pseudo-tty/test-stderr-stdout-handle-sigwinch.out b/test/pseudo-tty/test-stderr-stdout-handle-sigwinch.out index dffbe030404487..4023b51f479747 100644 --- a/test/pseudo-tty/test-stderr-stdout-handle-sigwinch.out +++ b/test/pseudo-tty/test-stderr-stdout-handle-sigwinch.out @@ -1,2 +1,2 @@ -calling stdout._refreshSize calling stderr._refreshSize +calling stdout._refreshSize From d422bffe1aea00f4f04a1b666070cd81fddc4a95 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 21 Nov 2018 09:08:40 +0800 Subject: [PATCH 2/3] fixup! console: lazy load process.stderr and process.stdout --- test/parallel/test-bootstrap-modules.js | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 00044ed78c289d..4ffbde8551fe28 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -1,14 +1,20 @@ -/* eslint-disable node-core/required-modules */ - +// Flags: --expose-internals 'use strict'; -// Ordinarily test files must require('common') but that action causes -// the global console to be compiled, defeating the purpose of this test. -// This makes sure no additional files are added without carefully considering -// lazy loading. Please adjust the value if necessary. - +// This list must be computed before we require any modules to +// to eliminate the noise. const list = process.moduleLoadList.slice(); +require('../common'); const assert = require('assert'); -assert(list.length <= 56, list); +// We use the internals to detect if this test is run in a worker +// to eliminate the noise introduced by --experimental-worker +const { internalBinding } = require('internal/test/binding'); +const { threadId } = internalBinding('worker'); +const isMainThread = threadId === 0; +const kMaxModuleCount = isMainThread ? 56 : 78; + +assert(list.length <= kMaxModuleCount, + `Total length: ${list.length}\n` + list.join('\n') +); From e93b4b99e87170c9da9291c8df4e74c1cfdf4725 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 22 Nov 2018 15:37:53 +0900 Subject: [PATCH 3/3] fixup! fixup! console: lazy load process.stderr and process.stdout --- test/parallel/test-bootstrap-modules.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 4ffbde8551fe28..272dec40060624 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -5,14 +5,10 @@ // to eliminate the noise. const list = process.moduleLoadList.slice(); -require('../common'); +const common = require('../common'); const assert = require('assert'); -// We use the internals to detect if this test is run in a worker -// to eliminate the noise introduced by --experimental-worker -const { internalBinding } = require('internal/test/binding'); -const { threadId } = internalBinding('worker'); -const isMainThread = threadId === 0; +const isMainThread = common.isMainThread; const kMaxModuleCount = isMainThread ? 56 : 78; assert(list.length <= kMaxModuleCount,