Skip to content
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

lib: reduce startup time #20567

Closed
wants to merge 20 commits into from
Closed

lib: reduce startup time #20567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
35cc355
string_decoder: lazy loaded
BridgeAR May 6, 2018
d480b63
querystring: lazy loaded
BridgeAR May 6, 2018
5c144fa
readline: lazy loaded
BridgeAR May 6, 2018
a18174b
os: lazy loaded
BridgeAR May 6, 2018
02c3b4b
dns: lazy loaded
BridgeAR May 6, 2018
9168e15
bootstrap: only load inspector stuff if necessary
BridgeAR May 6, 2018
1dfa4ec
esm: lazy load necessary loaders
BridgeAR May 6, 2018
246ab38
cluster: remove obsolete array allocation
BridgeAR May 6, 2018
e8ae9a4
trace_events: lazy loaded
BridgeAR May 6, 2018
ce21afd
lib: use capital letters in comments
BridgeAR May 7, 2018
a093ed3
bootstrap: remove unnecessary require
BridgeAR May 7, 2018
4733c2a
tools: stricter eslint rule for globals
BridgeAR May 7, 2018
e0080ce
assert,util: lazy load comparison functions
BridgeAR May 7, 2018
bd96a9d
async_hook: lazy loading for startup performance
BridgeAR May 7, 2018
d31581f
bootstrap: do not call performance hooks
BridgeAR May 7, 2018
0093e0d
stream: lazy load ReadableAsyncIterator
BridgeAR May 7, 2018
0b31adc
console: lazy load cli
BridgeAR May 7, 2018
5c3a245
stream: lazy load end-of-stream
BridgeAR May 7, 2018
b0bbbba
net: lazy load dns
BridgeAR May 7, 2018
3ca0b58
test: add loaded modules test
BridgeAR May 7, 2018
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
2 changes: 1 addition & 1 deletion lib/.eslintrc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ rules:
- selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError)$/])"
message: "Use an error exported by the internal/errors module."
# Custom rules in tools/eslint-rules
node-core/require-buffer: error
node-core/require-globals: error
node-core/buffer-constructor: error
node-core/no-let-in-for-declaration: error
node-core/lowercase-name-for-primitive: error
Expand Down
9 changes: 6 additions & 3 deletions lib/_stream_readable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ const {
ERR_METHOD_NOT_IMPLEMENTED,
ERR_STREAM_UNSHIFT_AFTER_END_EVENT
} = require('internal/errors').codes;
const ReadableAsyncIterator = require('internal/streams/async_iterator');
const { emitExperimentalWarning } = require('internal/util');
var StringDecoder;

// Lazy loaded to improve the startup performance.
let StringDecoder;
let ReadableAsyncIterator;

util.inherits(Readable, Stream);

