Skip to content

Commit

Permalink
module: only set cache when finding module succeeds
Browse files Browse the repository at this point in the history
PR-URL: #36642
Fixes: #36638
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
ZYSzys committed Feb 12, 2021
1 parent f2c2615 commit aecd5eb
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 1 deletion.
80 changes: 80 additions & 0 deletions benchmark/module/module-require.js
@@ -0,0 +1,80 @@
'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, {
type: ['.js', '.json', 'dir'],
n: [1e4],
});

function main({ type, n }) {
tmpdir.refresh();
createEntryPoint(n);

switch (type) {
case '.js':
measureJSFile(n);
break;
case '.json':
measureJSONFile(n);
break;
case 'dir':
measureDir(n);
}

tmpdir.refresh();
}

function measureJSFile(n) {
bench.start();
for (let i = 0; i < n; i++) {
require(`${benchmarkDirectory}/${i}`);
}
bench.end(n);
}

function measureJSONFile(n) {
bench.start();
for (let i = 0; i < n; i++) {
require(`${benchmarkDirectory}/json_${i}.json`);
}
bench.end(n);
}

function measureDir(n) {
bench.start();
for (let i = 0; i < n; i++) {
require(`${benchmarkDirectory}${i}`);
}
bench.end(n);
}

function createEntryPoint(n) {
fs.mkdirSync(benchmarkDirectory);

const JSFileContent = 'module.exports = [];';
const JSONFileContent = '[]';

for (let i = 0; i < n; i++) {
// JS file.
fs.writeFileSync(`${benchmarkDirectory}/${i}.js`, JSFileContent);

// JSON file.
fs.writeFileSync(`${benchmarkDirectory}/json_${i}.json`, JSONFileContent);

// Dir.
fs.mkdirSync(`${benchmarkDirectory}${i}`);
fs.writeFileSync(
`${benchmarkDirectory}${i}/package.json`,
'{"main": "index.js"}'
);
fs.writeFileSync(
`${benchmarkDirectory}${i}/index.js`,
JSFileContent
);
}
}
5 changes: 4 additions & 1 deletion lib/internal/modules/cjs/loader.js
Expand Up @@ -148,7 +148,10 @@ function stat(filename) {
if (result !== undefined) return result;
}
const result = internalModuleStat(filename);
if (statCache !== null) statCache.set(filename, result);
if (statCache !== null && result >= 0) {
// Only set cache when `internalModuleStat(filename)` succeeds.
statCache.set(filename, result);
}
return result;
}

Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-module-cache.js
@@ -0,0 +1,19 @@
'use strict';
require('../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

const filePath = path.join(tmpdir.path, 'test-module-cache.json');
assert.throws(
() => require(filePath),
{ code: 'MODULE_NOT_FOUND' }
);

fs.writeFileSync(filePath, '[]');

const content = require(filePath);
assert.strictEqual(Array.isArray(content), true);
assert.strictEqual(content.length, 0);

0 comments on commit aecd5eb

Please sign in to comment.