From b5683e7e3d65ddda97cd7e8f4128f3b8e4dfbcd3 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 2 Jan 2019 13:54:51 +0100 Subject: [PATCH 1/5] benchmark: add new module loading benchmarks --- benchmark/module/module-loader-deep.js | 51 ++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 benchmark/module/module-loader-deep.js diff --git a/benchmark/module/module-loader-deep.js b/benchmark/module/module-loader-deep.js new file mode 100644 index 00000000000000..f686b8df47dfba --- /dev/null +++ b/benchmark/module/module-loader-deep.js @@ -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); +} From faa0518da398ac58f2d6a15be20d50b27be905e1 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 28 Mar 2019 22:31:43 +0100 Subject: [PATCH 2/5] benchmark: improve module-loader benchmark Add more benchmark options to properly verify the gains. This makes sure the benchmark also tests requiring the same module again instead of only loading each module only once. --- benchmark/module/module-loader.js | 60 +++++++++++++++---------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/benchmark/module/module-loader.js b/benchmark/module/module-loader.js index e780d6376b5e8d..3b8e9be2d60636 100644 --- a/benchmark/module/module-loader.js +++ b/benchmark/module/module-loader.js @@ -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`, @@ -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); } From 50e2414a2f6ecd344abd0968413c72fdbfd7e9f9 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 28 Mar 2019 22:42:06 +0100 Subject: [PATCH 3/5] module: inline try catch Moving `try / catch` into separate functions is not necessary anymore due to V8 optimizations. --- lib/internal/modules/cjs/loader.js | 12 ++++-------- test/message/assert_throws_stack.out | 1 - test/message/console.out | 1 - test/message/core_line_numbers.out | 1 - test/message/error_exit.out | 1 - test/message/events_unhandled_error_common_trace.out | 1 - test/message/events_unhandled_error_nexttick.out | 1 - test/message/events_unhandled_error_sameline.out | 1 - test/message/if-error-has-good-stack.out | 2 +- test/message/undefined_reference_in_new_context.out | 2 +- test/message/unhandled_promise_trace_warnings.out | 2 -- test/message/util_inspect_error.out | 5 ----- test/message/vm_display_runtime_error.out | 4 ++-- test/message/vm_display_syntax_error.out | 4 ++-- test/message/vm_dont_display_runtime_error.out | 2 +- test/message/vm_dont_display_syntax_error.out | 3 ++- 16 files changed, 13 insertions(+), 30 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 5b0a1cf3f57a8f..787d7fcd3520c5 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -625,13 +625,7 @@ Module._load = function(request, parent, isMain) { Module._cache[filename] = module; - tryModuleLoad(module, filename); - - return module.exports; -}; - -function tryModuleLoad(module, filename) { - var threw = true; + let threw = true; try { module.load(filename); threw = false; @@ -640,7 +634,9 @@ function tryModuleLoad(module, filename) { delete Module._cache[filename]; } } -} + + return module.exports; +}; Module._resolveFilename = function(request, parent, isMain, options) { if (NativeModule.canBeRequiredByUsers(request)) { diff --git a/test/message/assert_throws_stack.out b/test/message/assert_throws_stack.out index 0457670d6a317c..7d4a7aecbcd410 100644 --- a/test/message/assert_throws_stack.out +++ b/test/message/assert_throws_stack.out @@ -16,4 +16,3 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal: at * at * at * - at * diff --git a/test/message/console.out b/test/message/console.out index 0a19ff8be321ff..6d6e2bd1abb5b7 100644 --- a/test/message/console.out +++ b/test/message/console.out @@ -7,4 +7,3 @@ Trace: foo at * at * at * - at * diff --git a/test/message/core_line_numbers.out b/test/message/core_line_numbers.out index 0336e5b451651e..53d3894825b4a4 100644 --- a/test/message/core_line_numbers.out +++ b/test/message/core_line_numbers.out @@ -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:*:* diff --git a/test/message/error_exit.out b/test/message/error_exit.out index 3364210099ca75..b975d41a55d929 100644 --- a/test/message/error_exit.out +++ b/test/message/error_exit.out @@ -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:*:* diff --git a/test/message/events_unhandled_error_common_trace.out b/test/message/events_unhandled_error_common_trace.out index 0d64143c67f865..610ea6064ddc00 100644 --- a/test/message/events_unhandled_error_common_trace.out +++ b/test/message/events_unhandled_error_common_trace.out @@ -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:*:* diff --git a/test/message/events_unhandled_error_nexttick.out b/test/message/events_unhandled_error_nexttick.out index d53e96ee21a628..ddb13807eea5b5 100644 --- a/test/message/events_unhandled_error_nexttick.out +++ b/test/message/events_unhandled_error_nexttick.out @@ -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:*:* diff --git a/test/message/events_unhandled_error_sameline.out b/test/message/events_unhandled_error_sameline.out index f877b254aafe97..798061a39bac8b 100644 --- a/test/message/events_unhandled_error_sameline.out +++ b/test/message/events_unhandled_error_sameline.out @@ -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:*:* diff --git a/test/message/if-error-has-good-stack.out b/test/message/if-error-has-good-stack.out index 2a4dfe8fb26630..c762d74aedbca2 100644 --- a/test/message/if-error-has-good-stack.out +++ b/test/message/if-error-has-good-stack.out @@ -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:*:* diff --git a/test/message/undefined_reference_in_new_context.out b/test/message/undefined_reference_in_new_context.out index 6b5fedfa993701..9cc5eced7ceaa2 100644 --- a/test/message/undefined_reference_in_new_context.out +++ b/test/message/undefined_reference_in_new_context.out @@ -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:*:*) diff --git a/test/message/unhandled_promise_trace_warnings.out b/test/message/unhandled_promise_trace_warnings.out index 2d3646b51a203d..13778fb798aa6c 100644 --- a/test/message/unhandled_promise_trace_warnings.out +++ b/test/message/unhandled_promise_trace_warnings.out @@ -12,7 +12,6 @@ at * at * at * - at * (node:*) Error: This was rejected at * (*test*message*unhandled_promise_trace_warnings.js:*) at * @@ -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 * diff --git a/test/message/util_inspect_error.out b/test/message/util_inspect_error.out index 31b65eb2e2bf3c..644ccd58831ef7 100644 --- a/test/message/util_inspect_error.out +++ b/test/message/util_inspect_error.out @@ -8,7 +8,6 @@ at * at * at * - at * nested: { err: Error: foo @@ -20,7 +19,6 @@ at * at * at * - at * { err: Error: foo bar @@ -31,7 +29,6 @@ at * at * at * - at * nested: { err: Error: foo bar @@ -42,7 +39,6 @@ at * at * at * - at * } } { Error: foo @@ -54,5 +50,4 @@ bar at * at * at * - at * foo: 'bar' } diff --git a/test/message/vm_display_runtime_error.out b/test/message/vm_display_runtime_error.out index bc95b2a7b092f5..7927510c4152ee 100644 --- a/test/message/vm_display_runtime_error.out +++ b/test/message/vm_display_runtime_error.out @@ -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!") ^ @@ -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:*:* diff --git a/test/message/vm_display_syntax_error.out b/test/message/vm_display_syntax_error.out index f0692723e81257..71d4a0ea1311f9 100644 --- a/test/message/vm_display_syntax_error.out +++ b/test/message/vm_display_syntax_error.out @@ -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; ^ @@ -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:*:* diff --git a/test/message/vm_dont_display_runtime_error.out b/test/message/vm_dont_display_runtime_error.out index 532cfbf4dd8125..831ec8b6be4eab 100644 --- a/test/message/vm_dont_display_runtime_error.out +++ b/test/message/vm_dont_display_runtime_error.out @@ -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:*:* diff --git a/test/message/vm_dont_display_syntax_error.out b/test/message/vm_dont_display_syntax_error.out index f27cb1a0859d8c..f98695166a015c 100644 --- a/test/message/vm_dont_display_syntax_error.out +++ b/test/message/vm_dont_display_syntax_error.out @@ -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:*:* + From 538f745154f1d6de75bf35f14ea2679285ac6608 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 28 Mar 2019 23:34:24 +0100 Subject: [PATCH 4/5] module: add path to the module object This adds the `path` property to the module object. It contains the current directory as path. That is necessary to add an extra caching layer. It also makes sure the `id` uses a default in case it's not set. Otherwise the `path.dirname(id)` command could fail. --- lib/internal/modules/cjs/loader.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 787d7fcd3520c5..9be6ecce17adc8 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -103,8 +103,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); From 9137c56963bee921273522dc91e6c7dd90cbaafa Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 28 Mar 2019 23:37:07 +0100 Subject: [PATCH 5/5] module: add extra caching layer This adds an extra modules caching layer that operates on the parent's `path` property and the current require argument. That together can be used as unique identifier to speed up loading the same module more than once. It is a cache on top of the current modules cache. It has the nice feature that this cache does not only work in the same file but it works for the whole current directory. So if the same file is loaded in any other file from the same directory, it will also hit this cache instead of having to resolve the file again. To keep it backwards compatible with the old modules cache, it detects invalidation of that cache. --- lib/internal/modules/cjs/loader.js | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 9be6ecce17adc8..115cdb7044f873 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -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) { @@ -598,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; } @@ -625,6 +641,9 @@ Module._load = function(request, parent, isMain) { } Module._cache[filename] = module; + if (parent !== undefined) { + relativeResolveCache[relResolveCacheIdentifier] = filename; + } let threw = true; try { @@ -633,6 +652,9 @@ Module._load = function(request, parent, isMain) { } finally { if (threw) { delete Module._cache[filename]; + if (parent !== undefined) { + delete relativeResolveCache[relResolveCacheIdentifier]; + } } }