Expand Down Expand Up @@ -985,7 +987,8 @@ Readable.prototype.wrap = function(stream) {

Readable.prototype[Symbol.asyncIterator] = function() {
emitExperimentalWarning('Readable[Symbol.asyncIterator]');

if (ReadableAsyncIterator === undefined)
ReadableAsyncIterator = require('internal/streams/async_iterator');
return new ReadableAsyncIterator(this);
};

Expand Down
20 changes: 15 additions & 5 deletions lib/assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@
'use strict';

const { Buffer } = require('buffer');
const {
isDeepEqual,
isDeepStrictEqual
} = require('internal/util/comparisons');
const { codes: {
ERR_AMBIGUOUS_ARGUMENT,
ERR_INVALID_ARG_TYPE,
Expand All @@ -34,9 +30,18 @@ const { codes: {
const { AssertionError, errorCache } = require('internal/assert');
const { openSync, closeSync, readSync } = require('fs');
const { inspect, types: { isPromise, isRegExp } } = require('util');
const { EOL } = require('os');
const { EOL } = require('internal/constants');
const { NativeModule } = require('internal/bootstrap/loaders');

let isDeepEqual;
let isDeepStrictEqual;

function lazyLoadComparison() {
const comparison = require('internal/util/comparisons');
isDeepEqual = comparison.isDeepEqual;
isDeepStrictEqual = comparison.isDeepStrictEqual;
}

// Escape control characters but not \n and \t to keep the line breaks and
// indentation intact.
// eslint-disable-next-line no-control-regex
Expand Down Expand Up @@ -285,6 +290,7 @@ assert.notEqual = function notEqual(actual, expected, message) {

// The equivalence assertion tests a deep equality relation.
assert.deepEqual = function deepEqual(actual, expected, message) {
if (isDeepEqual === undefined) lazyLoadComparison();
Copy link
Member

Choose a reason for hiding this comment

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

Instead of this, you could assign isDeepEqual a function that loads and then overwrites itself. That way you can be sure you caught all the use sites.

Copy link
Member

@joyeecheung joyeecheung May 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@hashseed Can we just use something like

let comparison;
function lazyComparison() {
  if (comparison === undefined) {
    comparison = require('internal/util/comparisons');
  }
  return comparison;
}

And use lazyComparison() everywhere for simplicy? I guess V8 should be able to inline that?

Copy link
Member

@hashseed hashseed May 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You would have to use lazyComparison()(args) the way you defined it. Inlining shouldn't be an issue here.

I'd even go a bit further and define a wrapper that can be reused:

function lazify(loader) {
  let loaded = undefined;
  return function(...args) {
    if (loaded === undefined) {
      loaded = loader();
      loader = undefined;
    }
    return loaded(...args);
  };
}

You can then use this to load comparison:

let comparison = lazify(() => require('internal/util/comparisons'));

I wonder though whether inlining that would be an issue. @bmeurer

Copy link
Member Author

Choose a reason for hiding this comment

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

I just ran benchmarks on all three of the variations and looking at the numbers it seems like everything gets inlined (both ways). Depending on how I write the benchmark, the outcome is favorable for some other different versions.

'use strict';

let foo;

let tmpBaz;
function baz() {
  if (tmpBaz === undefined) tmpBaz = require('util');
  return tmpBaz;
}

const lazyUtil = lazify(() => require('util'));
function lazify(loader) {
  let loaded;
  return function() {
    if (loaded === undefined) {
      loaded = loader();
      loader = undefined;
    }
    return loaded;
  };
}

function a(a, b) {
  if (foo === undefined) foo = require('util');
  return foo.isDeepStrictEqual(a, b);
}

function b(a, b) {
  return baz().isDeepStrictEqual(a, b);
}

function c(a, b) {
  return lazyUtil().isDeepStrictEqual(a, b);
}

function bench(fn) {
  const arr = [];
  arr.push(fn(0, 0));
  arr.push(fn(1, 1));
  arr.push(fn(2, 2));
  arr.push(fn(3, 3));
  console.time(`Runtime ${fn.name}`);
  for (var i = 0; i < 1e7; i++) {
    // if (i % 1e4 === 0) {
    //   foo = undefined;
    //   tmpBaz = undefined;
    // }
    arr.push(fn(i, i));
  }
  console.timeEnd(`Runtime ${fn.name}`);
  return arr;
}

for (var i = 0; i < 5; i++) {
  bench(a);
  bench(b);
  bench(c);
}

Without the if block, a is the winner with ~450ms vs ~500ms. With the if block, b is the winner with ~450ms vs ~500ms.

I personally normally use it the way as is right now but I see the point for it being error prone. Even though it also some times allows to only lazy load once outside of a loop instead of running a function in a hot loop.

Copy link
Member Author

@BridgeAR BridgeAR May 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I guess we should agree on one specific way how to do this. I do not have a strong opinion (anymore) and would like to let others decide which one to pick.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like Yang's way if it is fast.

if (!isDeepEqual(actual, expected)) {
innerFail({
actual,
Expand All @@ -298,6 +304,7 @@ assert.deepEqual = function deepEqual(actual, expected, message) {

// The non-equivalence assertion tests for any deep inequality.
assert.notDeepEqual = function notDeepEqual(actual, expected, message) {
if (isDeepEqual === undefined) lazyLoadComparison();
if (isDeepEqual(actual, expected)) {
innerFail({
actual,
Expand All @@ -311,6 +318,7 @@ assert.notDeepEqual = function notDeepEqual(actual, expected, message) {
/* eslint-enable */

assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {
if (isDeepEqual === undefined) lazyLoadComparison();
if (!isDeepStrictEqual(actual, expected)) {
innerFail({
actual,
Expand All @@ -324,6 +332,7 @@ assert.deepStrictEqual = function deepStrictEqual(actual, expected, message) {

assert.notDeepStrictEqual = notDeepStrictEqual;
function notDeepStrictEqual(actual, expected, message) {
if (isDeepEqual === undefined) lazyLoadComparison();
if (isDeepStrictEqual(actual, expected)) {
innerFail({
actual,
Expand Down Expand Up @@ -437,6 +446,7 @@ function expectedException(actual, expected, msg) {
throw new ERR_INVALID_ARG_VALUE('error',
expected, 'may not be an empty object');
}
if (isDeepEqual === undefined) lazyLoadComparison();
for (const key of keys) {
if (typeof actual[key] === 'string' &&
isRegExp(expected[key]) &&
Expand Down
5 changes: 4 additions & 1 deletion lib/console.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const {
} = require('internal/errors');
const { previewMapIterator, previewSetIterator } = require('internal/v8');
const { Buffer: { isBuffer } } = require('buffer');
const cliTable = require('internal/cli_table');
const util = require('util');
const {
isTypedArray, isSet, isMap, isSetIterator, isMapIterator,
Expand All @@ -49,6 +48,9 @@ const {
from: ArrayFrom,
} = Array;

// Lazy loaded for startup performance.
let cliTable;

// Track amount of indentation required via `console.group()`.
const kGroupIndent = Symbol('kGroupIndent');

Expand Down Expand Up @@ -329,6 +331,7 @@ Console.prototype.table = function(tabularData, properties) {
(typeof tabularData !== 'object' && typeof tabularData !== 'function'))
return this.log(tabularData);

if (cliTable === undefined) cliTable = require('internal/cli_table');
const final = (k, v) => this.log(cliTable(k, v));

const inspect = (v) => {
Expand Down
9 changes: 6 additions & 3 deletions lib/dgram.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const {
ERR_SOCKET_DGRAM_NOT_RUNNING
} = errors.codes;
const { Buffer } = require('buffer');
const dns = require('dns');
const util = require('util');
const { isUint8Array } = require('internal/util/types');
const EventEmitter = require('events');
Expand All @@ -47,6 +46,9 @@ const { UV_UDP_REUSEADDR } = process.binding('constants').os;

const { UDP, SendWrap } = process.binding('udp_wrap');

// Lazy load for startup performance.
let dns;

const BIND_STATE_UNBOUND = 0;
const BIND_STATE_BINDING = 1;
const BIND_STATE_BOUND = 2;
Expand All @@ -72,9 +74,10 @@ function lookup6(lookup, address, callback) {


function newHandle(type, lookup) {
if (lookup === undefined)
if (lookup === undefined) {
if (dns === undefined) dns = require('dns');
lookup = dns.lookup;
else if (typeof lookup !== 'function')
} else if (typeof lookup !== 'function')
throw new ERR_INVALID_ARG_TYPE('lookup', 'Function', lookup);

if (type === 'udp4') {
Expand Down
60 changes: 34 additions & 26 deletions lib/internal/bootstrap/node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

setupProcessObject();

// do this good and early, since it handles errors.
// Do this good and early, since it handles errors.
setupProcessFatal();

setupV8();
Expand Down Expand Up @@ -57,15 +57,27 @@
} = perf.constants;

_process.setup_hrtime();
_process.setup_performance();
_process.setup_cpuUsage();
_process.setupMemoryUsage();
_process.setupKillAndExit();
if (global.__coverage__)
NativeModule.require('internal/process/write-coverage').setup();

NativeModule.require('internal/trace_events_async_hooks').setup();
NativeModule.require('internal/inspector_async_hook').setup();

{
const traceEvents = process.binding('trace_events');
const traceEventCategory = 'node,node.async_hooks';

if (traceEvents.categoryGroupEnabled(traceEventCategory)) {
NativeModule.require('internal/trace_events_async_hooks')
.setup(traceEvents, traceEventCategory);
}
}


if (process.config.variables.v8_enable_inspector) {
NativeModule.require('internal/inspector_async_hook').setup();
}

_process.setupChannel();
_process.setupRawDebug();
Expand All @@ -77,10 +89,6 @@
setupGlobalURL();
}

// Ensure setURLConstructor() is called before the native
// URL::ToObject() method is used.
NativeModule.require('internal/url');

// On OpenBSD process.execPath will be relative unless we
// get the full path before process.execPath is used.
if (process.platform === 'openbsd') {
Expand All @@ -95,7 +103,7 @@
});
process.argv[0] = process.execPath;

// Handle `--debug*` deprecation and invalidation
// Handle `--debug*` deprecation and invalidation.
if (process._invalidDebug) {
process.emitWarning(
'`node --debug` and `node --debug-brk` are invalid. ' +
Expand Down Expand Up @@ -164,7 +172,7 @@
'DeprecationWarning', 'DEP0068');
}

// Start the debugger agent
// Start the debugger agent.
process.nextTick(function() {
NativeModule.require('internal/deps/node-inspect/lib/_inspect').start();
});
Expand All @@ -173,7 +181,7 @@
NativeModule.require('internal/v8_prof_processor');

} else {
// There is user code to be run
// There is user code to be run.

// If this is a worker in cluster mode, start up the communication
// channel. This needs to be done before any user code gets executed
Expand All @@ -191,7 +199,7 @@
perf.markMilestone(NODE_PERFORMANCE_MILESTONE_MODULE_LOAD_START);
perf.markMilestone(NODE_PERFORMANCE_MILESTONE_MODULE_LOAD_END);
// User passed '-e' or '--eval' arguments to Node without '-i' or
// '--interactive'
// '--interactive'.

perf.markMilestone(
NODE_PERFORMANCE_MILESTONE_PRELOAD_MODULE_LOAD_START);
Expand All @@ -205,7 +213,7 @@
evalScript('[eval]');
} else if (process.argv[1] && process.argv[1] !== '-') {
perf.markMilestone(NODE_PERFORMANCE_MILESTONE_MODULE_LOAD_START);
// make process.argv[1] into a full path
// Make process.argv[1] into a full path.
const path = NativeModule.require('path');
process.argv[1] = path.resolve(process.argv[1]);

Expand All @@ -217,10 +225,10 @@
preloadModules();
perf.markMilestone(
NODE_PERFORMANCE_MILESTONE_PRELOAD_MODULE_LOAD_END);
// check if user passed `-c` or `--check` arguments to Node.
// Check if user passed `-c` or `--check` arguments to Node.
if (process._syntax_check_only != null) {
const fs = NativeModule.require('fs');
// read the source
// Read the source.
const filename = CJSModule._resolveFilename(process.argv[1]);
const source = fs.readFileSync(filename, 'utf-8');
checkScriptSyntax(source, filename);
Expand Down Expand Up @@ -352,7 +360,7 @@
function setupGlobalConsole() {
const originalConsole = global.console;
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
// Setup Node.js global.console
// Setup Node.js global.console.
const wrappedConsole = NativeModule.require('console');
Object.defineProperty(global, 'console', {
configurable: true,
Expand Down Expand Up @@ -386,7 +394,7 @@
return;
}
const { addCommandLineAPI, consoleCall } = process.binding('inspector');
// Setup inspector command line API
// Setup inspector command line API.
const { makeRequireFunction } =
NativeModule.require('internal/modules/cjs/helpers');
const path = NativeModule.require('path');
Expand Down Expand Up @@ -436,14 +444,14 @@
exceptionHandlerState.captureFn(er);
} else if (!process.emit('uncaughtException', er)) {
// If someone handled it, then great. otherwise, die in C++ land
// since that means that we'll exit the process, emit the 'exit' event
// since that means that we'll exit the process, emit the 'exit' event.
try {
if (!process._exiting) {
process._exiting = true;
process.emit('exit', 1);
}
} catch (er) {
// nothing to be done about it at this point.
} catch {
// Nothing to be done about it at this point.
}
try {
const { kExpandStackSymbol } = NativeModule.require('internal/util');
Expand All @@ -454,7 +462,7 @@
}

// If we handled an error, then make sure any ticks get processed
// by ensuring that the next Immediate cycle isn't empty
// by ensuring that the next Immediate cycle isn't empty.
NativeModule.require('timers').setImmediate(noop);

// Emit the after() hooks now that the exception has been handled.
Expand Down Expand Up @@ -547,7 +555,7 @@
process._tickCallback();
}

// Load preload modules
// Load preload modules.
function preloadModules() {
if (process._preload_modules) {
const {
Expand All @@ -564,13 +572,13 @@
stripShebang, stripBOM
} = NativeModule.require('internal/modules/cjs/helpers');

// remove Shebang
// Remove Shebang.
source = stripShebang(source);
// remove BOM
// Remove BOM.
source = stripBOM(source);
// wrap it
// Wrap it.
source = CJSModule.wrap(source);
// compile the script, this will throw if it fails
// Compile the script, this will throw if it fails.
new vm.Script(source, { displayErrors: true, filename });
}

Expand Down
6 changes: 5 additions & 1 deletion lib/internal/child_process.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const {
ERR_MISSING_ARGS
}
} = require('internal/errors');
const { StringDecoder } = require('string_decoder');
const EventEmitter = require('events');
const net = require('net');
const dgram = require('dgram');
Expand Down Expand Up @@ -47,6 +46,9 @@ const {

const { SocketListSend, SocketListReceive } = SocketList;

// Lazy loaded for startup performance.
let StringDecoder;

const MAX_HANDLE_RETRANSMISSIONS = 3;

// this object contain function to convert TCP objects to native handle objects
Expand Down Expand Up @@ -476,6 +478,8 @@ function setupChannel(target, channel) {

const control = new Control(channel);

if (StringDecoder === undefined)
StringDecoder = require('string_decoder').StringDecoder;
var decoder = new StringDecoder('utf8');
var jsonBuffer = '';
var pendingHandle = null;
Expand Down
8 changes: 2 additions & 6 deletions lib/internal/cluster/master.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,8 @@ function queryServer(worker, message) {
if (worker.exitedAfterDisconnect)
return;

const args = [message.address,
message.port,
message.addressType,
message.fd,
message.index];
const key = args.join(':');
const key = `${message.address}:${message.port}:${message.addressType}:` +
`${message.fd}:${message.index}`;
var handle = handles[key];

if (handle === undefined) {
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const isWindows = process.platform === 'win32';

module.exports = {
// Alphabet chars.
CHAR_UPPERCASE_A: 65, /* A */
Expand Down Expand Up @@ -45,4 +47,6 @@ module.exports = {
// Digits
CHAR_0: 48, /* 0 */
CHAR_9: 57, /* 9 */

EOL: isWindows ? '\r\n' : '\n'
};
Loading