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. PR-URL: nodejs#24534 Reviewed-By: Gus Caplan <[email protected]>
refack · Jan 10, 2019 · df8ee76 · df8ee76
1 parent 691c9cb
commit df8ee76
Show file tree

Hide file tree

Showing 3 changed files with 133 additions and 84 deletions.
diff --git 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
@@ -1,14 +1,16 @@
-/* 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();
 
+const common = require('../common');
 const assert = require('assert');
 
-assert(list.length <= 78, list);
+const isMainThread = common.isMainThread;
+const kMaxModuleCount = isMainThread ? 56 : 78;
+
+assert(list.length <= kMaxModuleCount,
+       `Total length: ${list.length}\n` + list.join('\n')
+);
diff --git 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