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 1 commit
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
Prev Previous commit
repl: lazy load fs and destructure binding
  • Loading branch information
@jasnell
jasnell committed May 16, 2018
commit 180567eb8a89474e175e8264fc305645516bae1a
8 changes: 5 additions & 3 deletions lib/internal/repl.js
View file
Open in desktop
Original file line number Diff line number Diff 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 Down Expand 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');
Copy link
Member

@BridgeAR BridgeAR May 15, 2018

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
27 changes: 19 additions & 8 deletions lib/repl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,16 @@ const {
makeRequireFunction,
addBuiltinLibsToObject
} = require('internal/modules/cjs/helpers');
const internalUtil = require('internal/util');
Copy link
Member

@BridgeAR BridgeAR May 15, 2018

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 Down Expand 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 Down Expand 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 Down Expand 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 Down Expand 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