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

module: significantly improve loader performance #26970

Closed
wants to merge 5 commits into from
Closed

module: significantly improve loader performance #26970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b5683e7
benchmark: add new module loading benchmarks
BridgeAR Jan 2, 2019
faa0518
benchmark: improve module-loader benchmark
BridgeAR Mar 28, 2019
50e2414
module: inline try catch
BridgeAR Mar 28, 2019
538f745
module: add path to the module object
BridgeAR Mar 28, 2019
9137c56
module: add extra caching layer
BridgeAR Mar 28, 2019
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
51 changes: 51 additions & 0 deletions benchmark/module/module-loader-deep.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
'use strict';
const fs = require('fs');
const path = require('path');
const common = require('../common.js');

const tmpdir = require('../../test/common/tmpdir');
const benchmarkDirectory = path.join(tmpdir.path, 'nodejs-benchmark-module');

const bench = common.createBenchmark(main, {
ext: ['', '.js'],
files: [1e3],
cache: ['true', 'false']
});

function main({ ext, cache, files }) {
tmpdir.refresh();
fs.mkdirSync(benchmarkDirectory);
fs.writeFileSync(
`${benchmarkDirectory}/a.js`,
'module.exports = {};'
);
for (var i = 0; i <= files; i++) {
fs.mkdirSync(`${benchmarkDirectory}/${i}`);
fs.writeFileSync(
`${benchmarkDirectory}/${i}/package.json`,
'{"main": "index.js"}'
);
fs.writeFileSync(
`${benchmarkDirectory}/${i}/index.js`,
`require('../a${ext}');`
);
}

measureDir(cache === 'true', files);

tmpdir.refresh();
}

function measureDir(cache, files) {
var i;
if (cache) {
for (i = 0; i <= files; i++) {
require(`${benchmarkDirectory}/${i}`);
}
}
bench.start();
for (i = 0; i <= files; i++) {
require(`${benchmarkDirectory}/${i}`);
}
bench.end(files);
}
60 changes: 28 additions & 32 deletions benchmark/module/module-loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,27 @@
'use strict';
const fs = require('fs');
const path = require('path');
const { builtinModules } = require('module');
const common = require('../common.js');

const tmpdir = require('../../test/common/tmpdir');
const benchmarkDirectory = path.join(tmpdir.path, 'nodejs-benchmark-module');
let benchmarkDirectory = path.join(tmpdir.path, 'nodejs-benchmark-module');

// Filter all irregular modules.
const otherModules = builtinModules.filter((name) => !/\/|^_|^sys/.test(name));

const bench = common.createBenchmark(main, {
n: [5e4],
fullPath: ['true', 'false'],
useCache: ['true', 'false']
name: ['', '/', '/index.js'],
dir: ['rel', 'abs'],
files: [5e2],
n: [1, 1e3],
cache: ['true', 'false']
});

