From 020ba8bae20a1f97b99f39b02b593a1157fe674c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 14 Aug 2016 18:41:28 +0200 Subject: [PATCH 1/5] fs,module: add module-loader-only realpath cache Reintroduce a realpath cache with the same mechanisms which existed before b488b19eaf2b2e7a3ca5eccd2445e245847a5f76 (`fs: optimize realpath using uv_fs_realpath()`), but only for the synchronous version and with the cache being passed as a hidden option to make sure it is only used internally. The cache is hidden from userland applications because it has been decided that fully reintroducing as part of the public API might stand in the way of future optimizations. --- lib/fs.js | 56 +++++++++++++++++++++++++++++++++------------------ lib/module.js | 18 +++++++++++++++-- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 6e28a41118e131..f9f5714fff4267 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1560,6 +1560,10 @@ function encodeRealpathResult(result, options, err) { } } +// This is removed from the fs exports in lib/module.js in order to make +// sure that this stays internal. +const realpathCacheKey = fs.realpathCacheKey = Symbol('realpathCacheKey'); + fs.realpathSync = function realpathSync(p, options) { if (!options) options = {}; @@ -1574,6 +1578,12 @@ fs.realpathSync = function realpathSync(p, options) { const seenLinks = {}; const knownHard = {}; + const cache = options[realpathCacheKey]; + const original = p; + + if (cache && cache.has(p)) { + return cache.get(p); + } // current character position in p var pos; @@ -1614,39 +1624,45 @@ fs.realpathSync = function realpathSync(p, options) { pos = nextPartRe.lastIndex; // continue if not a symlink - if (knownHard[base]) { + if (knownHard[base] || (cache && cache.get(base) === base)) { continue; } var resolvedLink; - var stat = fs.lstatSync(base); - if (!stat.isSymbolicLink()) { - knownHard[base] = true; - continue; - } + if (cache && cache.has(base)) { + resolvedLink = cache.get(base); + } else { + var stat = fs.lstatSync(base); + if (!stat.isSymbolicLink()) { + knownHard[base] = true; + continue; + } - // read the link if it wasn't read before - // dev/ino always return 0 on windows, so skip the check. - var linkTarget = null; - if (!isWindows) { - var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); - if (seenLinks.hasOwnProperty(id)) { - linkTarget = seenLinks[id]; + // read the link if it wasn't read before + // dev/ino always return 0 on windows, so skip the check. + var linkTarget = null; + if (!isWindows) { + var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); + if (seenLinks.hasOwnProperty(id)) { + linkTarget = seenLinks[id]; + } } - } - if (linkTarget === null) { - fs.statSync(base); - linkTarget = fs.readlinkSync(base); - } - resolvedLink = pathModule.resolve(previous, linkTarget); + if (linkTarget === null) { + fs.statSync(base); + linkTarget = fs.readlinkSync(base); + } + resolvedLink = pathModule.resolve(previous, linkTarget); - if (!isWindows) seenLinks[id] = linkTarget; + if (cache) cache.set(base, resolvedLink); + if (!isWindows) seenLinks[id] = linkTarget; + } // resolve the link, then start over p = pathModule.resolve(resolvedLink, p.slice(pos)); start(); } + if (cache) cache.set(original, p); return encodeRealpathResult(p, options); }; diff --git a/lib/module.js b/lib/module.js index 96f1cfc42e47c6..deda06635a4f91 100644 --- a/lib/module.js +++ b/lib/module.js @@ -109,6 +109,14 @@ function tryPackage(requestPath, exts, isMain) { tryExtensions(path.resolve(filename, 'index'), exts, isMain); } +// In order to minimize unnecessary lstat() calls, +// this cache is a list of known-real paths. +// Set to an empty object to reset. +Module._realpathCache = new Map(); + +const realpathCacheKey = fs.realpathCacheKey; +delete fs.realpathCacheKey; + // check if the file exists and is not a directory // if using --preserve-symlinks and isMain is false, // keep symlinks intact, otherwise resolve to the @@ -118,7 +126,13 @@ function tryFile(requestPath, isMain) { if (preserveSymlinks && !isMain) { return rc === 0 && path.resolve(requestPath); } - return rc === 0 && fs.realpathSync(requestPath); + return rc === 0 && toRealPath(requestPath); +} + +function toRealPath(requestPath) { + return fs.realpathSync(requestPath, { + [realpathCacheKey]: Module._realpathCache + }); } // given a path check a the file exists with any of the set extensions @@ -164,7 +178,7 @@ Module._findPath = function(request, paths, isMain) { if (preserveSymlinks && !isMain) { filename = path.resolve(basePath); } else { - filename = fs.realpathSync(basePath); + filename = toRealPath(basePath); } } else if (rc === 1) { // Directory. if (exts === undefined) From 434424f18ef9ca2ba855c5609c59294f45eed980 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 14 Aug 2016 19:53:00 +0200 Subject: [PATCH 2/5] Replace `cache.has` with `.get()` & check. --- lib/fs.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index f9f5714fff4267..1b494ce1984b9f 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1581,8 +1581,9 @@ fs.realpathSync = function realpathSync(p, options) { const cache = options[realpathCacheKey]; const original = p; - if (cache && cache.has(p)) { - return cache.get(p); + const maybeCachedResult = cache && cache.get(p); + if (maybeCachedResult) { + return maybeCachedResult; } // current character position in p @@ -1629,8 +1630,9 @@ fs.realpathSync = function realpathSync(p, options) { } var resolvedLink; - if (cache && cache.has(base)) { - resolvedLink = cache.get(base); + const maybeCachedResolved = cache && cache.get(base); + if (maybeCachedResolved) { + resolvedLink = maybeCachedResolved; } else { var stat = fs.lstatSync(base); if (!stat.isSymbolicLink()) { From 7f0c2245355311b14b47e2e6f735811c363660f8 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 18 Aug 2016 03:42:42 +0200 Subject: [PATCH 3/5] Use local `const` instead of property on `Module` object --- lib/module.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/module.js b/lib/module.js index deda06635a4f91..3460f4fb2cee54 100644 --- a/lib/module.js +++ b/lib/module.js @@ -112,7 +112,7 @@ function tryPackage(requestPath, exts, isMain) { // In order to minimize unnecessary lstat() calls, // this cache is a list of known-real paths. // Set to an empty object to reset. -Module._realpathCache = new Map(); +const realpathCache = new Map(); const realpathCacheKey = fs.realpathCacheKey; delete fs.realpathCacheKey; @@ -131,7 +131,7 @@ function tryFile(requestPath, isMain) { function toRealPath(requestPath) { return fs.realpathSync(requestPath, { - [realpathCacheKey]: Module._realpathCache + [realpathCacheKey]: realpathCache }); } From f3e8373c00893c18a5157ebd45c8fc766762f06c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 10 Sep 2016 12:05:33 +0200 Subject: [PATCH 4/5] Address review comments --- lib/fs.js | 4 ++-- lib/module.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 1b494ce1984b9f..e27cbbd77f1b5e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1644,7 +1644,7 @@ fs.realpathSync = function realpathSync(p, options) { // dev/ino always return 0 on windows, so skip the check. var linkTarget = null; if (!isWindows) { - var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); + const id = `${stat.dev.toString(32)}:${stat.ino.toString(32)}`; if (seenLinks.hasOwnProperty(id)) { linkTarget = seenLinks[id]; } @@ -1761,7 +1761,7 @@ fs.realpath = function realpath(p, options, callback) { // call gotTarget as soon as the link target is known // dev/ino always return 0 on windows, so skip the check. if (!isWindows) { - var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); + const id = `${stat.dev.toString(32)}:${stat.ino.toString(32)}`; if (seenLinks.hasOwnProperty(id)) { return gotTarget(null, seenLinks[id], base); } diff --git a/lib/module.js b/lib/module.js index 3460f4fb2cee54..c61a27a9e3cd1f 100644 --- a/lib/module.js +++ b/lib/module.js @@ -111,7 +111,7 @@ function tryPackage(requestPath, exts, isMain) { // In order to minimize unnecessary lstat() calls, // this cache is a list of known-real paths. -// Set to an empty object to reset. +// Set to an empty Map to reset. const realpathCache = new Map(); const realpathCacheKey = fs.realpathCacheKey; From c9954f4b342ceef7daa2675f23286dd2ecf89fcc Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 10 Sep 2016 12:11:19 +0200 Subject: [PATCH 5/5] fixup: `id` cannot be const --- lib/fs.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index e27cbbd77f1b5e..2358e98f04ddca 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1642,9 +1642,10 @@ fs.realpathSync = function realpathSync(p, options) { // read the link if it wasn't read before // dev/ino always return 0 on windows, so skip the check. - var linkTarget = null; + let linkTarget = null; + let id; if (!isWindows) { - const id = `${stat.dev.toString(32)}:${stat.ino.toString(32)}`; + id = `${stat.dev.toString(32)}:${stat.ino.toString(32)}`; if (seenLinks.hasOwnProperty(id)) { linkTarget = seenLinks[id]; } @@ -1760,8 +1761,9 @@ fs.realpath = function realpath(p, options, callback) { // stat & read the link if not read before // call gotTarget as soon as the link target is known // dev/ino always return 0 on windows, so skip the check. + let id; if (!isWindows) { - const id = `${stat.dev.toString(32)}:${stat.ino.toString(32)}`; + id = `${stat.dev.toString(32)}:${stat.ino.toString(32)}`; if (seenLinks.hasOwnProperty(id)) { return gotTarget(null, seenLinks[id], base); }