Skip to content

lib: lazy loading and cleanup#20734

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 3 commits into from
Closed

lib: lazy loading and cleanup #20734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
410d73d
dgram,net: lazy load dns when necessary
jasnell May 14, 2018
e0aa08b
fs: destructure internalUtils assignments
jasnell May 14, 2018
180567e
repl: lazy load fs and destructure binding
jasnell May 15, 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
9 changes: 6 additions & 3 deletions lib/dgram.js
View file
Open in desktop
Original file line numberDiff line numberDiff 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@@ -56,6 +55,7 @@ const SEND_BUFFER = false;

// Lazily loaded
var cluster = null;
var dns = null;

const errnoException = errors.errnoException;
const exceptionWithHostPort = errors.exceptionWithHostPort;
Expand All@@ -72,10 +72,13 @@ function lookup6(lookup, address, callback) {


function newHandle(type, lookup) {
if (lookup === undefined)
if (lookup === undefined) {
if (!dns)
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') {
const handle = new UDP();
Expand Down
20 changes: 12 additions & 8 deletions lib/fs.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,11 @@ const { FSEvent } = process.binding('fs_event_wrap');
const promises = require('internal/fs/promises');
const internalFS = require('internal/fs/utils');
const { getPathFromURL } = require('internal/url');
const internalUtil = require('internal/util');
const {
customPromisifyArgs,
deprecate,
promisify
} = require('internal/util');
const {
copyObject,
getOptions,
Expand DownExpand Up@@ -230,7 +234,7 @@ fs.exists = function(path, callback) {
}
};

Object.defineProperty(fs.exists, internalUtil.promisify.custom, {
Object.defineProperty(fs.exists, promisify.custom, {
value: (path) => {
const promise = createPromise();
fs.exists(path, (exists) => promiseResolve(promise, exists));
Expand DownExpand Up@@ -591,7 +595,7 @@ fs.read = function(fd, buffer, offset, length, position, callback) {
binding.read(fd, buffer, offset, length, position, req);
};

Object.defineProperty(fs.read, internalUtil.customPromisifyArgs,
Object.defineProperty(fs.read, customPromisifyArgs,
{ value: ['bytesRead', 'buffer'], enumerable: false });

fs.readSync = function(fd, buffer, offset, length, position) {
Expand DownExpand Up@@ -659,7 +663,7 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
return binding.writeString(fd, buffer, offset, length, req);
};

Object.defineProperty(fs.write, internalUtil.customPromisifyArgs,
Object.defineProperty(fs.write, customPromisifyArgs,
{ value: ['bytesWritten', 'buffer'], enumerable: false });

// usage:
Expand DownExpand Up@@ -2367,8 +2371,8 @@ WriteStream.prototype.destroySoon = WriteStream.prototype.end;
var SyncWriteStream = internalFS.SyncWriteStream;
Object.defineProperty(fs, 'SyncWriteStream', {
configurable: true,
get: internalUtil.deprecate(() => SyncWriteStream,
'fs.SyncWriteStream is deprecated.', 'DEP0061'),
set: internalUtil.deprecate((val) => { SyncWriteStream = val; },
'fs.SyncWriteStream is deprecated.', 'DEP0061')
get: deprecate(() => SyncWriteStream,
'fs.SyncWriteStream is deprecated.', 'DEP0061'),
set: deprecate((val) => { SyncWriteStream = val; },
'fs.SyncWriteStream is deprecated.', 'DEP0061')
});
8 changes: 5 additions & 3 deletions lib/internal/repl.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,9 +2,6 @@

const { Interface } = require('readline');
const REPL = require('repl');
const path = require('path');
const fs = require('fs');
const os = require('os');
const util = require('util');
const debug = util.debuglog('repl');
module.exports = Object.create(REPL);
Expand DownExpand Up@@ -69,6 +66,11 @@ function createRepl(env, opts, cb) {
}

function setupHistory(repl, historyPath, ready) {
// Lazy load
const fs = require('fs');
const path = require('path');

@BridgeARBridgeARMay 15, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

path is definitely always loaded eager and I do not see how we can prevent that as it is necessary during bootstrapping. So this should probably not be lazy loaded.

const os = require('os');

// Empty string disables persistent history
if (typeof historyPath === 'string')
historyPath = historyPath.trim();
Expand Down
9 changes: 8 additions & 1 deletion lib/net.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,7 +75,9 @@ const {
ERR_SOCKET_BAD_PORT,
ERR_SOCKET_CLOSED
} = errors.codes;
const dns = require('dns');

// Lazy loaded
var dns = null;

const kLastWriteQueueSize = Symbol('lastWriteQueueSize');

Expand DownExpand Up@@ -1034,6 +1036,9 @@ function lookupAndConnect(self, options) {
throw new ERR_INVALID_ARG_TYPE('options.lookup',
'Function', options.lookup);

if (!dns)
dns = require('dns');

var dnsopts = {
family: options.family,
hints: options.hints || 0
Expand DownExpand Up@@ -1482,6 +1487,8 @@ Server.prototype.listen = function(...args) {
};

function lookupAndListen(self, port, address, backlog, exclusive) {
if (!dns)
dns = require('dns');
dns.lookup(address, function doListen(err, ip, addressType) {
if (err) {
self.emit('error', err);
Expand Down
27 changes: 19 additions & 8 deletions lib/repl.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,15 +47,16 @@ const {
makeRequireFunction,
addBuiltinLibsToObject
} = require('internal/modules/cjs/helpers');
const internalUtil = require('internal/util');

@BridgeARBridgeARMay 15, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I am not mistaken it will not (easily) be possible to lazy load internal/util. I tried to do that and it is used for a lot of things that are eagerly loaded.

const { isTypedArray } = require('internal/util/types');
const util = require('util');
const utilBinding = process.binding('util');
const {
startSigintWatchdog,
stopSigintWatchdog
} = process.binding('util');
const { inherits } = util;
const Stream = require('stream');
const vm = require('vm');
const path = require('path');
const fs = require('fs');
const { Interface } = require('readline');
const { Console } = require('console');
const CJSModule = require('internal/modules/cjs/loader');
Expand All@@ -72,6 +73,13 @@ const { experimentalREPLAwait } = process.binding('config');

// Lazy-loaded.
let processTopLevelAwait;
let fs;

function lazyFs() {
if (!fs)
fs = require('fs');
return fs;
}

const parentModule = module;
const replMap = new WeakMap();
Expand DownExpand Up@@ -301,7 +309,7 @@ function REPLServer(prompt,
if (self.breakEvalOnSigint) {
// Start the SIGINT watchdog before entering raw mode so that a very
// quick Ctrl+C doesn't lead to aborting the process completely.
if (!utilBinding.startSigintWatchdog())
if (!startSigintWatchdog())
throw new ERR_CANNOT_WATCH_SIGINT();
previouslyInRawMode = self._setRawMode(false);
}
Expand All@@ -325,7 +333,7 @@ function REPLServer(prompt,

// Returns true if there were pending SIGINTs *after* the script
// has terminated without being interrupted itself.
if (utilBinding.stopSigintWatchdog()) {
if (stopSigintWatchdog()) {
self.emit('SIGINT');
}
}
Expand DownExpand Up@@ -397,14 +405,15 @@ function REPLServer(prompt,
self.eval = self._domain.bind(eval_);

self._domain.on('error', function debugDomainError(e) {
const { decorateErrorStack, isError: _isError } = require('internal/util');
debug('domain error');
const top = replMap.get(self);
const pstrace = Error.prepareStackTrace;
Error.prepareStackTrace = prepareStackTrace(pstrace);
if (typeof e === 'object')
internalUtil.decorateErrorStack(e);
decorateErrorStack(e);
Error.prepareStackTrace = pstrace;
const isError = internalUtil.isError(e);
const isError = _isError(e);
if (!self.underscoreErrAssigned)
self.lastError = e;
if (e instanceof SyntaxError && e.stack) {
Expand DownExpand Up@@ -1020,6 +1029,7 @@ function complete(line, callback) {
}

for (i = 0; i < paths.length; i++) {
const fs = lazyFs();
dir = path.resolve(paths[i], subdir);
try {
files = fs.readdirSync(dir);
Expand DownExpand Up@@ -1428,7 +1438,7 @@ function defineDefaultCommands(repl) {
help: 'Save all evaluated commands in this REPL session to a file',
action: function(file) {
try {
fs.writeFileSync(file, this.lines.join('\n') + '\n');
lazyFs().writeFileSync(file, this.lines.join('\n') + '\n');
this.outputStream.write('Session saved to: ' + file + '\n');
} catch (e) {
this.outputStream.write('Failed to save: ' + file + '\n');
Expand All@@ -1441,6 +1451,7 @@ function defineDefaultCommands(repl) {
help: 'Load JS from a file into the REPL session',
action: function(file) {
try {
const fs = lazyFs();
var stats = fs.statSync(file);
if (stats && stats.isFile()) {
_turnOnEditorMode(this);
Expand Down