Skip to content

Commit

Permalink
module: cache stat() results more aggressively
Browse files Browse the repository at this point in the history
Reduce the number of stat() system calls that require() makes by caching
the results more aggressively.

To avoid unbounded growth without implementing a LRU cache, scope the
cache to the lifetime of the first call to require().  Recursive calls
(i.e. require() calls in the included code) transparently profit from
the cache.

The benchmarked application is the loopback-sample-app[0] and it sees
the number of stat calls at start-up go down by 40%, from 4736 to 2810.

[0] https://github.com/strongloop/loopback-sample-app

PR-URL: #4575
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
bnoordhuis authored and evanlucas committed Jan 20, 2016
1 parent 14061c6 commit eb2b8c6
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 6 deletions.
11 changes: 9 additions & 2 deletions lib/internal/module.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
'use strict';

module.exports = { makeRequireFunction, stripBOM };
exports = module.exports = { makeRequireFunction, stripBOM };

exports.requireDepth = 0;

// Invoke with makeRequireFunction.call(module) where |module| is the
// Module object to use as the context for the require() function.
Expand All @@ -9,7 +11,12 @@ function makeRequireFunction() {
const self = this;

function require(path) {
return self.require(path);
try {
exports.requireDepth += 1;
return self.require(path);
} finally {
exports.requireDepth -= 1;
}
}

require.resolve = function(request) {
Expand Down
26 changes: 22 additions & 4 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ function hasOwnProperty(obj, prop) {
}


function stat(filename) {
filename = path._makeLong(filename);
const cache = stat.cache;
if (cache !== null) {
const result = cache.get(filename);
if (result !== undefined) return result;
}
const result = internalModuleStat(filename);
if (cache !== null) cache.set(filename, result);
return result;
}
stat.cache = null;


function Module(id, parent) {
this.id = id;
this.exports = {};
Expand Down Expand Up @@ -104,7 +118,7 @@ Module._realpathCache = {};

// check if the file exists and is not a directory
function tryFile(requestPath) {
const rc = internalModuleStat(path._makeLong(requestPath));
const rc = stat(requestPath);
return rc === 0 && toRealPath(requestPath);
}

Expand Down Expand Up @@ -141,12 +155,12 @@ Module._findPath = function(request, paths) {
// For each path
for (var i = 0, PL = paths.length; i < PL; i++) {
// Don't search further if path doesn't exist
if (paths[i] && internalModuleStat(path._makeLong(paths[i])) < 1) continue;
if (paths[i] && stat(paths[i]) < 1) continue;
var basePath = path.resolve(paths[i], request);
var filename;

if (!trailingSlash) {
const rc = internalModuleStat(path._makeLong(basePath));
const rc = stat(basePath);
if (rc === 0) { // File.
filename = toRealPath(basePath);
} else if (rc === 1) { // Directory.
Expand Down Expand Up @@ -394,7 +408,11 @@ Module.prototype._compile = function(content, filename) {
const dirname = path.dirname(filename);
const require = internalModule.makeRequireFunction.call(this);
const args = [this.exports, require, this, filename, dirname];
return compiledWrapper.apply(this.exports, args);
const depth = internalModule.requireDepth;
if (depth === 0) stat.cache = new Map();
const result = compiledWrapper.apply(this.exports, args);
if (depth === 0) stat.cache = null;
return result;
};


Expand Down
9 changes: 9 additions & 0 deletions test/fixtures/module-require-depth/one.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Flags: --expose_internals
'use strict';
const assert = require('assert');
const internalModule = require('internal/module');

exports.requireDepth = internalModule.requireDepth;
assert.strictEqual(internalModule.requireDepth, 1);
assert.deepStrictEqual(require('./two'), { requireDepth: 2 });
assert.strictEqual(internalModule.requireDepth, 1);
9 changes: 9 additions & 0 deletions test/fixtures/module-require-depth/two.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Flags: --expose_internals
'use strict';
const assert = require('assert');
const internalModule = require('internal/module');

exports.requireDepth = internalModule.requireDepth;
assert.strictEqual(internalModule.requireDepth, 2);
assert.deepStrictEqual(require('./one'), { requireDepth: 1 });
assert.strictEqual(internalModule.requireDepth, 2);
13 changes: 13 additions & 0 deletions test/parallel/test-module-require-depth.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Flags: --expose_internals
'use strict';
const common = require('../common');
const assert = require('assert');
const internalModule = require('internal/module');

// Module one loads two too so the expected depth for two is, well, two.
assert.strictEqual(internalModule.requireDepth, 0);
const one = require(common.fixturesDir + '/module-require-depth/one');
const two = require(common.fixturesDir + '/module-require-depth/two');
assert.deepStrictEqual(one, { requireDepth: 1 });
assert.deepStrictEqual(two, { requireDepth: 2 });
assert.strictEqual(internalModule.requireDepth, 0);

0 comments on commit eb2b8c6

Please sign in to comment.