function main({ n, fullPath, useCache }) {
function main({ n, name, cache, files, dir }) {
tmpdir.refresh();
try { fs.mkdirSync(benchmarkDirectory); } catch {}
for (var i = 0; i <= n; i++) {
fs.mkdirSync(benchmarkDirectory);
for (var i = 0; i <= files; i++) {
fs.mkdirSync(`${benchmarkDirectory}${i}`);
fs.writeFileSync(
`${benchmarkDirectory}${i}/package.json`,
Expand All @@ -27,38 +33,28 @@ function main({ n, fullPath, useCache }) {
);
}

if (fullPath === 'true')
measureFull(n, useCache === 'true');
else
measureDir(n, useCache === 'true');
if (dir === 'rel')
benchmarkDirectory = path.relative(__dirname, benchmarkDirectory);

tmpdir.refresh();
}
measureDir(n, cache === 'true', files, name);

function measureFull(n, useCache) {
var i;
if (useCache) {
for (i = 0; i <= n; i++) {
require(`${benchmarkDirectory}${i}/index.js`);
}
}
bench.start();
for (i = 0; i <= n; i++) {
require(`${benchmarkDirectory}${i}/index.js`);
}
bench.end(n);
tmpdir.refresh();
}

function measureDir(n, useCache) {
function measureDir(n, cache, files, name) {
var i;
if (useCache) {
for (i = 0; i <= n; i++) {
require(`${benchmarkDirectory}${i}`);
if (cache) {
for (i = 0; i <= files; i++) {
require(`${benchmarkDirectory}${i}${name}`);
}
}
bench.start();
for (i = 0; i <= n; i++) {
require(`${benchmarkDirectory}${i}`);
for (i = 0; i <= files; i++) {
for (var j = 0; j < n; j++)
require(`${benchmarkDirectory}${i}${name}`);
// Pretend mixed input (otherwise the results are less representative due to
// highly specialized code).
require(otherModules[i % otherModules.length]);
}
bench.end(n);
bench.end(n * files);
}
39 changes: 29 additions & 10 deletions lib/internal/modules/cjs/loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ const {

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

const relativeResolveCache = Object.create(null);

let requireDepth = 0;
let statCache = new Map();
function stat(filename) {
Expand All @@ -103,8 +105,9 @@ function updateChildren(parent, child, scan) {
children.push(child);
}

function Module(id, parent) {
function Module(id = '', parent) {
this.id = id;
this.path = path.dirname(id);
this.exports = {};
this.parent = parent;
updateChildren(parent, this, false);
Expand Down Expand Up @@ -597,14 +600,28 @@ Module._resolveLookupPaths = function(request, parent, newReturn) {
// Then have it load the file contents before returning its exports
// object.
Module._load = function(request, parent, isMain) {
let relResolveCacheIdentifier;
if (parent) {
debug('Module._load REQUEST %s parent: %s', request, parent.id);
// Fast path for (lazy loaded) modules in the same directory. The indirect
// caching is required to allow cache invalidation without changing the old
// cache key names.
relResolveCacheIdentifier = `${parent.path}\x00${request}`;
const filename = relativeResolveCache[relResolveCacheIdentifier];
if (filename !== undefined) {
const cachedModule = Module._cache[filename];
if (cachedModule !== undefined) {
updateChildren(parent, cachedModule, true);
return cachedModule.exports;
}
delete relativeResolveCache[relResolveCacheIdentifier];
}
}

const filename = Module._resolveFilename(request, parent, isMain);

const cachedModule = Module._cache[filename];
if (cachedModule) {
if (cachedModule !== undefined) {
updateChildren(parent, cachedModule, true);
return cachedModule.exports;
}
Copy link
Member

Choose a reason for hiding this comment

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

Could this be done at the Module._resolveFilename level?
A cache key could be something like

cacheKey =
  request + "\0" +
  dirPath + "\0" +
  (isMain ? "1" : "")

Copy link
Member Author

Choose a reason for hiding this comment

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

That would not be beneficial as far as I can tell. I would also rather not move any parts around since the loader is highly monkey patched and every subtle change can cause side-effects.

isMain should not have any impact on the key as it's only about the very first require call and that is not required for the cache key.

Expand All @@ -624,23 +641,25 @@ Module._load = function(request, parent, isMain) {
}

Module._cache[filename] = module;
if (parent !== undefined) {
relativeResolveCache[relResolveCacheIdentifier] = filename;
}

tryModuleLoad(module, filename);

return module.exports;
};

function tryModuleLoad(module, filename) {
BridgeAR marked this conversation as resolved.
Show resolved Hide resolved
var threw = true;
let threw = true;
try {
module.load(filename);
threw = false;
} finally {
if (threw) {
delete Module._cache[filename];
if (parent !== undefined) {
delete relativeResolveCache[relResolveCacheIdentifier];
}
}
}
}

return module.exports;
};

Module._resolveFilename = function(request, parent, isMain, options) {
if (NativeModule.canBeRequiredByUsers(request)) {
Expand Down
1 change: 0 additions & 1 deletion test/message/assert_throws_stack.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,3 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
at *
at *
at *
at *
1 change: 0 additions & 1 deletion test/message/console.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,3 @@ Trace: foo
at *
at *
at *
at *
1 change: 0 additions & 1 deletion test/message/core_line_numbers.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ RangeError: Invalid input
at Module._compile (internal/modules/cjs/loader.js:*:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*
1 change: 0 additions & 1 deletion test/message/error_exit.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
at Module._compile (internal/modules/cjs/loader.js:*:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*
1 change: 0 additions & 1 deletion test/message/events_unhandled_error_common_trace.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ Error: foo:bar
at Module._compile (internal/modules/cjs/loader.js:*:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*
Expand Down
1 change: 0 additions & 1 deletion test/message/events_unhandled_error_nexttick.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ Error
at Module._compile (internal/modules/cjs/loader.js:*:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*
Expand Down
1 change: 0 additions & 1 deletion test/message/events_unhandled_error_sameline.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ Error
at Module._compile (internal/modules/cjs/loader.js:*:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*
Expand Down
2 changes: 1 addition & 1 deletion test/message/if-error-has-good-stack.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ AssertionError [ERR_ASSERTION]: ifError got unwanted exception: test error
at Module._compile (internal/modules/cjs/loader.js:*:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
at Module.load (internal/modules/cjs/loader.js:*:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*
2 changes: 1 addition & 1 deletion test/message/undefined_reference_in_new_context.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ ReferenceError: foo is not defined
at Module._compile (internal/modules/cjs/loader.js:*)
at *..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
2 changes: 0 additions & 2 deletions test/message/unhandled_promise_trace_warnings.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
at *
at *
at *
at *
(node:*) Error: This was rejected
at * (*test*message*unhandled_promise_trace_warnings.js:*)
at *
Expand All @@ -21,7 +20,6 @@
at *
at *
at *
at *
(node:*) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
at *
at *
Expand Down
5 changes: 0 additions & 5 deletions test/message/util_inspect_error.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
at *
at *
at *
at *
nested:
{ err:
Error: foo
Expand All @@ -20,7 +19,6 @@
at *
at *
at *
at *
{
err: Error: foo
bar
Expand All @@ -31,7 +29,6 @@
at *
at *
at *
at *
nested: {
err: Error: foo
bar
Expand All @@ -42,7 +39,6 @@
at *
at *
at *
at *
}
}
{ Error: foo
Expand All @@ -54,5 +50,4 @@ bar
at *
at *
at *
at *
foo: 'bar' }
4 changes: 2 additions & 2 deletions test/message/vm_display_runtime_error.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ Error: boo!
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*)
at internal/main/run_main_module.js:*:*
test.vm:1
throw new Error("spooky!")
^
Expand All @@ -26,6 +26,6 @@ Error: spooky!
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*)
at internal/main/run_main_module.js:*:*
4 changes: 2 additions & 2 deletions test/message/vm_display_syntax_error.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ SyntaxError: Unexpected number
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*)
at internal/main/run_main_module.js:*:*
test.vm:1
var 5;
^
Expand All @@ -24,6 +24,6 @@ SyntaxError: Unexpected number
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*)
at internal/main/run_main_module.js:*:*
2 changes: 1 addition & 1 deletion test/message/vm_dont_display_runtime_error.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ Error: boo!
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*)
at internal/main/run_main_module.js:*:*
3 changes: 2 additions & 1 deletion test/message/vm_dont_display_syntax_error.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ SyntaxError: Unexpected number
at Module._compile (internal/modules/cjs/loader.js:*)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
at Module.load (internal/modules/cjs/loader.js:*)
at tryModuleLoad (internal/modules/cjs/loader.js:*:*)
at Function.Module._load (internal/modules/cjs/loader.js:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*)
at internal/main/run_main_module.js:*:*