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

Jump to bottom

lib: improve module loading performance #5172

Merged
merged 2 commits into from
Apr 14, 2016
Merged

lib: improve module loading performance #5172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e4674a
benchmark: add module loader benchmark parameter
mscdex Feb 10, 2016
ae18bbe
lib: improve module loading performance
mscdex Apr 7, 2016
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
21 changes: 17 additions & 4 deletions benchmark/module/module-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@ var tmpDirectory = path.join(__dirname, '..', 'tmp');
var benchmarkDirectory = path.join(tmpDirectory, 'nodejs-benchmark-module');

var bench = common.createBenchmark(main, {
thousands: [50]
thousands: [50],
fullPath: ['true', 'false']
});

function main(conf) {
var n = +conf.thousands * 1e3;

rmrf(tmpDirectory);
try { fs.mkdirSync(tmpDirectory); } catch (e) {}
try { fs.mkdirSync(benchmarkDirectory); } catch (e) {}

var n = +conf.thousands * 1e3;
for (var i = 0; i <= n; i++) {
fs.mkdirSync(benchmarkDirectory + i);
fs.writeFileSync(
Expand All @@ -28,10 +30,21 @@ function main(conf) {
);
}

measure(n);
if (conf.fullPath === 'true')
measureFull(n);
else
measureDir(n);
}

function measureFull(n) {
bench.start();
for (var i = 0; i <= n; i++) {
require(benchmarkDirectory + i + '/index.js');
}
bench.end(n / 1e3);
}

function measure(n) {
function measureDir(n) {
bench.start();
for (var i = 0; i <= n; i++) {
require(benchmarkDirectory + i);
Expand Down
93 changes: 54 additions & 39 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,42 @@ function tryToString(buf, encoding, callback) {
callback(e, buf);
}

function tryStatSync(fd, isUserFd) {
var threw = true;
var st;
try {
st = fs.fstatSync(fd);
threw = false;
} finally {
if (threw && !isUserFd) fs.closeSync(fd);
}
return st;
}

function tryCreateBuffer(size, fd, isUserFd) {
var threw = true;
var buffer;
try {
buffer = Buffer.allocUnsafe(size);
threw = false;
} finally {
if (threw && !isUserFd) fs.closeSync(fd);
}
return buffer;
}

function tryReadSync(fd, isUserFd, buffer, pos, len) {
var threw = true;
var bytesRead;
try {
bytesRead = fs.readSync(fd, buffer, pos, len);
threw = false;
} finally {
if (threw && !isUserFd) fs.closeSync(fd);
}
return bytesRead;
}

Copy link
Member

Choose a reason for hiding this comment

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

Seeing these I can't help but wonder if there's value in having a more generic utility attempt(fn,fin) mechanism... e.g.

const attempt = require('internal/util').attempt;
attempt(() => doSomething(), (threw) => doCleanup())

where attempt() ==

function attempt (fn, fin) {
  var threw = true;
  try {
    fn();
    threw = false;
  } finally {
    fin(threw);
  }
}

Likely not worth the additional complexity and perf cost tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I dunno. Hopefully v8 will optimize try-* blocks before it comes to that.

fs.readFileSync = function(path, options) {
if (!options) {
options = { encoding: null, flag: 'r' };
Expand All @@ -466,57 +502,36 @@ fs.readFileSync = function(path, options) {
var isUserFd = isFd(path); // file descriptor ownership
var fd = isUserFd ? path : fs.openSync(path, flag, 0o666);

var st;
var size;
var threw = true;
try {
st = fs.fstatSync(fd);
size = st.isFile() ? st.size : 0;
threw = false;
} finally {
if (threw && !isUserFd) fs.closeSync(fd);
}

var st = tryStatSync(fd, isUserFd);
var size = st.isFile() ? st.size : 0;
var pos = 0;
var buffer; // single buffer with file data
var buffers; // list for when size is unknown

if (size === 0) {
buffers = [];
} else {
threw = true;
try {
buffer = Buffer.allocUnsafe(size);
threw = false;
} finally {
if (threw && !isUserFd) fs.closeSync(fd);
}
buffer = tryCreateBuffer(size, fd, isUserFd);
}

var done = false;
var bytesRead;

while (!done) {
threw = true;
try {
if (size !== 0) {
bytesRead = fs.readSync(fd, buffer, pos, size - pos);
} else {
// the kernel lies about many files.
// Go ahead and try to read some bytes.
buffer = Buffer.allocUnsafe(8192);
bytesRead = fs.readSync(fd, buffer, 0, 8192);
if (bytesRead) {
buffers.push(buffer.slice(0, bytesRead));
}
if (size !== 0) {
do {
bytesRead = tryReadSync(fd, isUserFd, buffer, pos, size - pos);
pos += bytesRead;
} while (bytesRead !== 0 && pos < size);
} else {
do {
// the kernel lies about many files.
// Go ahead and try to read some bytes.
buffer = Buffer.allocUnsafe(8192);
bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192);
if (bytesRead !== 0) {
buffers.push(buffer.slice(0, bytesRead));
}
threw = false;
} finally {
if (threw && !isUserFd) fs.closeSync(fd);
}

pos += bytesRead;
done = (bytesRead === 0) || (size !== 0 && pos >= size);
pos += bytesRead;
} while (bytesRead !== 0);
}

if (!isUserFd)
Expand Down
52 changes: 30 additions & 22 deletions lib/internal/bootstrap_node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,37 +284,45 @@
};
}

function evalScript(name) {
var Module = NativeModule.require('module');
var path = NativeModule.require('path');

function tryGetCwd(path) {
var threw = true;
var cwd;
try {
var cwd = process.cwd();
} catch (e) {
// getcwd(3) can fail if the current working directory has been deleted.
// Fall back to the directory name of the (absolute) executable path.
// It's not really correct but what are the alternatives?
cwd = path.dirname(process.execPath);
cwd = process.cwd();
threw = false;
} finally {
if (threw) {
// getcwd(3) can fail if the current working directory has been deleted.
// Fall back to the directory name of the (absolute) executable path.
// It's not really correct but what are the alternatives?
return path.dirname(process.execPath);
}
}
return cwd;
}

function evalScript(name) {
const Module = NativeModule.require('module');
const path = NativeModule.require('path');
const cwd = tryGetCwd(path);

var module = new Module(name);
const module = new Module(name);
module.filename = path.join(cwd, name);
module.paths = Module._nodeModulePaths(cwd);
var script = process._eval;
var body = script;
script = `global.__filename = ${JSON.stringify(name)};\n` +
'global.exports = exports;\n' +
'global.module = module;\n' +
'global.__dirname = __dirname;\n' +
'global.require = require;\n' +
'return require("vm").runInThisContext(' +
`${JSON.stringify(body)}, { filename: ` +
`${JSON.stringify(name)}, displayErrors: true });\n`;
const body = process._eval;
const script = `global.__filename = ${JSON.stringify(name)};\n` +
'global.exports = exports;\n' +
'global.module = module;\n' +
'global.__dirname = __dirname;\n' +
'global.require = require;\n' +
'return require("vm").runInThisContext(' +
`${JSON.stringify(body)}, { filename: ` +
`${JSON.stringify(name)}, displayErrors: true });\n`;
// Defer evaluation for a tick. This is a workaround for deferred
// events not firing when evaluating scripts from the command line,
// see https://github.com/nodejs/node/issues/1600.
process.nextTick(function() {
var result = module._compile(script, `${name}-wrapper`);
const result = module._compile(script, `${name}-wrapper`);
if (process._print_eval) console.log(result);
});
}
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ function makeRequireFunction() {
}
}

require.resolve = function(request) {
function resolve(request) {
return Module._resolveFilename(request, self);
};
}

require.resolve = resolve;

require.main = process.mainModule;

Expand Down
Loading