Commit 262f0ec

Browse files
codebytereaduh95
authored andcommitted
fs: stop stat()ing every entry in recursive readdir
readdir({ recursive: true }) asked the binding for names only and then called internalModuleStat() on every entry to find the directories to descend into; with withFileTypes it built the Dirents and still stat()ed every entry that was not already a directory. Both variants also ran path.join() and path.relative() per entry to build the relative result. Ask the binding for file types in all cases, descend into directories directly, and only stat() symbolic links and entries of unknown type (which is what could point to a directory). The relative name is the parent's prefix plus the entry name. Results, their order and the symlink-following behavior are unchanged for fs.readdirSync, fs.readdir and fs.promises.readdir. The known_issues test for Buffer paths (#58892) called back without checking the error; the error now reaches the callback instead of being thrown from the completion handler, so the test asserts success to keep expressing the issue. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65487 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 5b7d02e commit 262f0ec

5 files changed

Lines changed: 112 additions & 150 deletions

File tree

‎benchmark/fs/bench-readdir.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

13-
functionmain({ n, dir, withFileTypes }){
14+
functionmain({ n, dir, withFileTypes, recursive}){
1415
withFileTypes=withFileTypes==='true';
16+
recursive=recursive==='true';
1517
constfullPath=path.resolve(__dirname,'../../',dir);
1618
bench.start();
1719
(functionr(cntr){
1820
if(cntr--<=0)
1921
returnbench.end(n);
20-
fs.readdir(fullPath,{ withFileTypes },()=>{
22+
fs.readdir(fullPath,{ withFileTypes, recursive},()=>{
2123
r(cntr);
2224
});
2325
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

1314

14-
functionmain({ n, dir, withFileTypes }){
15+
functionmain({ n, dir, withFileTypes, recursive}){
1516
withFileTypes=withFileTypes==='true';
17+
recursive=recursive==='true';
1618
constfullPath=path.resolve(__dirname,'../../',dir);
1719
bench.start();
1820
for(leti=0;i<n;i++){
19-
fs.readdirSync(fullPath,{ withFileTypes });
21+
fs.readdirSync(fullPath,{ withFileTypes, recursive});
2022
}
2123
bench.end(n);
2224
}

‎lib/fs.js‎

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ const {
5757
F_OK,
5858
O_WRONLY,
5959
O_SYMLINK,
60+
UV_DIRENT_DIR,
61+
UV_DIRENT_LINK,
62+
UV_DIRENT_UNKNOWN,
6063
}=constants;
6164

6265
constpathModule=require('path');
@@ -1739,6 +1742,43 @@ function mkdirSync(path, options) {
17391742
}
17401743
}
17411744

1745+
/**
1746+
* Appends one directory's entries to `context.results` and the subdirectories
1747+
* still to visit to `context.dirs` (with the prefix their entries get in
1748+
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1749+
* result with file types, so only symbolic links and entries of unknown type
1750+
* need a stat() to find out whether they lead to a directory.
1751+
* @param {string} dir
1752+
* @param {string} prefix
1753+
* @param {[string[], number[]]} result
1754+
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1755+
*/
1756+
functioncollectRecursiveReaddirResult(dir,prefix,{0: names,1: types},context){
1757+
const{ length }=names;
1758+
for(leti=0;i<length;i++){
1759+
constname=names[i];
1760+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1761+
letisDirectory;
1762+
if(context.withFileTypes){
1763+
constdirent=getDirent(dir,name,types[i]);
1764+
ArrayPrototypePush(context.results,dirent);
1765+
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1766+
isDirectory=dirent.isDirectory()||
1767+
(dirent.isSymbolicLink()&&binding.internalModuleStat(pathModule.join(dir,name))===1);
1768+
}else{
1769+
ArrayPrototypePush(context.results,relative);
1770+
consttype=types[i];
1771+
isDirectory=type===UV_DIRENT_DIR||
1772+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1773+
binding.internalModuleStat(pathModule.join(dir,name))===1);
1774+
}
1775+
if(isDirectory){
1776+
ArrayPrototypePush(context.dirs,pathModule.join(dir,name));
1777+
ArrayPrototypePush(context.prefixes,relative);
1778+
}
1779+
}
1780+
}
1781+
17421782
/*
17431783
* An recursive algorithm for reading the entire contents of the `basePath` directory.
17441784
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1754,15 +1794,20 @@ function mkdirSync(path, options) {
17541794
functionreaddirRecursive(basePath,options,callback){
17551795
constcontext={
17561796
withFileTypes: Boolean(options.withFileTypes),
1757-
encoding: options.encoding,
1758-
basePath,
1759-
readdirResults: [],
1760-
pathsQueue: [basePath],
1797+
results: [],
1798+
dirs: [basePath],
1799+
prefixes: [''],
17611800
};
17621801

17631802
leti=0;
17641803

1765-
functionread(path){
1804+
/**
1805+
* Reads one directory from `context.dirs` and then moves on to the next
1806+
* one, or calls back once none are left.
1807+
* @param {string} path
1808+
* @param {string} prefix path of this directory relative to `basePath`
1809+
*/
1810+
functionread(path,prefix){
17661811
constreq=newFSReqCallback();
17671812
req.oncomplete=(err,result)=>{
17681813
if(err){
@@ -1771,68 +1816,28 @@ function readdirRecursive(basePath, options, callback) {
17711816
}
17721817

17731818
if(result===undefined){
1774-
callback(null,context.readdirResults);
1819+
callback(null,context.results);
17751820
return;
17761821
}
17771822

1778-
processReaddirResult({
1779-
result,
1780-
currentPath: path,
1781-
context,
1782-
});
1823+
try{
1824+
collectRecursiveReaddirResult(path,prefix,result,context);
1825+
}catch(err){
1826+
callback(err);
1827+
return;
1828+
}
17831829

1784-
if(i<context.pathsQueue.length){
1785-
read(context.pathsQueue[i++]);
1830+
if(i<context.dirs.length){
1831+
read(context.dirs[i],context.prefixes[i++]);
17861832
}else{
1787-
callback(null,context.readdirResults);
1833+
callback(null,context.results);
17881834
}
17891835
};
17901836

1791-
binding.readdir(
1792-
path,
1793-
context.encoding,
1794-
context.withFileTypes,
1795-
req,
1796-
);
1797-
}
1798-
1799-
read(context.pathsQueue[i++]);
1800-
}
1801-
1802-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1803-
// The first array is the names, and the second array is the types.
1804-
// They are guaranteed to be the same length; hence, setting `length` to the length
1805-
// of the first array within the result.
1806-
constprocessReaddirResult=(args)=>(args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
1807-
1808-
functionhandleDirents({ result, currentPath, context }){
1809-
const{0: names,1: types}=result;
1810-
const{ length }=names;
1811-
1812-
for(leti=0;i<length;i++){
1813-
// Avoid excluding symlinks, as they are not directories.
1814-
// Refs: https://github.com/nodejs/node/issues/52663
1815-
constfullPath=pathModule.join(currentPath,names[i]);
1816-
constdirent=getDirent(currentPath,names[i],types[i]);
1817-
ArrayPrototypePush(context.readdirResults,dirent);
1818-
1819-
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
1820-
ArrayPrototypePush(context.pathsQueue,fullPath);
1821-
}
1837+
binding.readdir(path,options.encoding,true,req);
18221838
}
1823-
}
1824-
1825-
functionhandleFilePaths({ result, currentPath, context }){
1826-
for(leti=0;i<result.length;i++){
1827-
constresultPath=pathModule.join(currentPath,result[i]);
1828-
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1829-
conststat=binding.internalModuleStat(resultPath);
1830-
ArrayPrototypePush(context.readdirResults,relativeResultPath);
18311839

1832-
if(stat===1){
1833-
ArrayPrototypePush(context.pathsQueue,resultPath);
1834-
}
1835-
}
1840+
read(context.dirs[i],context.prefixes[i++]);
18361841
}
18371842

18381843
/**
@@ -1846,35 +1851,20 @@ function handleFilePaths({ result, currentPath, context }) {
18461851
functionreaddirSyncRecursive(basePath,options){
18471852
constcontext={
18481853
withFileTypes: Boolean(options.withFileTypes),
1849-
encoding: options.encoding,
1850-
basePath,
1851-
readdirResults: [],
1852-
pathsQueue: [basePath],
1854+
results: [],
1855+
dirs: [basePath],
1856+
prefixes: [''],
18531857
};
18541858

1855-
functionread(path){
1856-
constreaddirResult=binding.readdir(
1857-
path,
1858-
context.encoding,
1859-
context.withFileTypes,
1860-
);
1861-
1862-
if(readdirResult===undefined){
1863-
return;
1859+
for(leti=0;i<context.dirs.length;i++){
1860+
constdir=context.dirs[i];
1861+
constresult=binding.readdir(dir,options.encoding,true);
1862+
if(result!==undefined){
1863+
collectRecursiveReaddirResult(dir,context.prefixes[i],result,context);
18641864
}
1865-
1866-
processReaddirResult({
1867-
result: readdirResult,
1868-
currentPath: path,
1869-
context,
1870-
});
1871-
}
1872-
1873-
for(leti=0;i<context.pathsQueue.length;i++){
1874-
read(context.pathsQueue[i]);
18751865
}
18761866

1877-
returncontext.readdirResults;
1867+
returncontext.results;
18781868
}
18791869

18801870
/**

‎lib/internal/fs/promises.js‎

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const {
3232
O_WRONLY,
3333
S_IFMT,
3434
S_IFREG,
35+
UV_DIRENT_DIR,
36+
UV_DIRENT_LINK,
37+
UV_DIRENT_UNKNOWN,
3538
}=constants;
3639

3740
constbinding=internalBinding('fs');
@@ -63,6 +66,7 @@ const {
6366
kWriteFileMaxChunkSize,
6467
},
6568
copyObject,
69+
getDirent,
6670
getDirents,
6771
getOptions,
6872
getStatFsFromBinding,
@@ -1638,73 +1642,37 @@ async function mkdir(path, options) {
16381642
}
16391643

16401644
asyncfunctionreaddirRecursive(originalPath,options){
1645+
constwithFileTypes=!!options.withFileTypes;
1646+
constreaddirWithTypes=(path)=>PromisePrototypeThen(
1647+
binding.readdir(path,options.encoding,true,kUsePromises),
1648+
undefined,
1649+
handleErrorFromBinding,
1650+
);
16411651
constresult=[];
1642-
constqueue=[
1643-
[
1644-
originalPath,
1645-
awaitPromisePrototypeThen(
1646-
binding.readdir(
1647-
originalPath,
1648-
options.encoding,
1649-
!!options.withFileTypes,
1650-
kUsePromises,
1651-
),
1652-
undefined,
1653-
handleErrorFromBinding,
1654-
),
1655-
],
1656-
];
1657-
1658-
1659-
if(options.withFileTypes){
1660-
while(queue.length>0){
1661-
// If we want to implement BFS make this a `shift` call instead of `pop`
1662-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1663-
for(constdirentofgetDirents(path,readdir)){
1652+
constqueue=[[originalPath,'',awaitreaddirWithTypes(originalPath)]];
1653+
1654+
while(queue.length>0){
1655+
// If we want to implement BFS make this a `shift` call instead of `pop`
1656+
const{0: path,1: prefix,2: {0: names,1: types}}=ArrayPrototypePop(queue);
1657+
for(leti=0;i<names.length;i++){
1658+
constname=names[i];
1659+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1660+
letisDirectory;
1661+
if(withFileTypes){
1662+
constdirent=getDirent(path,name,types[i]);
16641663
ArrayPrototypePush(result,dirent);
1665-
if(dirent.isDirectory()){
1666-
constdirentPath=pathModule.join(path,dirent.name);
1667-
ArrayPrototypePush(queue,[
1668-
direntPath,
1669-
awaitPromisePrototypeThen(
1670-
binding.readdir(
1671-
direntPath,
1672-
options.encoding,
1673-
true,
1674-
kUsePromises,
1675-
),
1676-
undefined,
1677-
handleErrorFromBinding,
1678-
),
1679-
]);
1680-
}
1664+
isDirectory=dirent.isDirectory();
1665+
}else{
1666+
ArrayPrototypePush(result,relative);
1667+
// Entries that are, or may be, symbolic links to directories are followed.
1668+
consttype=types[i];
1669+
isDirectory=type===UV_DIRENT_DIR||
1670+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1671+
binding.internalModuleStat(pathModule.join(path,name))===1);
16811672
}
1682-
}
1683-
}else{
1684-
while(queue.length>0){
1685-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1686-
for(constentofreaddir){
1687-
constdirentPath=pathModule.join(path,ent);
1688-
conststat=binding.internalModuleStat(direntPath);
1689-
ArrayPrototypePush(
1690-
result,
1691-
pathModule.relative(originalPath,direntPath),
1692-
);
1693-
if(stat===1){
1694-
ArrayPrototypePush(queue,[
1695-
direntPath,
1696-
awaitPromisePrototypeThen(
1697-
binding.readdir(
1698-
direntPath,
1699-
options.encoding,
1700-
false,
1701-
kUsePromises,
1702-
),
1703-
undefined,
1704-
handleErrorFromBinding,
1705-
),
1706-
]);
1707-
}
1673+
if(isDirectory){
1674+
constdirentPath=pathModule.join(path,name);
1675+
ArrayPrototypePush(queue,[direntPath,relative,awaitreaddirWithTypes(direntPath)]);
17081676
}
17091677
}
17101678
}

‎test/known_issues/test-fs-readdir-recursive-with-buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const { readdir } = require('node:fs');
1212
const{ join }=require('node:path');
1313

1414
consttestDirPath=join(__dirname,'..','..');
15-
readdir(Buffer.from(testDirPath),{recursive: true},common.mustCall());
15+
readdir(Buffer.from(testDirPath),{recursive: true},common.mustSucceed());

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Commit 262f0ec

Browse files
codebytereaduh95
authored andcommitted
fs: stop stat()ing every entry in recursive readdir
readdir({ recursive: true }) asked the binding for names only and then called internalModuleStat() on every entry to find the directories to descend into; with withFileTypes it built the Dirents and still stat()ed every entry that was not already a directory. Both variants also ran path.join() and path.relative() per entry to build the relative result. Ask the binding for file types in all cases, descend into directories directly, and only stat() symbolic links and entries of unknown type (which is what could point to a directory). The relative name is the parent's prefix plus the entry name. Results, their order and the symlink-following behavior are unchanged for fs.readdirSync, fs.readdir and fs.promises.readdir. The known_issues test for Buffer paths (#58892) called back without checking the error; the error now reaches the callback instead of being thrown from the completion handler, so the test asserts success to keep expressing the issue. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65487 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 5b7d02e commit 262f0ec

5 files changed

Lines changed: 112 additions & 150 deletions

File tree

‎benchmark/fs/bench-readdir.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

13-
functionmain({ n, dir, withFileTypes }){
14+
functionmain({ n, dir, withFileTypes, recursive}){
1415
withFileTypes=withFileTypes==='true';
16+
recursive=recursive==='true';
1517
constfullPath=path.resolve(__dirname,'../../',dir);
1618
bench.start();
1719
(functionr(cntr){
1820
if(cntr--<=0)
1921
returnbench.end(n);
20-
fs.readdir(fullPath,{ withFileTypes },()=>{
22+
fs.readdir(fullPath,{ withFileTypes, recursive},()=>{
2123
r(cntr);
2224
});
2325
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

1314

14-
functionmain({ n, dir, withFileTypes }){
15+
functionmain({ n, dir, withFileTypes, recursive}){
1516
withFileTypes=withFileTypes==='true';
17+
recursive=recursive==='true';
1618
constfullPath=path.resolve(__dirname,'../../',dir);
1719
bench.start();
1820
for(leti=0;i<n;i++){
19-
fs.readdirSync(fullPath,{ withFileTypes });
21+
fs.readdirSync(fullPath,{ withFileTypes, recursive});
2022
}
2123
bench.end(n);
2224
}

‎lib/fs.js‎

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ const {
5757
F_OK,
5858
O_WRONLY,
5959
O_SYMLINK,
60+
UV_DIRENT_DIR,
61+
UV_DIRENT_LINK,
62+
UV_DIRENT_UNKNOWN,
6063
}=constants;
6164

6265
constpathModule=require('path');
@@ -1739,6 +1742,43 @@ function mkdirSync(path, options) {
17391742
}
17401743
}
17411744

1745+
/**
1746+
* Appends one directory's entries to `context.results` and the subdirectories
1747+
* still to visit to `context.dirs` (with the prefix their entries get in
1748+
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1749+
* result with file types, so only symbolic links and entries of unknown type
1750+
* need a stat() to find out whether they lead to a directory.
1751+
* @param {string} dir
1752+
* @param {string} prefix
1753+
* @param {[string[], number[]]} result
1754+
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1755+
*/
1756+
functioncollectRecursiveReaddirResult(dir,prefix,{0: names,1: types},context){
1757+
const{ length }=names;
1758+
for(leti=0;i<length;i++){
1759+
constname=names[i];
1760+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1761+
letisDirectory;
1762+
if(context.withFileTypes){
1763+
constdirent=getDirent(dir,name,types[i]);
1764+
ArrayPrototypePush(context.results,dirent);
1765+
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1766+
isDirectory=dirent.isDirectory()||
1767+
(dirent.isSymbolicLink()&&binding.internalModuleStat(pathModule.join(dir,name))===1);
1768+
}else{
1769+
ArrayPrototypePush(context.results,relative);
1770+
consttype=types[i];
1771+
isDirectory=type===UV_DIRENT_DIR||
1772+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1773+
binding.internalModuleStat(pathModule.join(dir,name))===1);
1774+
}
1775+
if(isDirectory){
1776+
ArrayPrototypePush(context.dirs,pathModule.join(dir,name));
1777+
ArrayPrototypePush(context.prefixes,relative);
1778+
}
1779+
}
1780+
}
1781+
17421782
/*
17431783
* An recursive algorithm for reading the entire contents of the `basePath` directory.
17441784
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1754,15 +1794,20 @@ function mkdirSync(path, options) {
17541794
functionreaddirRecursive(basePath,options,callback){
17551795
constcontext={
17561796
withFileTypes: Boolean(options.withFileTypes),
1757-
encoding: options.encoding,
1758-
basePath,
1759-
readdirResults: [],
1760-
pathsQueue: [basePath],
1797+
results: [],
1798+
dirs: [basePath],
1799+
prefixes: [''],
17611800
};
17621801

17631802
leti=0;
17641803

1765-
functionread(path){
1804+
/**
1805+
* Reads one directory from `context.dirs` and then moves on to the next
1806+
* one, or calls back once none are left.
1807+
* @param {string} path
1808+
* @param {string} prefix path of this directory relative to `basePath`
1809+
*/
1810+
functionread(path,prefix){
17661811
constreq=newFSReqCallback();
17671812
req.oncomplete=(err,result)=>{
17681813
if(err){
@@ -1771,68 +1816,28 @@ function readdirRecursive(basePath, options, callback) {
17711816
}
17721817

17731818
if(result===undefined){
1774-
callback(null,context.readdirResults);
1819+
callback(null,context.results);
17751820
return;
17761821
}
17771822

1778-
processReaddirResult({
1779-
result,
1780-
currentPath: path,
1781-
context,
1782-
});
1823+
try{
1824+
collectRecursiveReaddirResult(path,prefix,result,context);
1825+
}catch(err){
1826+
callback(err);
1827+
return;
1828+
}
17831829

1784-
if(i<context.pathsQueue.length){
1785-
read(context.pathsQueue[i++]);
1830+
if(i<context.dirs.length){
1831+
read(context.dirs[i],context.prefixes[i++]);
17861832
}else{
1787-
callback(null,context.readdirResults);
1833+
callback(null,context.results);
17881834
}
17891835
};
17901836

1791-
binding.readdir(
1792-
path,
1793-
context.encoding,
1794-
context.withFileTypes,
1795-
req,
1796-
);
1797-
}
1798-
1799-
read(context.pathsQueue[i++]);
1800-
}
1801-
1802-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1803-
// The first array is the names, and the second array is the types.
1804-
// They are guaranteed to be the same length; hence, setting `length` to the length
1805-
// of the first array within the result.
1806-
constprocessReaddirResult=(args)=>(args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
1807-
1808-
functionhandleDirents({ result, currentPath, context }){
1809-
const{0: names,1: types}=result;
1810-
const{ length }=names;
1811-
1812-
for(leti=0;i<length;i++){
1813-
// Avoid excluding symlinks, as they are not directories.
1814-
// Refs: https://github.com/nodejs/node/issues/52663
1815-
constfullPath=pathModule.join(currentPath,names[i]);
1816-
constdirent=getDirent(currentPath,names[i],types[i]);
1817-
ArrayPrototypePush(context.readdirResults,dirent);
1818-
1819-
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
1820-
ArrayPrototypePush(context.pathsQueue,fullPath);
1821-
}
1837+
binding.readdir(path,options.encoding,true,req);
18221838
}
1823-
}
1824-
1825-
functionhandleFilePaths({ result, currentPath, context }){
1826-
for(leti=0;i<result.length;i++){
1827-
constresultPath=pathModule.join(currentPath,result[i]);
1828-
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1829-
conststat=binding.internalModuleStat(resultPath);
1830-
ArrayPrototypePush(context.readdirResults,relativeResultPath);
18311839

1832-
if(stat===1){
1833-
ArrayPrototypePush(context.pathsQueue,resultPath);
1834-
}
1835-
}
1840+
read(context.dirs[i],context.prefixes[i++]);
18361841
}
18371842

18381843
/**
@@ -1846,35 +1851,20 @@ function handleFilePaths({ result, currentPath, context }) {
18461851
functionreaddirSyncRecursive(basePath,options){
18471852
constcontext={
18481853
withFileTypes: Boolean(options.withFileTypes),
1849-
encoding: options.encoding,
1850-
basePath,
1851-
readdirResults: [],
1852-
pathsQueue: [basePath],
1854+
results: [],
1855+
dirs: [basePath],
1856+
prefixes: [''],
18531857
};
18541858

1855-
functionread(path){
1856-
constreaddirResult=binding.readdir(
1857-
path,
1858-
context.encoding,
1859-
context.withFileTypes,
1860-
);
1861-
1862-
if(readdirResult===undefined){
1863-
return;
1859+
for(leti=0;i<context.dirs.length;i++){
1860+
constdir=context.dirs[i];
1861+
constresult=binding.readdir(dir,options.encoding,true);
1862+
if(result!==undefined){
1863+
collectRecursiveReaddirResult(dir,context.prefixes[i],result,context);
18641864
}
1865-
1866-
processReaddirResult({
1867-
result: readdirResult,
1868-
currentPath: path,
1869-
context,
1870-
});
1871-
}
1872-
1873-
for(leti=0;i<context.pathsQueue.length;i++){
1874-
read(context.pathsQueue[i]);
18751865
}
18761866

1877-
returncontext.readdirResults;
1867+
returncontext.results;
18781868
}
18791869

18801870
/**

‎lib/internal/fs/promises.js‎

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const {
3232
O_WRONLY,
3333
S_IFMT,
3434
S_IFREG,
35+
UV_DIRENT_DIR,
36+
UV_DIRENT_LINK,
37+
UV_DIRENT_UNKNOWN,
3538
}=constants;
3639

3740
constbinding=internalBinding('fs');
@@ -63,6 +66,7 @@ const {
6366
kWriteFileMaxChunkSize,
6467
},
6568
copyObject,
69+
getDirent,
6670
getDirents,
6771
getOptions,
6872
getStatFsFromBinding,
@@ -1638,73 +1642,37 @@ async function mkdir(path, options) {
16381642
}
16391643

16401644
asyncfunctionreaddirRecursive(originalPath,options){
1645+
constwithFileTypes=!!options.withFileTypes;
1646+
constreaddirWithTypes=(path)=>PromisePrototypeThen(
1647+
binding.readdir(path,options.encoding,true,kUsePromises),
1648+
undefined,
1649+
handleErrorFromBinding,
1650+
);
16411651
constresult=[];
1642-
constqueue=[
1643-
[
1644-
originalPath,
1645-
awaitPromisePrototypeThen(
1646-
binding.readdir(
1647-
originalPath,
1648-
options.encoding,
1649-
!!options.withFileTypes,
1650-
kUsePromises,
1651-
),
1652-
undefined,
1653-
handleErrorFromBinding,
1654-
),
1655-
],
1656-
];
1657-
1658-
1659-
if(options.withFileTypes){
1660-
while(queue.length>0){
1661-
// If we want to implement BFS make this a `shift` call instead of `pop`
1662-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1663-
for(constdirentofgetDirents(path,readdir)){
1652+
constqueue=[[originalPath,'',awaitreaddirWithTypes(originalPath)]];
1653+
1654+
while(queue.length>0){
1655+
// If we want to implement BFS make this a `shift` call instead of `pop`
1656+
const{0: path,1: prefix,2: {0: names,1: types}}=ArrayPrototypePop(queue);
1657+
for(leti=0;i<names.length;i++){
1658+
constname=names[i];
1659+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1660+
letisDirectory;
1661+
if(withFileTypes){
1662+
constdirent=getDirent(path,name,types[i]);
16641663
ArrayPrototypePush(result,dirent);
1665-
if(dirent.isDirectory()){
1666-
constdirentPath=pathModule.join(path,dirent.name);
1667-
ArrayPrototypePush(queue,[
1668-
direntPath,
1669-
awaitPromisePrototypeThen(
1670-
binding.readdir(
1671-
direntPath,
1672-
options.encoding,
1673-
true,
1674-
kUsePromises,
1675-
),
1676-
undefined,
1677-
handleErrorFromBinding,
1678-
),
1679-
]);
1680-
}
1664+
isDirectory=dirent.isDirectory();
1665+
}else{
1666+
ArrayPrototypePush(result,relative);
1667+
// Entries that are, or may be, symbolic links to directories are followed.
1668+
consttype=types[i];
1669+
isDirectory=type===UV_DIRENT_DIR||
1670+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1671+
binding.internalModuleStat(pathModule.join(path,name))===1);
16811672
}
1682-
}
1683-
}else{
1684-
while(queue.length>0){
1685-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1686-
for(constentofreaddir){
1687-
constdirentPath=pathModule.join(path,ent);
1688-
conststat=binding.internalModuleStat(direntPath);
1689-
ArrayPrototypePush(
1690-
result,
1691-
pathModule.relative(originalPath,direntPath),
1692-
);
1693-
if(stat===1){
1694-
ArrayPrototypePush(queue,[
1695-
direntPath,
1696-
awaitPromisePrototypeThen(
1697-
binding.readdir(
1698-
direntPath,
1699-
options.encoding,
1700-
false,
1701-
kUsePromises,
1702-
),
1703-
undefined,
1704-
handleErrorFromBinding,
1705-
),
1706-
]);
1707-
}
1673+
if(isDirectory){
1674+
constdirentPath=pathModule.join(path,name);
1675+
ArrayPrototypePush(queue,[direntPath,relative,awaitreaddirWithTypes(direntPath)]);
17081676
}
17091677
}
17101678
}

‎test/known_issues/test-fs-readdir-recursive-with-buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const { readdir } = require('node:fs');
1212
const{ join }=require('node:path');
1313

1414
consttestDirPath=join(__dirname,'..','..');
15-
readdir(Buffer.from(testDirPath),{recursive: true},common.mustCall());
15+
readdir(Buffer.from(testDirPath),{recursive: true},common.mustSucceed());

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 262f0ec

Browse files
codebytereaduh95
authored andcommitted
fs: stop stat()ing every entry in recursive readdir
readdir({ recursive: true }) asked the binding for names only and then called internalModuleStat() on every entry to find the directories to descend into; with withFileTypes it built the Dirents and still stat()ed every entry that was not already a directory. Both variants also ran path.join() and path.relative() per entry to build the relative result. Ask the binding for file types in all cases, descend into directories directly, and only stat() symbolic links and entries of unknown type (which is what could point to a directory). The relative name is the parent's prefix plus the entry name. Results, their order and the symlink-following behavior are unchanged for fs.readdirSync, fs.readdir and fs.promises.readdir. The known_issues test for Buffer paths (#58892) called back without checking the error; the error now reaches the callback instead of being thrown from the completion handler, so the test asserts success to keep expressing the issue. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65487 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 5b7d02e commit 262f0ec

5 files changed

Lines changed: 112 additions & 150 deletions

File tree

‎benchmark/fs/bench-readdir.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

13-
functionmain({ n, dir, withFileTypes }){
14+
functionmain({ n, dir, withFileTypes, recursive}){
1415
withFileTypes=withFileTypes==='true';
16+
recursive=recursive==='true';
1517
constfullPath=path.resolve(__dirname,'../../',dir);
1618
bench.start();
1719
(functionr(cntr){
1820
if(cntr--<=0)
1921
returnbench.end(n);
20-
fs.readdir(fullPath,{ withFileTypes },()=>{
22+
fs.readdir(fullPath,{ withFileTypes, recursive},()=>{
2123
r(cntr);
2224
});
2325
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

1314

14-
functionmain({ n, dir, withFileTypes }){
15+
functionmain({ n, dir, withFileTypes, recursive}){
1516
withFileTypes=withFileTypes==='true';
17+
recursive=recursive==='true';
1618
constfullPath=path.resolve(__dirname,'../../',dir);
1719
bench.start();
1820
for(leti=0;i<n;i++){
19-
fs.readdirSync(fullPath,{ withFileTypes });
21+
fs.readdirSync(fullPath,{ withFileTypes, recursive});
2022
}
2123
bench.end(n);
2224
}

‎lib/fs.js‎

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ const {
5757
F_OK,
5858
O_WRONLY,
5959
O_SYMLINK,
60+
UV_DIRENT_DIR,
61+
UV_DIRENT_LINK,
62+
UV_DIRENT_UNKNOWN,
6063
}=constants;
6164

6265
constpathModule=require('path');
@@ -1739,6 +1742,43 @@ function mkdirSync(path, options) {
17391742
}
17401743
}
17411744

1745+
/**
1746+
* Appends one directory's entries to `context.results` and the subdirectories
1747+
* still to visit to `context.dirs` (with the prefix their entries get in
1748+
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1749+
* result with file types, so only symbolic links and entries of unknown type
1750+
* need a stat() to find out whether they lead to a directory.
1751+
* @param {string} dir
1752+
* @param {string} prefix
1753+
* @param {[string[], number[]]} result
1754+
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1755+
*/
1756+
functioncollectRecursiveReaddirResult(dir,prefix,{0: names,1: types},context){
1757+
const{ length }=names;
1758+
for(leti=0;i<length;i++){
1759+
constname=names[i];
1760+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1761+
letisDirectory;
1762+
if(context.withFileTypes){
1763+
constdirent=getDirent(dir,name,types[i]);
1764+
ArrayPrototypePush(context.results,dirent);
1765+
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1766+
isDirectory=dirent.isDirectory()||
1767+
(dirent.isSymbolicLink()&&binding.internalModuleStat(pathModule.join(dir,name))===1);
1768+
}else{
1769+
ArrayPrototypePush(context.results,relative);
1770+
consttype=types[i];
1771+
isDirectory=type===UV_DIRENT_DIR||
1772+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1773+
binding.internalModuleStat(pathModule.join(dir,name))===1);
1774+
}
1775+
if(isDirectory){
1776+
ArrayPrototypePush(context.dirs,pathModule.join(dir,name));
1777+
ArrayPrototypePush(context.prefixes,relative);
1778+
}
1779+
}
1780+
}
1781+
17421782
/*
17431783
* An recursive algorithm for reading the entire contents of the `basePath` directory.
17441784
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1754,15 +1794,20 @@ function mkdirSync(path, options) {
17541794
functionreaddirRecursive(basePath,options,callback){
17551795
constcontext={
17561796
withFileTypes: Boolean(options.withFileTypes),
1757-
encoding: options.encoding,
1758-
basePath,
1759-
readdirResults: [],
1760-
pathsQueue: [basePath],
1797+
results: [],
1798+
dirs: [basePath],
1799+
prefixes: [''],
17611800
};
17621801

17631802
leti=0;
17641803

1765-
functionread(path){
1804+
/**
1805+
* Reads one directory from `context.dirs` and then moves on to the next
1806+
* one, or calls back once none are left.
1807+
* @param {string} path
1808+
* @param {string} prefix path of this directory relative to `basePath`
1809+
*/
1810+
functionread(path,prefix){
17661811
constreq=newFSReqCallback();
17671812
req.oncomplete=(err,result)=>{
17681813
if(err){
@@ -1771,68 +1816,28 @@ function readdirRecursive(basePath, options, callback) {
17711816
}
17721817

17731818
if(result===undefined){
1774-
callback(null,context.readdirResults);
1819+
callback(null,context.results);
17751820
return;
17761821
}
17771822

1778-
processReaddirResult({
1779-
result,
1780-
currentPath: path,
1781-
context,
1782-
});
1823+
try{
1824+
collectRecursiveReaddirResult(path,prefix,result,context);
1825+
}catch(err){
1826+
callback(err);
1827+
return;
1828+
}
17831829

1784-
if(i<context.pathsQueue.length){
1785-
read(context.pathsQueue[i++]);
1830+
if(i<context.dirs.length){
1831+
read(context.dirs[i],context.prefixes[i++]);
17861832
}else{
1787-
callback(null,context.readdirResults);
1833+
callback(null,context.results);
17881834
}
17891835
};
17901836

1791-
binding.readdir(
1792-
path,
1793-
context.encoding,
1794-
context.withFileTypes,
1795-
req,
1796-
);
1797-
}
1798-
1799-
read(context.pathsQueue[i++]);
1800-
}
1801-
1802-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1803-
// The first array is the names, and the second array is the types.
1804-
// They are guaranteed to be the same length; hence, setting `length` to the length
1805-
// of the first array within the result.
1806-
constprocessReaddirResult=(args)=>(args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
1807-
1808-
functionhandleDirents({ result, currentPath, context }){
1809-
const{0: names,1: types}=result;
1810-
const{ length }=names;
1811-
1812-
for(leti=0;i<length;i++){
1813-
// Avoid excluding symlinks, as they are not directories.
1814-
// Refs: https://github.com/nodejs/node/issues/52663
1815-
constfullPath=pathModule.join(currentPath,names[i]);
1816-
constdirent=getDirent(currentPath,names[i],types[i]);
1817-
ArrayPrototypePush(context.readdirResults,dirent);
1818-
1819-
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
1820-
ArrayPrototypePush(context.pathsQueue,fullPath);
1821-
}
1837+
binding.readdir(path,options.encoding,true,req);
18221838
}
1823-
}
1824-
1825-
functionhandleFilePaths({ result, currentPath, context }){
1826-
for(leti=0;i<result.length;i++){
1827-
constresultPath=pathModule.join(currentPath,result[i]);
1828-
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1829-
conststat=binding.internalModuleStat(resultPath);
1830-
ArrayPrototypePush(context.readdirResults,relativeResultPath);
18311839

1832-
if(stat===1){
1833-
ArrayPrototypePush(context.pathsQueue,resultPath);
1834-
}
1835-
}
1840+
read(context.dirs[i],context.prefixes[i++]);
18361841
}
18371842

18381843
/**
@@ -1846,35 +1851,20 @@ function handleFilePaths({ result, currentPath, context }) {
18461851
functionreaddirSyncRecursive(basePath,options){
18471852
constcontext={
18481853
withFileTypes: Boolean(options.withFileTypes),
1849-
encoding: options.encoding,
1850-
basePath,
1851-
readdirResults: [],
1852-
pathsQueue: [basePath],
1854+
results: [],
1855+
dirs: [basePath],
1856+
prefixes: [''],
18531857
};
18541858

1855-
functionread(path){
1856-
constreaddirResult=binding.readdir(
1857-
path,
1858-
context.encoding,
1859-
context.withFileTypes,
1860-
);
1861-
1862-
if(readdirResult===undefined){
1863-
return;
1859+
for(leti=0;i<context.dirs.length;i++){
1860+
constdir=context.dirs[i];
1861+
constresult=binding.readdir(dir,options.encoding,true);
1862+
if(result!==undefined){
1863+
collectRecursiveReaddirResult(dir,context.prefixes[i],result,context);
18641864
}
1865-
1866-
processReaddirResult({
1867-
result: readdirResult,
1868-
currentPath: path,
1869-
context,
1870-
});
1871-
}
1872-
1873-
for(leti=0;i<context.pathsQueue.length;i++){
1874-
read(context.pathsQueue[i]);
18751865
}
18761866

1877-
returncontext.readdirResults;
1867+
returncontext.results;
18781868
}
18791869

18801870
/**

‎lib/internal/fs/promises.js‎

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const {
3232
O_WRONLY,
3333
S_IFMT,
3434
S_IFREG,
35+
UV_DIRENT_DIR,
36+
UV_DIRENT_LINK,
37+
UV_DIRENT_UNKNOWN,
3538
}=constants;
3639

3740
constbinding=internalBinding('fs');
@@ -63,6 +66,7 @@ const {
6366
kWriteFileMaxChunkSize,
6467
},
6568
copyObject,
69+
getDirent,
6670
getDirents,
6771
getOptions,
6872
getStatFsFromBinding,
@@ -1638,73 +1642,37 @@ async function mkdir(path, options) {
16381642
}
16391643

16401644
asyncfunctionreaddirRecursive(originalPath,options){
1645+
constwithFileTypes=!!options.withFileTypes;
1646+
constreaddirWithTypes=(path)=>PromisePrototypeThen(
1647+
binding.readdir(path,options.encoding,true,kUsePromises),
1648+
undefined,
1649+
handleErrorFromBinding,
1650+
);
16411651
constresult=[];
1642-
constqueue=[
1643-
[
1644-
originalPath,
1645-
awaitPromisePrototypeThen(
1646-
binding.readdir(
1647-
originalPath,
1648-
options.encoding,
1649-
!!options.withFileTypes,
1650-
kUsePromises,
1651-
),
1652-
undefined,
1653-
handleErrorFromBinding,
1654-
),
1655-
],
1656-
];
1657-
1658-
1659-
if(options.withFileTypes){
1660-
while(queue.length>0){
1661-
// If we want to implement BFS make this a `shift` call instead of `pop`
1662-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1663-
for(constdirentofgetDirents(path,readdir)){
1652+
constqueue=[[originalPath,'',awaitreaddirWithTypes(originalPath)]];
1653+
1654+
while(queue.length>0){
1655+
// If we want to implement BFS make this a `shift` call instead of `pop`
1656+
const{0: path,1: prefix,2: {0: names,1: types}}=ArrayPrototypePop(queue);
1657+
for(leti=0;i<names.length;i++){
1658+
constname=names[i];
1659+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1660+
letisDirectory;
1661+
if(withFileTypes){
1662+
constdirent=getDirent(path,name,types[i]);
16641663
ArrayPrototypePush(result,dirent);
1665-
if(dirent.isDirectory()){
1666-
constdirentPath=pathModule.join(path,dirent.name);
1667-
ArrayPrototypePush(queue,[
1668-
direntPath,
1669-
awaitPromisePrototypeThen(
1670-
binding.readdir(
1671-
direntPath,
1672-
options.encoding,
1673-
true,
1674-
kUsePromises,
1675-
),
1676-
undefined,
1677-
handleErrorFromBinding,
1678-
),
1679-
]);
1680-
}
1664+
isDirectory=dirent.isDirectory();
1665+
}else{
1666+
ArrayPrototypePush(result,relative);
1667+
// Entries that are, or may be, symbolic links to directories are followed.
1668+
consttype=types[i];
1669+
isDirectory=type===UV_DIRENT_DIR||
1670+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1671+
binding.internalModuleStat(pathModule.join(path,name))===1);
16811672
}
1682-
}
1683-
}else{
1684-
while(queue.length>0){
1685-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1686-
for(constentofreaddir){
1687-
constdirentPath=pathModule.join(path,ent);
1688-
conststat=binding.internalModuleStat(direntPath);
1689-
ArrayPrototypePush(
1690-
result,
1691-
pathModule.relative(originalPath,direntPath),
1692-
);
1693-
if(stat===1){
1694-
ArrayPrototypePush(queue,[
1695-
direntPath,
1696-
awaitPromisePrototypeThen(
1697-
binding.readdir(
1698-
direntPath,
1699-
options.encoding,
1700-
false,
1701-
kUsePromises,
1702-
),
1703-
undefined,
1704-
handleErrorFromBinding,
1705-
),
1706-
]);
1707-
}
1673+
if(isDirectory){
1674+
constdirentPath=pathModule.join(path,name);
1675+
ArrayPrototypePush(queue,[direntPath,relative,awaitreaddirWithTypes(direntPath)]);
17081676
}
17091677
}
17101678
}

‎test/known_issues/test-fs-readdir-recursive-with-buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const { readdir } = require('node:fs');
1212
const{ join }=require('node:path');
1313

1414
consttestDirPath=join(__dirname,'..','..');
15-
readdir(Buffer.from(testDirPath),{recursive: true},common.mustCall());
15+
readdir(Buffer.from(testDirPath),{recursive: true},common.mustSucceed());

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 262f0ec

Browse files
codebytereaduh95
authored andcommitted
fs: stop stat()ing every entry in recursive readdir
readdir({ recursive: true }) asked the binding for names only and then called internalModuleStat() on every entry to find the directories to descend into; with withFileTypes it built the Dirents and still stat()ed every entry that was not already a directory. Both variants also ran path.join() and path.relative() per entry to build the relative result. Ask the binding for file types in all cases, descend into directories directly, and only stat() symbolic links and entries of unknown type (which is what could point to a directory). The relative name is the parent's prefix plus the entry name. Results, their order and the symlink-following behavior are unchanged for fs.readdirSync, fs.readdir and fs.promises.readdir. The known_issues test for Buffer paths (#58892) called back without checking the error; the error now reaches the callback instead of being thrown from the completion handler, so the test asserts success to keep expressing the issue. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65487 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 5b7d02e commit 262f0ec

5 files changed

Lines changed: 112 additions & 150 deletions

File tree

‎benchmark/fs/bench-readdir.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

13-
functionmain({ n, dir, withFileTypes }){
14+
functionmain({ n, dir, withFileTypes, recursive}){
1415
withFileTypes=withFileTypes==='true';
16+
recursive=recursive==='true';
1517
constfullPath=path.resolve(__dirname,'../../',dir);
1618
bench.start();
1719
(functionr(cntr){
1820
if(cntr--<=0)
1921
returnbench.end(n);
20-
fs.readdir(fullPath,{ withFileTypes },()=>{
22+
fs.readdir(fullPath,{ withFileTypes, recursive},()=>{
2123
r(cntr);
2224
});
2325
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

1314

14-
functionmain({ n, dir, withFileTypes }){
15+
functionmain({ n, dir, withFileTypes, recursive}){
1516
withFileTypes=withFileTypes==='true';
17+
recursive=recursive==='true';
1618
constfullPath=path.resolve(__dirname,'../../',dir);
1719
bench.start();
1820
for(leti=0;i<n;i++){
19-
fs.readdirSync(fullPath,{ withFileTypes });
21+
fs.readdirSync(fullPath,{ withFileTypes, recursive});
2022
}
2123
bench.end(n);
2224
}

‎lib/fs.js‎

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ const {
5757
F_OK,
5858
O_WRONLY,
5959
O_SYMLINK,
60+
UV_DIRENT_DIR,
61+
UV_DIRENT_LINK,
62+
UV_DIRENT_UNKNOWN,
6063
}=constants;
6164

6265
constpathModule=require('path');
@@ -1739,6 +1742,43 @@ function mkdirSync(path, options) {
17391742
}
17401743
}
17411744

1745+
/**
1746+
* Appends one directory's entries to `context.results` and the subdirectories
1747+
* still to visit to `context.dirs` (with the prefix their entries get in
1748+
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1749+
* result with file types, so only symbolic links and entries of unknown type
1750+
* need a stat() to find out whether they lead to a directory.
1751+
* @param {string} dir
1752+
* @param {string} prefix
1753+
* @param {[string[], number[]]} result
1754+
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1755+
*/
1756+
functioncollectRecursiveReaddirResult(dir,prefix,{0: names,1: types},context){
1757+
const{ length }=names;
1758+
for(leti=0;i<length;i++){
1759+
constname=names[i];
1760+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1761+
letisDirectory;
1762+
if(context.withFileTypes){
1763+
constdirent=getDirent(dir,name,types[i]);
1764+
ArrayPrototypePush(context.results,dirent);
1765+
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1766+
isDirectory=dirent.isDirectory()||
1767+
(dirent.isSymbolicLink()&&binding.internalModuleStat(pathModule.join(dir,name))===1);
1768+
}else{
1769+
ArrayPrototypePush(context.results,relative);
1770+
consttype=types[i];
1771+
isDirectory=type===UV_DIRENT_DIR||
1772+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1773+
binding.internalModuleStat(pathModule.join(dir,name))===1);
1774+
}
1775+
if(isDirectory){
1776+
ArrayPrototypePush(context.dirs,pathModule.join(dir,name));
1777+
ArrayPrototypePush(context.prefixes,relative);
1778+
}
1779+
}
1780+
}
1781+
17421782
/*
17431783
* An recursive algorithm for reading the entire contents of the `basePath` directory.
17441784
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1754,15 +1794,20 @@ function mkdirSync(path, options) {
17541794
functionreaddirRecursive(basePath,options,callback){
17551795
constcontext={
17561796
withFileTypes: Boolean(options.withFileTypes),
1757-
encoding: options.encoding,
1758-
basePath,
1759-
readdirResults: [],
1760-
pathsQueue: [basePath],
1797+
results: [],
1798+
dirs: [basePath],
1799+
prefixes: [''],
17611800
};
17621801

17631802
leti=0;
17641803

1765-
functionread(path){
1804+
/**
1805+
* Reads one directory from `context.dirs` and then moves on to the next
1806+
* one, or calls back once none are left.
1807+
* @param {string} path
1808+
* @param {string} prefix path of this directory relative to `basePath`
1809+
*/
1810+
functionread(path,prefix){
17661811
constreq=newFSReqCallback();
17671812
req.oncomplete=(err,result)=>{
17681813
if(err){
@@ -1771,68 +1816,28 @@ function readdirRecursive(basePath, options, callback) {
17711816
}
17721817

17731818
if(result===undefined){
1774-
callback(null,context.readdirResults);
1819+
callback(null,context.results);
17751820
return;
17761821
}
17771822

1778-
processReaddirResult({
1779-
result,
1780-
currentPath: path,
1781-
context,
1782-
});
1823+
try{
1824+
collectRecursiveReaddirResult(path,prefix,result,context);
1825+
}catch(err){
1826+
callback(err);
1827+
return;
1828+
}
17831829

1784-
if(i<context.pathsQueue.length){
1785-
read(context.pathsQueue[i++]);
1830+
if(i<context.dirs.length){
1831+
read(context.dirs[i],context.prefixes[i++]);
17861832
}else{
1787-
callback(null,context.readdirResults);
1833+
callback(null,context.results);
17881834
}
17891835
};
17901836

1791-
binding.readdir(
1792-
path,
1793-
context.encoding,
1794-
context.withFileTypes,
1795-
req,
1796-
);
1797-
}
1798-
1799-
read(context.pathsQueue[i++]);
1800-
}
1801-
1802-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1803-
// The first array is the names, and the second array is the types.
1804-
// They are guaranteed to be the same length; hence, setting `length` to the length
1805-
// of the first array within the result.
1806-
constprocessReaddirResult=(args)=>(args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
1807-
1808-
functionhandleDirents({ result, currentPath, context }){
1809-
const{0: names,1: types}=result;
1810-
const{ length }=names;
1811-
1812-
for(leti=0;i<length;i++){
1813-
// Avoid excluding symlinks, as they are not directories.
1814-
// Refs: https://github.com/nodejs/node/issues/52663
1815-
constfullPath=pathModule.join(currentPath,names[i]);
1816-
constdirent=getDirent(currentPath,names[i],types[i]);
1817-
ArrayPrototypePush(context.readdirResults,dirent);
1818-
1819-
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
1820-
ArrayPrototypePush(context.pathsQueue,fullPath);
1821-
}
1837+
binding.readdir(path,options.encoding,true,req);
18221838
}
1823-
}
1824-
1825-
functionhandleFilePaths({ result, currentPath, context }){
1826-
for(leti=0;i<result.length;i++){
1827-
constresultPath=pathModule.join(currentPath,result[i]);
1828-
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1829-
conststat=binding.internalModuleStat(resultPath);
1830-
ArrayPrototypePush(context.readdirResults,relativeResultPath);
18311839

1832-
if(stat===1){
1833-
ArrayPrototypePush(context.pathsQueue,resultPath);
1834-
}
1835-
}
1840+
read(context.dirs[i],context.prefixes[i++]);
18361841
}
18371842

18381843
/**
@@ -1846,35 +1851,20 @@ function handleFilePaths({ result, currentPath, context }) {
18461851
functionreaddirSyncRecursive(basePath,options){
18471852
constcontext={
18481853
withFileTypes: Boolean(options.withFileTypes),
1849-
encoding: options.encoding,
1850-
basePath,
1851-
readdirResults: [],
1852-
pathsQueue: [basePath],
1854+
results: [],
1855+
dirs: [basePath],
1856+
prefixes: [''],
18531857
};
18541858

1855-
functionread(path){
1856-
constreaddirResult=binding.readdir(
1857-
path,
1858-
context.encoding,
1859-
context.withFileTypes,
1860-
);
1861-
1862-
if(readdirResult===undefined){
1863-
return;
1859+
for(leti=0;i<context.dirs.length;i++){
1860+
constdir=context.dirs[i];
1861+
constresult=binding.readdir(dir,options.encoding,true);
1862+
if(result!==undefined){
1863+
collectRecursiveReaddirResult(dir,context.prefixes[i],result,context);
18641864
}
1865-
1866-
processReaddirResult({
1867-
result: readdirResult,
1868-
currentPath: path,
1869-
context,
1870-
});
1871-
}
1872-
1873-
for(leti=0;i<context.pathsQueue.length;i++){
1874-
read(context.pathsQueue[i]);
18751865
}
18761866

1877-
returncontext.readdirResults;
1867+
returncontext.results;
18781868
}
18791869

18801870
/**

‎lib/internal/fs/promises.js‎

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const {
3232
O_WRONLY,
3333
S_IFMT,
3434
S_IFREG,
35+
UV_DIRENT_DIR,
36+
UV_DIRENT_LINK,
37+
UV_DIRENT_UNKNOWN,
3538
}=constants;
3639

3740
constbinding=internalBinding('fs');
@@ -63,6 +66,7 @@ const {
6366
kWriteFileMaxChunkSize,
6467
},
6568
copyObject,
69+
getDirent,
6670
getDirents,
6771
getOptions,
6872
getStatFsFromBinding,
@@ -1638,73 +1642,37 @@ async function mkdir(path, options) {
16381642
}
16391643

16401644
asyncfunctionreaddirRecursive(originalPath,options){
1645+
constwithFileTypes=!!options.withFileTypes;
1646+
constreaddirWithTypes=(path)=>PromisePrototypeThen(
1647+
binding.readdir(path,options.encoding,true,kUsePromises),
1648+
undefined,
1649+
handleErrorFromBinding,
1650+
);
16411651
constresult=[];
1642-
constqueue=[
1643-
[
1644-
originalPath,
1645-
awaitPromisePrototypeThen(
1646-
binding.readdir(
1647-
originalPath,
1648-
options.encoding,
1649-
!!options.withFileTypes,
1650-
kUsePromises,
1651-
),
1652-
undefined,
1653-
handleErrorFromBinding,
1654-
),
1655-
],
1656-
];
1657-
1658-
1659-
if(options.withFileTypes){
1660-
while(queue.length>0){
1661-
// If we want to implement BFS make this a `shift` call instead of `pop`
1662-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1663-
for(constdirentofgetDirents(path,readdir)){
1652+
constqueue=[[originalPath,'',awaitreaddirWithTypes(originalPath)]];
1653+
1654+
while(queue.length>0){
1655+
// If we want to implement BFS make this a `shift` call instead of `pop`
1656+
const{0: path,1: prefix,2: {0: names,1: types}}=ArrayPrototypePop(queue);
1657+
for(leti=0;i<names.length;i++){
1658+
constname=names[i];
1659+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1660+
letisDirectory;
1661+
if(withFileTypes){
1662+
constdirent=getDirent(path,name,types[i]);
16641663
ArrayPrototypePush(result,dirent);
1665-
if(dirent.isDirectory()){
1666-
constdirentPath=pathModule.join(path,dirent.name);
1667-
ArrayPrototypePush(queue,[
1668-
direntPath,
1669-
awaitPromisePrototypeThen(
1670-
binding.readdir(
1671-
direntPath,
1672-
options.encoding,
1673-
true,
1674-
kUsePromises,
1675-
),
1676-
undefined,
1677-
handleErrorFromBinding,
1678-
),
1679-
]);
1680-
}
1664+
isDirectory=dirent.isDirectory();
1665+
}else{
1666+
ArrayPrototypePush(result,relative);
1667+
// Entries that are, or may be, symbolic links to directories are followed.
1668+
consttype=types[i];
1669+
isDirectory=type===UV_DIRENT_DIR||
1670+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1671+
binding.internalModuleStat(pathModule.join(path,name))===1);
16811672
}
1682-
}
1683-
}else{
1684-
while(queue.length>0){
1685-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1686-
for(constentofreaddir){
1687-
constdirentPath=pathModule.join(path,ent);
1688-
conststat=binding.internalModuleStat(direntPath);
1689-
ArrayPrototypePush(
1690-
result,
1691-
pathModule.relative(originalPath,direntPath),
1692-
);
1693-
if(stat===1){
1694-
ArrayPrototypePush(queue,[
1695-
direntPath,
1696-
awaitPromisePrototypeThen(
1697-
binding.readdir(
1698-
direntPath,
1699-
options.encoding,
1700-
false,
1701-
kUsePromises,
1702-
),
1703-
undefined,
1704-
handleErrorFromBinding,
1705-
),
1706-
]);
1707-
}
1673+
if(isDirectory){
1674+
constdirentPath=pathModule.join(path,name);
1675+
ArrayPrototypePush(queue,[direntPath,relative,awaitreaddirWithTypes(direntPath)]);
17081676
}
17091677
}
17101678
}

‎test/known_issues/test-fs-readdir-recursive-with-buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const { readdir } = require('node:fs');
1212
const{ join }=require('node:path');
1313

1414
consttestDirPath=join(__dirname,'..','..');
15-
readdir(Buffer.from(testDirPath),{recursive: true},common.mustCall());
15+
readdir(Buffer.from(testDirPath),{recursive: true},common.mustSucceed());

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Commit 262f0ec

Browse files
codebytereaduh95
authored andcommitted
fs: stop stat()ing every entry in recursive readdir
readdir({ recursive: true }) asked the binding for names only and then called internalModuleStat() on every entry to find the directories to descend into; with withFileTypes it built the Dirents and still stat()ed every entry that was not already a directory. Both variants also ran path.join() and path.relative() per entry to build the relative result. Ask the binding for file types in all cases, descend into directories directly, and only stat() symbolic links and entries of unknown type (which is what could point to a directory). The relative name is the parent's prefix plus the entry name. Results, their order and the symlink-following behavior are unchanged for fs.readdirSync, fs.readdir and fs.promises.readdir. The known_issues test for Buffer paths (#58892) called back without checking the error; the error now reaches the callback instead of being thrown from the completion handler, so the test asserts success to keep expressing the issue. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65487 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 5b7d02e commit 262f0ec

5 files changed

Lines changed: 112 additions & 150 deletions

File tree

‎benchmark/fs/bench-readdir.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

13-
functionmain({ n, dir, withFileTypes }){
14+
functionmain({ n, dir, withFileTypes, recursive}){
1415
withFileTypes=withFileTypes==='true';
16+
recursive=recursive==='true';
1517
constfullPath=path.resolve(__dirname,'../../',dir);
1618
bench.start();
1719
(functionr(cntr){
1820
if(cntr--<=0)
1921
returnbench.end(n);
20-
fs.readdir(fullPath,{ withFileTypes },()=>{
22+
fs.readdir(fullPath,{ withFileTypes, recursive},()=>{
2123
r(cntr);
2224
});
2325
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

1314

14-
functionmain({ n, dir, withFileTypes }){
15+
functionmain({ n, dir, withFileTypes, recursive}){
1516
withFileTypes=withFileTypes==='true';
17+
recursive=recursive==='true';
1618
constfullPath=path.resolve(__dirname,'../../',dir);
1719
bench.start();
1820
for(leti=0;i<n;i++){
19-
fs.readdirSync(fullPath,{ withFileTypes });
21+
fs.readdirSync(fullPath,{ withFileTypes, recursive});
2022
}
2123
bench.end(n);
2224
}

‎lib/fs.js‎

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ const {
5757
F_OK,
5858
O_WRONLY,
5959
O_SYMLINK,
60+
UV_DIRENT_DIR,
61+
UV_DIRENT_LINK,
62+
UV_DIRENT_UNKNOWN,
6063
}=constants;
6164

6265
constpathModule=require('path');
@@ -1739,6 +1742,43 @@ function mkdirSync(path, options) {
17391742
}
17401743
}
17411744

1745+
/**
1746+
* Appends one directory's entries to `context.results` and the subdirectories
1747+
* still to visit to `context.dirs` (with the prefix their entries get in
1748+
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1749+
* result with file types, so only symbolic links and entries of unknown type
1750+
* need a stat() to find out whether they lead to a directory.
1751+
* @param {string} dir
1752+
* @param {string} prefix
1753+
* @param {[string[], number[]]} result
1754+
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1755+
*/
1756+
functioncollectRecursiveReaddirResult(dir,prefix,{0: names,1: types},context){
1757+
const{ length }=names;
1758+
for(leti=0;i<length;i++){
1759+
constname=names[i];
1760+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1761+
letisDirectory;
1762+
if(context.withFileTypes){
1763+
constdirent=getDirent(dir,name,types[i]);
1764+
ArrayPrototypePush(context.results,dirent);
1765+
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1766+
isDirectory=dirent.isDirectory()||
1767+
(dirent.isSymbolicLink()&&binding.internalModuleStat(pathModule.join(dir,name))===1);
1768+
}else{
1769+
ArrayPrototypePush(context.results,relative);
1770+
consttype=types[i];
1771+
isDirectory=type===UV_DIRENT_DIR||
1772+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1773+
binding.internalModuleStat(pathModule.join(dir,name))===1);
1774+
}
1775+
if(isDirectory){
1776+
ArrayPrototypePush(context.dirs,pathModule.join(dir,name));
1777+
ArrayPrototypePush(context.prefixes,relative);
1778+
}
1779+
}
1780+
}
1781+
17421782
/*
17431783
* An recursive algorithm for reading the entire contents of the `basePath` directory.
17441784
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1754,15 +1794,20 @@ function mkdirSync(path, options) {
17541794
functionreaddirRecursive(basePath,options,callback){
17551795
constcontext={
17561796
withFileTypes: Boolean(options.withFileTypes),
1757-
encoding: options.encoding,
1758-
basePath,
1759-
readdirResults: [],
1760-
pathsQueue: [basePath],
1797+
results: [],
1798+
dirs: [basePath],
1799+
prefixes: [''],
17611800
};
17621801

17631802
leti=0;
17641803

1765-
functionread(path){
1804+
/**
1805+
* Reads one directory from `context.dirs` and then moves on to the next
1806+
* one, or calls back once none are left.
1807+
* @param {string} path
1808+
* @param {string} prefix path of this directory relative to `basePath`
1809+
*/
1810+
functionread(path,prefix){
17661811
constreq=newFSReqCallback();
17671812
req.oncomplete=(err,result)=>{
17681813
if(err){
@@ -1771,68 +1816,28 @@ function readdirRecursive(basePath, options, callback) {
17711816
}
17721817

17731818
if(result===undefined){
1774-
callback(null,context.readdirResults);
1819+
callback(null,context.results);
17751820
return;
17761821
}
17771822

1778-
processReaddirResult({
1779-
result,
1780-
currentPath: path,
1781-
context,
1782-
});
1823+
try{
1824+
collectRecursiveReaddirResult(path,prefix,result,context);
1825+
}catch(err){
1826+
callback(err);
1827+
return;
1828+
}
17831829

1784-
if(i<context.pathsQueue.length){
1785-
read(context.pathsQueue[i++]);
1830+
if(i<context.dirs.length){
1831+
read(context.dirs[i],context.prefixes[i++]);
17861832
}else{
1787-
callback(null,context.readdirResults);
1833+
callback(null,context.results);
17881834
}
17891835
};
17901836

1791-
binding.readdir(
1792-
path,
1793-
context.encoding,
1794-
context.withFileTypes,
1795-
req,
1796-
);
1797-
}
1798-
1799-
read(context.pathsQueue[i++]);
1800-
}
1801-
1802-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1803-
// The first array is the names, and the second array is the types.
1804-
// They are guaranteed to be the same length; hence, setting `length` to the length
1805-
// of the first array within the result.
1806-
constprocessReaddirResult=(args)=>(args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
1807-
1808-
functionhandleDirents({ result, currentPath, context }){
1809-
const{0: names,1: types}=result;
1810-
const{ length }=names;
1811-
1812-
for(leti=0;i<length;i++){
1813-
// Avoid excluding symlinks, as they are not directories.
1814-
// Refs: https://github.com/nodejs/node/issues/52663
1815-
constfullPath=pathModule.join(currentPath,names[i]);
1816-
constdirent=getDirent(currentPath,names[i],types[i]);
1817-
ArrayPrototypePush(context.readdirResults,dirent);
1818-
1819-
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
1820-
ArrayPrototypePush(context.pathsQueue,fullPath);
1821-
}
1837+
binding.readdir(path,options.encoding,true,req);
18221838
}
1823-
}
1824-
1825-
functionhandleFilePaths({ result, currentPath, context }){
1826-
for(leti=0;i<result.length;i++){
1827-
constresultPath=pathModule.join(currentPath,result[i]);
1828-
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1829-
conststat=binding.internalModuleStat(resultPath);
1830-
ArrayPrototypePush(context.readdirResults,relativeResultPath);
18311839

1832-
if(stat===1){
1833-
ArrayPrototypePush(context.pathsQueue,resultPath);
1834-
}
1835-
}
1840+
read(context.dirs[i],context.prefixes[i++]);
18361841
}
18371842

18381843
/**
@@ -1846,35 +1851,20 @@ function handleFilePaths({ result, currentPath, context }) {
18461851
functionreaddirSyncRecursive(basePath,options){
18471852
constcontext={
18481853
withFileTypes: Boolean(options.withFileTypes),
1849-
encoding: options.encoding,
1850-
basePath,
1851-
readdirResults: [],
1852-
pathsQueue: [basePath],
1854+
results: [],
1855+
dirs: [basePath],
1856+
prefixes: [''],
18531857
};
18541858

1855-
functionread(path){
1856-
constreaddirResult=binding.readdir(
1857-
path,
1858-
context.encoding,
1859-
context.withFileTypes,
1860-
);
1861-
1862-
if(readdirResult===undefined){
1863-
return;
1859+
for(leti=0;i<context.dirs.length;i++){
1860+
constdir=context.dirs[i];
1861+
constresult=binding.readdir(dir,options.encoding,true);
1862+
if(result!==undefined){
1863+
collectRecursiveReaddirResult(dir,context.prefixes[i],result,context);
18641864
}
1865-
1866-
processReaddirResult({
1867-
result: readdirResult,
1868-
currentPath: path,
1869-
context,
1870-
});
1871-
}
1872-
1873-
for(leti=0;i<context.pathsQueue.length;i++){
1874-
read(context.pathsQueue[i]);
18751865
}
18761866

1877-
returncontext.readdirResults;
1867+
returncontext.results;
18781868
}
18791869

18801870
/**

‎lib/internal/fs/promises.js‎

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const {
3232
O_WRONLY,
3333
S_IFMT,
3434
S_IFREG,
35+
UV_DIRENT_DIR,
36+
UV_DIRENT_LINK,
37+
UV_DIRENT_UNKNOWN,
3538
}=constants;
3639

3740
constbinding=internalBinding('fs');
@@ -63,6 +66,7 @@ const {
6366
kWriteFileMaxChunkSize,
6467
},
6568
copyObject,
69+
getDirent,
6670
getDirents,
6771
getOptions,
6872
getStatFsFromBinding,
@@ -1638,73 +1642,37 @@ async function mkdir(path, options) {
16381642
}
16391643

16401644
asyncfunctionreaddirRecursive(originalPath,options){
1645+
constwithFileTypes=!!options.withFileTypes;
1646+
constreaddirWithTypes=(path)=>PromisePrototypeThen(
1647+
binding.readdir(path,options.encoding,true,kUsePromises),
1648+
undefined,
1649+
handleErrorFromBinding,
1650+
);
16411651
constresult=[];
1642-
constqueue=[
1643-
[
1644-
originalPath,
1645-
awaitPromisePrototypeThen(
1646-
binding.readdir(
1647-
originalPath,
1648-
options.encoding,
1649-
!!options.withFileTypes,
1650-
kUsePromises,
1651-
),
1652-
undefined,
1653-
handleErrorFromBinding,
1654-
),
1655-
],
1656-
];
1657-
1658-
1659-
if(options.withFileTypes){
1660-
while(queue.length>0){
1661-
// If we want to implement BFS make this a `shift` call instead of `pop`
1662-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1663-
for(constdirentofgetDirents(path,readdir)){
1652+
constqueue=[[originalPath,'',awaitreaddirWithTypes(originalPath)]];
1653+
1654+
while(queue.length>0){
1655+
// If we want to implement BFS make this a `shift` call instead of `pop`
1656+
const{0: path,1: prefix,2: {0: names,1: types}}=ArrayPrototypePop(queue);
1657+
for(leti=0;i<names.length;i++){
1658+
constname=names[i];
1659+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1660+
letisDirectory;
1661+
if(withFileTypes){
1662+
constdirent=getDirent(path,name,types[i]);
16641663
ArrayPrototypePush(result,dirent);
1665-
if(dirent.isDirectory()){
1666-
constdirentPath=pathModule.join(path,dirent.name);
1667-
ArrayPrototypePush(queue,[
1668-
direntPath,
1669-
awaitPromisePrototypeThen(
1670-
binding.readdir(
1671-
direntPath,
1672-
options.encoding,
1673-
true,
1674-
kUsePromises,
1675-
),
1676-
undefined,
1677-
handleErrorFromBinding,
1678-
),
1679-
]);
1680-
}
1664+
isDirectory=dirent.isDirectory();
1665+
}else{
1666+
ArrayPrototypePush(result,relative);
1667+
// Entries that are, or may be, symbolic links to directories are followed.
1668+
consttype=types[i];
1669+
isDirectory=type===UV_DIRENT_DIR||
1670+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1671+
binding.internalModuleStat(pathModule.join(path,name))===1);
16811672
}
1682-
}
1683-
}else{
1684-
while(queue.length>0){
1685-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1686-
for(constentofreaddir){
1687-
constdirentPath=pathModule.join(path,ent);
1688-
conststat=binding.internalModuleStat(direntPath);
1689-
ArrayPrototypePush(
1690-
result,
1691-
pathModule.relative(originalPath,direntPath),
1692-
);
1693-
if(stat===1){
1694-
ArrayPrototypePush(queue,[
1695-
direntPath,
1696-
awaitPromisePrototypeThen(
1697-
binding.readdir(
1698-
direntPath,
1699-
options.encoding,
1700-
false,
1701-
kUsePromises,
1702-
),
1703-
undefined,
1704-
handleErrorFromBinding,
1705-
),
1706-
]);
1707-
}
1673+
if(isDirectory){
1674+
constdirentPath=pathModule.join(path,name);
1675+
ArrayPrototypePush(queue,[direntPath,relative,awaitreaddirWithTypes(direntPath)]);
17081676
}
17091677
}
17101678
}

‎test/known_issues/test-fs-readdir-recursive-with-buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const { readdir } = require('node:fs');
1212
const{ join }=require('node:path');
1313

1414
consttestDirPath=join(__dirname,'..','..');
15-
readdir(Buffer.from(testDirPath),{recursive: true},common.mustCall());
15+
readdir(Buffer.from(testDirPath),{recursive: true},common.mustSucceed());

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 262f0ec

Browse files
codebytereaduh95
authored andcommitted
fs: stop stat()ing every entry in recursive readdir
readdir({ recursive: true }) asked the binding for names only and then called internalModuleStat() on every entry to find the directories to descend into; with withFileTypes it built the Dirents and still stat()ed every entry that was not already a directory. Both variants also ran path.join() and path.relative() per entry to build the relative result. Ask the binding for file types in all cases, descend into directories directly, and only stat() symbolic links and entries of unknown type (which is what could point to a directory). The relative name is the parent's prefix plus the entry name. Results, their order and the symlink-following behavior are unchanged for fs.readdirSync, fs.readdir and fs.promises.readdir. The known_issues test for Buffer paths (#58892) called back without checking the error; the error now reaches the callback instead of being thrown from the completion handler, so the test asserts success to keep expressing the issue. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65487 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 5b7d02e commit 262f0ec

5 files changed

Lines changed: 112 additions & 150 deletions

File tree

‎benchmark/fs/bench-readdir.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

13-
functionmain({ n, dir, withFileTypes }){
14+
functionmain({ n, dir, withFileTypes, recursive}){
1415
withFileTypes=withFileTypes==='true';
16+
recursive=recursive==='true';
1517
constfullPath=path.resolve(__dirname,'../../',dir);
1618
bench.start();
1719
(functionr(cntr){
1820
if(cntr--<=0)
1921
returnbench.end(n);
20-
fs.readdir(fullPath,{ withFileTypes },()=>{
22+
fs.readdir(fullPath,{ withFileTypes, recursive},()=>{
2123
r(cntr);
2224
});
2325
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

1314

14-
functionmain({ n, dir, withFileTypes }){
15+
functionmain({ n, dir, withFileTypes, recursive}){
1516
withFileTypes=withFileTypes==='true';
17+
recursive=recursive==='true';
1618
constfullPath=path.resolve(__dirname,'../../',dir);
1719
bench.start();
1820
for(leti=0;i<n;i++){
19-
fs.readdirSync(fullPath,{ withFileTypes });
21+
fs.readdirSync(fullPath,{ withFileTypes, recursive});
2022
}
2123
bench.end(n);
2224
}

‎lib/fs.js‎

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ const {
5757
F_OK,
5858
O_WRONLY,
5959
O_SYMLINK,
60+
UV_DIRENT_DIR,
61+
UV_DIRENT_LINK,
62+
UV_DIRENT_UNKNOWN,
6063
}=constants;
6164

6265
constpathModule=require('path');
@@ -1739,6 +1742,43 @@ function mkdirSync(path, options) {
17391742
}
17401743
}
17411744

1745+
/**
1746+
* Appends one directory's entries to `context.results` and the subdirectories
1747+
* still to visit to `context.dirs` (with the prefix their entries get in
1748+
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1749+
* result with file types, so only symbolic links and entries of unknown type
1750+
* need a stat() to find out whether they lead to a directory.
1751+
* @param {string} dir
1752+
* @param {string} prefix
1753+
* @param {[string[], number[]]} result
1754+
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1755+
*/
1756+
functioncollectRecursiveReaddirResult(dir,prefix,{0: names,1: types},context){
1757+
const{ length }=names;
1758+
for(leti=0;i<length;i++){
1759+
constname=names[i];
1760+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1761+
letisDirectory;
1762+
if(context.withFileTypes){
1763+
constdirent=getDirent(dir,name,types[i]);
1764+
ArrayPrototypePush(context.results,dirent);
1765+
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1766+
isDirectory=dirent.isDirectory()||
1767+
(dirent.isSymbolicLink()&&binding.internalModuleStat(pathModule.join(dir,name))===1);
1768+
}else{
1769+
ArrayPrototypePush(context.results,relative);
1770+
consttype=types[i];
1771+
isDirectory=type===UV_DIRENT_DIR||
1772+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1773+
binding.internalModuleStat(pathModule.join(dir,name))===1);
1774+
}
1775+
if(isDirectory){
1776+
ArrayPrototypePush(context.dirs,pathModule.join(dir,name));
1777+
ArrayPrototypePush(context.prefixes,relative);
1778+
}
1779+
}
1780+
}
1781+
17421782
/*
17431783
* An recursive algorithm for reading the entire contents of the `basePath` directory.
17441784
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1754,15 +1794,20 @@ function mkdirSync(path, options) {
17541794
functionreaddirRecursive(basePath,options,callback){
17551795
constcontext={
17561796
withFileTypes: Boolean(options.withFileTypes),
1757-
encoding: options.encoding,
1758-
basePath,
1759-
readdirResults: [],
1760-
pathsQueue: [basePath],
1797+
results: [],
1798+
dirs: [basePath],
1799+
prefixes: [''],
17611800
};
17621801

17631802
leti=0;
17641803

1765-
functionread(path){
1804+
/**
1805+
* Reads one directory from `context.dirs` and then moves on to the next
1806+
* one, or calls back once none are left.
1807+
* @param {string} path
1808+
* @param {string} prefix path of this directory relative to `basePath`
1809+
*/
1810+
functionread(path,prefix){
17661811
constreq=newFSReqCallback();
17671812
req.oncomplete=(err,result)=>{
17681813
if(err){
@@ -1771,68 +1816,28 @@ function readdirRecursive(basePath, options, callback) {
17711816
}
17721817

17731818
if(result===undefined){
1774-
callback(null,context.readdirResults);
1819+
callback(null,context.results);
17751820
return;
17761821
}
17771822

1778-
processReaddirResult({
1779-
result,
1780-
currentPath: path,
1781-
context,
1782-
});
1823+
try{
1824+
collectRecursiveReaddirResult(path,prefix,result,context);
1825+
}catch(err){
1826+
callback(err);
1827+
return;
1828+
}
17831829

1784-
if(i<context.pathsQueue.length){
1785-
read(context.pathsQueue[i++]);
1830+
if(i<context.dirs.length){
1831+
read(context.dirs[i],context.prefixes[i++]);
17861832
}else{
1787-
callback(null,context.readdirResults);
1833+
callback(null,context.results);
17881834
}
17891835
};
17901836

1791-
binding.readdir(
1792-
path,
1793-
context.encoding,
1794-
context.withFileTypes,
1795-
req,
1796-
);
1797-
}
1798-
1799-
read(context.pathsQueue[i++]);
1800-
}
1801-
1802-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1803-
// The first array is the names, and the second array is the types.
1804-
// They are guaranteed to be the same length; hence, setting `length` to the length
1805-
// of the first array within the result.
1806-
constprocessReaddirResult=(args)=>(args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
1807-
1808-
functionhandleDirents({ result, currentPath, context }){
1809-
const{0: names,1: types}=result;
1810-
const{ length }=names;
1811-
1812-
for(leti=0;i<length;i++){
1813-
// Avoid excluding symlinks, as they are not directories.
1814-
// Refs: https://github.com/nodejs/node/issues/52663
1815-
constfullPath=pathModule.join(currentPath,names[i]);
1816-
constdirent=getDirent(currentPath,names[i],types[i]);
1817-
ArrayPrototypePush(context.readdirResults,dirent);
1818-
1819-
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
1820-
ArrayPrototypePush(context.pathsQueue,fullPath);
1821-
}
1837+
binding.readdir(path,options.encoding,true,req);
18221838
}
1823-
}
1824-
1825-
functionhandleFilePaths({ result, currentPath, context }){
1826-
for(leti=0;i<result.length;i++){
1827-
constresultPath=pathModule.join(currentPath,result[i]);
1828-
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1829-
conststat=binding.internalModuleStat(resultPath);
1830-
ArrayPrototypePush(context.readdirResults,relativeResultPath);
18311839

1832-
if(stat===1){
1833-
ArrayPrototypePush(context.pathsQueue,resultPath);
1834-
}
1835-
}
1840+
read(context.dirs[i],context.prefixes[i++]);
18361841
}
18371842

18381843
/**
@@ -1846,35 +1851,20 @@ function handleFilePaths({ result, currentPath, context }) {
18461851
functionreaddirSyncRecursive(basePath,options){
18471852
constcontext={
18481853
withFileTypes: Boolean(options.withFileTypes),
1849-
encoding: options.encoding,
1850-
basePath,
1851-
readdirResults: [],
1852-
pathsQueue: [basePath],
1854+
results: [],
1855+
dirs: [basePath],
1856+
prefixes: [''],
18531857
};
18541858

1855-
functionread(path){
1856-
constreaddirResult=binding.readdir(
1857-
path,
1858-
context.encoding,
1859-
context.withFileTypes,
1860-
);
1861-
1862-
if(readdirResult===undefined){
1863-
return;
1859+
for(leti=0;i<context.dirs.length;i++){
1860+
constdir=context.dirs[i];
1861+
constresult=binding.readdir(dir,options.encoding,true);
1862+
if(result!==undefined){
1863+
collectRecursiveReaddirResult(dir,context.prefixes[i],result,context);
18641864
}
1865-
1866-
processReaddirResult({
1867-
result: readdirResult,
1868-
currentPath: path,
1869-
context,
1870-
});
1871-
}
1872-
1873-
for(leti=0;i<context.pathsQueue.length;i++){
1874-
read(context.pathsQueue[i]);
18751865
}
18761866

1877-
returncontext.readdirResults;
1867+
returncontext.results;
18781868
}
18791869

18801870
/**

‎lib/internal/fs/promises.js‎

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const {
3232
O_WRONLY,
3333
S_IFMT,
3434
S_IFREG,
35+
UV_DIRENT_DIR,
36+
UV_DIRENT_LINK,
37+
UV_DIRENT_UNKNOWN,
3538
}=constants;
3639

3740
constbinding=internalBinding('fs');
@@ -63,6 +66,7 @@ const {
6366
kWriteFileMaxChunkSize,
6467
},
6568
copyObject,
69+
getDirent,
6670
getDirents,
6771
getOptions,
6872
getStatFsFromBinding,
@@ -1638,73 +1642,37 @@ async function mkdir(path, options) {
16381642
}
16391643

16401644
asyncfunctionreaddirRecursive(originalPath,options){
1645+
constwithFileTypes=!!options.withFileTypes;
1646+
constreaddirWithTypes=(path)=>PromisePrototypeThen(
1647+
binding.readdir(path,options.encoding,true,kUsePromises),
1648+
undefined,
1649+
handleErrorFromBinding,
1650+
);
16411651
constresult=[];
1642-
constqueue=[
1643-
[
1644-
originalPath,
1645-
awaitPromisePrototypeThen(
1646-
binding.readdir(
1647-
originalPath,
1648-
options.encoding,
1649-
!!options.withFileTypes,
1650-
kUsePromises,
1651-
),
1652-
undefined,
1653-
handleErrorFromBinding,
1654-
),
1655-
],
1656-
];
1657-
1658-
1659-
if(options.withFileTypes){
1660-
while(queue.length>0){
1661-
// If we want to implement BFS make this a `shift` call instead of `pop`
1662-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1663-
for(constdirentofgetDirents(path,readdir)){
1652+
constqueue=[[originalPath,'',awaitreaddirWithTypes(originalPath)]];
1653+
1654+
while(queue.length>0){
1655+
// If we want to implement BFS make this a `shift` call instead of `pop`
1656+
const{0: path,1: prefix,2: {0: names,1: types}}=ArrayPrototypePop(queue);
1657+
for(leti=0;i<names.length;i++){
1658+
constname=names[i];
1659+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1660+
letisDirectory;
1661+
if(withFileTypes){
1662+
constdirent=getDirent(path,name,types[i]);
16641663
ArrayPrototypePush(result,dirent);
1665-
if(dirent.isDirectory()){
1666-
constdirentPath=pathModule.join(path,dirent.name);
1667-
ArrayPrototypePush(queue,[
1668-
direntPath,
1669-
awaitPromisePrototypeThen(
1670-
binding.readdir(
1671-
direntPath,
1672-
options.encoding,
1673-
true,
1674-
kUsePromises,
1675-
),
1676-
undefined,
1677-
handleErrorFromBinding,
1678-
),
1679-
]);
1680-
}
1664+
isDirectory=dirent.isDirectory();
1665+
}else{
1666+
ArrayPrototypePush(result,relative);
1667+
// Entries that are, or may be, symbolic links to directories are followed.
1668+
consttype=types[i];
1669+
isDirectory=type===UV_DIRENT_DIR||
1670+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1671+
binding.internalModuleStat(pathModule.join(path,name))===1);
16811672
}
1682-
}
1683-
}else{
1684-
while(queue.length>0){
1685-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1686-
for(constentofreaddir){
1687-
constdirentPath=pathModule.join(path,ent);
1688-
conststat=binding.internalModuleStat(direntPath);
1689-
ArrayPrototypePush(
1690-
result,
1691-
pathModule.relative(originalPath,direntPath),
1692-
);
1693-
if(stat===1){
1694-
ArrayPrototypePush(queue,[
1695-
direntPath,
1696-
awaitPromisePrototypeThen(
1697-
binding.readdir(
1698-
direntPath,
1699-
options.encoding,
1700-
false,
1701-
kUsePromises,
1702-
),
1703-
undefined,
1704-
handleErrorFromBinding,
1705-
),
1706-
]);
1707-
}
1673+
if(isDirectory){
1674+
constdirentPath=pathModule.join(path,name);
1675+
ArrayPrototypePush(queue,[direntPath,relative,awaitreaddirWithTypes(direntPath)]);
17081676
}
17091677
}
17101678
}

‎test/known_issues/test-fs-readdir-recursive-with-buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const { readdir } = require('node:fs');
1212
const{ join }=require('node:path');
1313

1414
consttestDirPath=join(__dirname,'..','..');
15-
readdir(Buffer.from(testDirPath),{recursive: true},common.mustCall());
15+
readdir(Buffer.from(testDirPath),{recursive: true},common.mustSucceed());

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 262f0ec

Browse files
codebytereaduh95
authored andcommitted
fs: stop stat()ing every entry in recursive readdir
readdir({ recursive: true }) asked the binding for names only and then called internalModuleStat() on every entry to find the directories to descend into; with withFileTypes it built the Dirents and still stat()ed every entry that was not already a directory. Both variants also ran path.join() and path.relative() per entry to build the relative result. Ask the binding for file types in all cases, descend into directories directly, and only stat() symbolic links and entries of unknown type (which is what could point to a directory). The relative name is the parent's prefix plus the entry name. Results, their order and the symlink-following behavior are unchanged for fs.readdirSync, fs.readdir and fs.promises.readdir. The known_issues test for Buffer paths (#58892) called back without checking the error; the error now reaches the callback instead of being thrown from the completion handler, so the test asserts success to keep expressing the issue. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65487 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 5b7d02e commit 262f0ec

5 files changed

Lines changed: 112 additions & 150 deletions

File tree

‎benchmark/fs/bench-readdir.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

13-
functionmain({ n, dir, withFileTypes }){
14+
functionmain({ n, dir, withFileTypes, recursive}){
1415
withFileTypes=withFileTypes==='true';
16+
recursive=recursive==='true';
1517
constfullPath=path.resolve(__dirname,'../../',dir);
1618
bench.start();
1719
(functionr(cntr){
1820
if(cntr--<=0)
1921
returnbench.end(n);
20-
fs.readdir(fullPath,{ withFileTypes },()=>{
22+
fs.readdir(fullPath,{ withFileTypes, recursive},()=>{
2123
r(cntr);
2224
});
2325
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

1314

14-
functionmain({ n, dir, withFileTypes }){
15+
functionmain({ n, dir, withFileTypes, recursive}){
1516
withFileTypes=withFileTypes==='true';
17+
recursive=recursive==='true';
1618
constfullPath=path.resolve(__dirname,'../../',dir);
1719
bench.start();
1820
for(leti=0;i<n;i++){
19-
fs.readdirSync(fullPath,{ withFileTypes });
21+
fs.readdirSync(fullPath,{ withFileTypes, recursive});
2022
}
2123
bench.end(n);
2224
}

‎lib/fs.js‎

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ const {
5757
F_OK,
5858
O_WRONLY,
5959
O_SYMLINK,
60+
UV_DIRENT_DIR,
61+
UV_DIRENT_LINK,
62+
UV_DIRENT_UNKNOWN,
6063
}=constants;
6164

6265
constpathModule=require('path');
@@ -1739,6 +1742,43 @@ function mkdirSync(path, options) {
17391742
}
17401743
}
17411744

1745+
/**
1746+
* Appends one directory's entries to `context.results` and the subdirectories
1747+
* still to visit to `context.dirs` (with the prefix their entries get in
1748+
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1749+
* result with file types, so only symbolic links and entries of unknown type
1750+
* need a stat() to find out whether they lead to a directory.
1751+
* @param {string} dir
1752+
* @param {string} prefix
1753+
* @param {[string[], number[]]} result
1754+
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1755+
*/
1756+
functioncollectRecursiveReaddirResult(dir,prefix,{0: names,1: types},context){
1757+
const{ length }=names;
1758+
for(leti=0;i<length;i++){
1759+
constname=names[i];
1760+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1761+
letisDirectory;
1762+
if(context.withFileTypes){
1763+
constdirent=getDirent(dir,name,types[i]);
1764+
ArrayPrototypePush(context.results,dirent);
1765+
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1766+
isDirectory=dirent.isDirectory()||
1767+
(dirent.isSymbolicLink()&&binding.internalModuleStat(pathModule.join(dir,name))===1);
1768+
}else{
1769+
ArrayPrototypePush(context.results,relative);
1770+
consttype=types[i];
1771+
isDirectory=type===UV_DIRENT_DIR||
1772+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1773+
binding.internalModuleStat(pathModule.join(dir,name))===1);
1774+
}
1775+
if(isDirectory){
1776+
ArrayPrototypePush(context.dirs,pathModule.join(dir,name));
1777+
ArrayPrototypePush(context.prefixes,relative);
1778+
}
1779+
}
1780+
}
1781+
17421782
/*
17431783
* An recursive algorithm for reading the entire contents of the `basePath` directory.
17441784
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1754,15 +1794,20 @@ function mkdirSync(path, options) {
17541794
functionreaddirRecursive(basePath,options,callback){
17551795
constcontext={
17561796
withFileTypes: Boolean(options.withFileTypes),
1757-
encoding: options.encoding,
1758-
basePath,
1759-
readdirResults: [],
1760-
pathsQueue: [basePath],
1797+
results: [],
1798+
dirs: [basePath],
1799+
prefixes: [''],
17611800
};
17621801

17631802
leti=0;
17641803

1765-
functionread(path){
1804+
/**
1805+
* Reads one directory from `context.dirs` and then moves on to the next
1806+
* one, or calls back once none are left.
1807+
* @param {string} path
1808+
* @param {string} prefix path of this directory relative to `basePath`
1809+
*/
1810+
functionread(path,prefix){
17661811
constreq=newFSReqCallback();
17671812
req.oncomplete=(err,result)=>{
17681813
if(err){
@@ -1771,68 +1816,28 @@ function readdirRecursive(basePath, options, callback) {
17711816
}
17721817

17731818
if(result===undefined){
1774-
callback(null,context.readdirResults);
1819+
callback(null,context.results);
17751820
return;
17761821
}
17771822

1778-
processReaddirResult({
1779-
result,
1780-
currentPath: path,
1781-
context,
1782-
});
1823+
try{
1824+
collectRecursiveReaddirResult(path,prefix,result,context);
1825+
}catch(err){
1826+
callback(err);
1827+
return;
1828+
}
17831829

1784-
if(i<context.pathsQueue.length){
1785-
read(context.pathsQueue[i++]);
1830+
if(i<context.dirs.length){
1831+
read(context.dirs[i],context.prefixes[i++]);
17861832
}else{
1787-
callback(null,context.readdirResults);
1833+
callback(null,context.results);
17881834
}
17891835
};
17901836

1791-
binding.readdir(
1792-
path,
1793-
context.encoding,
1794-
context.withFileTypes,
1795-
req,
1796-
);
1797-
}
1798-
1799-
read(context.pathsQueue[i++]);
1800-
}
1801-
1802-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1803-
// The first array is the names, and the second array is the types.
1804-
// They are guaranteed to be the same length; hence, setting `length` to the length
1805-
// of the first array within the result.
1806-
constprocessReaddirResult=(args)=>(args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
1807-
1808-
functionhandleDirents({ result, currentPath, context }){
1809-
const{0: names,1: types}=result;
1810-
const{ length }=names;
1811-
1812-
for(leti=0;i<length;i++){
1813-
// Avoid excluding symlinks, as they are not directories.
1814-
// Refs: https://github.com/nodejs/node/issues/52663
1815-
constfullPath=pathModule.join(currentPath,names[i]);
1816-
constdirent=getDirent(currentPath,names[i],types[i]);
1817-
ArrayPrototypePush(context.readdirResults,dirent);
1818-
1819-
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
1820-
ArrayPrototypePush(context.pathsQueue,fullPath);
1821-
}
1837+
binding.readdir(path,options.encoding,true,req);
18221838
}
1823-
}
1824-
1825-
functionhandleFilePaths({ result, currentPath, context }){
1826-
for(leti=0;i<result.length;i++){
1827-
constresultPath=pathModule.join(currentPath,result[i]);
1828-
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1829-
conststat=binding.internalModuleStat(resultPath);
1830-
ArrayPrototypePush(context.readdirResults,relativeResultPath);
18311839

1832-
if(stat===1){
1833-
ArrayPrototypePush(context.pathsQueue,resultPath);
1834-
}
1835-
}
1840+
read(context.dirs[i],context.prefixes[i++]);
18361841
}
18371842

18381843
/**
@@ -1846,35 +1851,20 @@ function handleFilePaths({ result, currentPath, context }) {
18461851
functionreaddirSyncRecursive(basePath,options){
18471852
constcontext={
18481853
withFileTypes: Boolean(options.withFileTypes),
1849-
encoding: options.encoding,
1850-
basePath,
1851-
readdirResults: [],
1852-
pathsQueue: [basePath],
1854+
results: [],
1855+
dirs: [basePath],
1856+
prefixes: [''],
18531857
};
18541858

1855-
functionread(path){
1856-
constreaddirResult=binding.readdir(
1857-
path,
1858-
context.encoding,
1859-
context.withFileTypes,
1860-
);
1861-
1862-
if(readdirResult===undefined){
1863-
return;
1859+
for(leti=0;i<context.dirs.length;i++){
1860+
constdir=context.dirs[i];
1861+
constresult=binding.readdir(dir,options.encoding,true);
1862+
if(result!==undefined){
1863+
collectRecursiveReaddirResult(dir,context.prefixes[i],result,context);
18641864
}
1865-
1866-
processReaddirResult({
1867-
result: readdirResult,
1868-
currentPath: path,
1869-
context,
1870-
});
1871-
}
1872-
1873-
for(leti=0;i<context.pathsQueue.length;i++){
1874-
read(context.pathsQueue[i]);
18751865
}
18761866

1877-
returncontext.readdirResults;
1867+
returncontext.results;
18781868
}
18791869

18801870
/**

‎lib/internal/fs/promises.js‎

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const {
3232
O_WRONLY,
3333
S_IFMT,
3434
S_IFREG,
35+
UV_DIRENT_DIR,
36+
UV_DIRENT_LINK,
37+
UV_DIRENT_UNKNOWN,
3538
}=constants;
3639

3740
constbinding=internalBinding('fs');
@@ -63,6 +66,7 @@ const {
6366
kWriteFileMaxChunkSize,
6467
},
6568
copyObject,
69+
getDirent,
6670
getDirents,
6771
getOptions,
6872
getStatFsFromBinding,
@@ -1638,73 +1642,37 @@ async function mkdir(path, options) {
16381642
}
16391643

16401644
asyncfunctionreaddirRecursive(originalPath,options){
1645+
constwithFileTypes=!!options.withFileTypes;
1646+
constreaddirWithTypes=(path)=>PromisePrototypeThen(
1647+
binding.readdir(path,options.encoding,true,kUsePromises),
1648+
undefined,
1649+
handleErrorFromBinding,
1650+
);
16411651
constresult=[];
1642-
constqueue=[
1643-
[
1644-
originalPath,
1645-
awaitPromisePrototypeThen(
1646-
binding.readdir(
1647-
originalPath,
1648-
options.encoding,
1649-
!!options.withFileTypes,
1650-
kUsePromises,
1651-
),
1652-
undefined,
1653-
handleErrorFromBinding,
1654-
),
1655-
],
1656-
];
1657-
1658-
1659-
if(options.withFileTypes){
1660-
while(queue.length>0){
1661-
// If we want to implement BFS make this a `shift` call instead of `pop`
1662-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1663-
for(constdirentofgetDirents(path,readdir)){
1652+
constqueue=[[originalPath,'',awaitreaddirWithTypes(originalPath)]];
1653+
1654+
while(queue.length>0){
1655+
// If we want to implement BFS make this a `shift` call instead of `pop`
1656+
const{0: path,1: prefix,2: {0: names,1: types}}=ArrayPrototypePop(queue);
1657+
for(leti=0;i<names.length;i++){
1658+
constname=names[i];
1659+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1660+
letisDirectory;
1661+
if(withFileTypes){
1662+
constdirent=getDirent(path,name,types[i]);
16641663
ArrayPrototypePush(result,dirent);
1665-
if(dirent.isDirectory()){
1666-
constdirentPath=pathModule.join(path,dirent.name);
1667-
ArrayPrototypePush(queue,[
1668-
direntPath,
1669-
awaitPromisePrototypeThen(
1670-
binding.readdir(
1671-
direntPath,
1672-
options.encoding,
1673-
true,
1674-
kUsePromises,
1675-
),
1676-
undefined,
1677-
handleErrorFromBinding,
1678-
),
1679-
]);
1680-
}
1664+
isDirectory=dirent.isDirectory();
1665+
}else{
1666+
ArrayPrototypePush(result,relative);
1667+
// Entries that are, or may be, symbolic links to directories are followed.
1668+
consttype=types[i];
1669+
isDirectory=type===UV_DIRENT_DIR||
1670+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1671+
binding.internalModuleStat(pathModule.join(path,name))===1);
16811672
}
1682-
}
1683-
}else{
1684-
while(queue.length>0){
1685-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1686-
for(constentofreaddir){
1687-
constdirentPath=pathModule.join(path,ent);
1688-
conststat=binding.internalModuleStat(direntPath);
1689-
ArrayPrototypePush(
1690-
result,
1691-
pathModule.relative(originalPath,direntPath),
1692-
);
1693-
if(stat===1){
1694-
ArrayPrototypePush(queue,[
1695-
direntPath,
1696-
awaitPromisePrototypeThen(
1697-
binding.readdir(
1698-
direntPath,
1699-
options.encoding,
1700-
false,
1701-
kUsePromises,
1702-
),
1703-
undefined,
1704-
handleErrorFromBinding,
1705-
),
1706-
]);
1707-
}
1673+
if(isDirectory){
1674+
constdirentPath=pathModule.join(path,name);
1675+
ArrayPrototypePush(queue,[direntPath,relative,awaitreaddirWithTypes(direntPath)]);
17081676
}
17091677
}
17101678
}

‎test/known_issues/test-fs-readdir-recursive-with-buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const { readdir } = require('node:fs');
1212
const{ join }=require('node:path');
1313

1414
consttestDirPath=join(__dirname,'..','..');
15-
readdir(Buffer.from(testDirPath),{recursive: true},common.mustCall());
15+
readdir(Buffer.from(testDirPath),{recursive: true},common.mustSucceed());

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Commit 262f0ec

Browse files
codebytereaduh95
authored andcommitted
fs: stop stat()ing every entry in recursive readdir
readdir({ recursive: true }) asked the binding for names only and then called internalModuleStat() on every entry to find the directories to descend into; with withFileTypes it built the Dirents and still stat()ed every entry that was not already a directory. Both variants also ran path.join() and path.relative() per entry to build the relative result. Ask the binding for file types in all cases, descend into directories directly, and only stat() symbolic links and entries of unknown type (which is what could point to a directory). The relative name is the parent's prefix plus the entry name. Results, their order and the symlink-following behavior are unchanged for fs.readdirSync, fs.readdir and fs.promises.readdir. The known_issues test for Buffer paths (#58892) called back without checking the error; the error now reaches the callback instead of being thrown from the completion handler, so the test asserts success to keep expressing the issue. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65487 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 5b7d02e commit 262f0ec

5 files changed

Lines changed: 112 additions & 150 deletions

File tree

‎benchmark/fs/bench-readdir.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

13-
functionmain({ n, dir, withFileTypes }){
14+
functionmain({ n, dir, withFileTypes, recursive}){
1415
withFileTypes=withFileTypes==='true';
16+
recursive=recursive==='true';
1517
constfullPath=path.resolve(__dirname,'../../',dir);
1618
bench.start();
1719
(functionr(cntr){
1820
if(cntr--<=0)
1921
returnbench.end(n);
20-
fs.readdir(fullPath,{ withFileTypes },()=>{
22+
fs.readdir(fullPath,{ withFileTypes, recursive},()=>{
2123
r(cntr);
2224
});
2325
}(n));

‎benchmark/fs/bench-readdirSync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ const bench = common.createBenchmark(main, {
88
n: [10],
99
dir: ['lib','test/parallel'],
1010
withFileTypes: ['true','false'],
11+
recursive: ['true','false'],
1112
});
1213

1314

14-
functionmain({ n, dir, withFileTypes }){
15+
functionmain({ n, dir, withFileTypes, recursive}){
1516
withFileTypes=withFileTypes==='true';
17+
recursive=recursive==='true';
1618
constfullPath=path.resolve(__dirname,'../../',dir);
1719
bench.start();
1820
for(leti=0;i<n;i++){
19-
fs.readdirSync(fullPath,{ withFileTypes });
21+
fs.readdirSync(fullPath,{ withFileTypes, recursive});
2022
}
2123
bench.end(n);
2224
}

‎lib/fs.js‎

Lines changed: 71 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ const {
5757
F_OK,
5858
O_WRONLY,
5959
O_SYMLINK,
60+
UV_DIRENT_DIR,
61+
UV_DIRENT_LINK,
62+
UV_DIRENT_UNKNOWN,
6063
}=constants;
6164

6265
constpathModule=require('path');
@@ -1739,6 +1742,43 @@ function mkdirSync(path, options) {
17391742
}
17401743
}
17411744

1745+
/**
1746+
* Appends one directory's entries to `context.results` and the subdirectories
1747+
* still to visit to `context.dirs` (with the prefix their entries get in
1748+
* string results in `context.prefixes`). `result` is a `binding.readdir()`
1749+
* result with file types, so only symbolic links and entries of unknown type
1750+
* need a stat() to find out whether they lead to a directory.
1751+
* @param {string} dir
1752+
* @param {string} prefix
1753+
* @param {[string[], number[]]} result
1754+
* @param {{ withFileTypes: boolean, results: (string | Dirent)[], dirs: string[], prefixes: string[] }} context
1755+
*/
1756+
functioncollectRecursiveReaddirResult(dir,prefix,{0: names,1: types},context){
1757+
const{ length }=names;
1758+
for(leti=0;i<length;i++){
1759+
constname=names[i];
1760+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1761+
letisDirectory;
1762+
if(context.withFileTypes){
1763+
constdirent=getDirent(dir,name,types[i]);
1764+
ArrayPrototypePush(context.results,dirent);
1765+
// Follow symbolic links to directories, see https://github.com/nodejs/node/issues/52663
1766+
isDirectory=dirent.isDirectory()||
1767+
(dirent.isSymbolicLink()&&binding.internalModuleStat(pathModule.join(dir,name))===1);
1768+
}else{
1769+
ArrayPrototypePush(context.results,relative);
1770+
consttype=types[i];
1771+
isDirectory=type===UV_DIRENT_DIR||
1772+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1773+
binding.internalModuleStat(pathModule.join(dir,name))===1);
1774+
}
1775+
if(isDirectory){
1776+
ArrayPrototypePush(context.dirs,pathModule.join(dir,name));
1777+
ArrayPrototypePush(context.prefixes,relative);
1778+
}
1779+
}
1780+
}
1781+
17421782
/*
17431783
* An recursive algorithm for reading the entire contents of the `basePath` directory.
17441784
* This function does not validate `basePath` as a directory. It is passed directly to
@@ -1754,15 +1794,20 @@ function mkdirSync(path, options) {
17541794
functionreaddirRecursive(basePath,options,callback){
17551795
constcontext={
17561796
withFileTypes: Boolean(options.withFileTypes),
1757-
encoding: options.encoding,
1758-
basePath,
1759-
readdirResults: [],
1760-
pathsQueue: [basePath],
1797+
results: [],
1798+
dirs: [basePath],
1799+
prefixes: [''],
17611800
};
17621801

17631802
leti=0;
17641803

1765-
functionread(path){
1804+
/**
1805+
* Reads one directory from `context.dirs` and then moves on to the next
1806+
* one, or calls back once none are left.
1807+
* @param {string} path
1808+
* @param {string} prefix path of this directory relative to `basePath`
1809+
*/
1810+
functionread(path,prefix){
17661811
constreq=newFSReqCallback();
17671812
req.oncomplete=(err,result)=>{
17681813
if(err){
@@ -1771,68 +1816,28 @@ function readdirRecursive(basePath, options, callback) {
17711816
}
17721817

17731818
if(result===undefined){
1774-
callback(null,context.readdirResults);
1819+
callback(null,context.results);
17751820
return;
17761821
}
17771822

1778-
processReaddirResult({
1779-
result,
1780-
currentPath: path,
1781-
context,
1782-
});
1823+
try{
1824+
collectRecursiveReaddirResult(path,prefix,result,context);
1825+
}catch(err){
1826+
callback(err);
1827+
return;
1828+
}
17831829

1784-
if(i<context.pathsQueue.length){
1785-
read(context.pathsQueue[i++]);
1830+
if(i<context.dirs.length){
1831+
read(context.dirs[i],context.prefixes[i++]);
17861832
}else{
1787-
callback(null,context.readdirResults);
1833+
callback(null,context.results);
17881834
}
17891835
};
17901836

1791-
binding.readdir(
1792-
path,
1793-
context.encoding,
1794-
context.withFileTypes,
1795-
req,
1796-
);
1797-
}
1798-
1799-
read(context.pathsQueue[i++]);
1800-
}
1801-
1802-
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
1803-
// The first array is the names, and the second array is the types.
1804-
// They are guaranteed to be the same length; hence, setting `length` to the length
1805-
// of the first array within the result.
1806-
constprocessReaddirResult=(args)=>(args.context.withFileTypes ? handleDirents(args) : handleFilePaths(args));
1807-
1808-
functionhandleDirents({ result, currentPath, context }){
1809-
const{0: names,1: types}=result;
1810-
const{ length }=names;
1811-
1812-
for(leti=0;i<length;i++){
1813-
// Avoid excluding symlinks, as they are not directories.
1814-
// Refs: https://github.com/nodejs/node/issues/52663
1815-
constfullPath=pathModule.join(currentPath,names[i]);
1816-
constdirent=getDirent(currentPath,names[i],types[i]);
1817-
ArrayPrototypePush(context.readdirResults,dirent);
1818-
1819-
if(dirent.isDirectory()||binding.internalModuleStat(fullPath)===1){
1820-
ArrayPrototypePush(context.pathsQueue,fullPath);
1821-
}
1837+
binding.readdir(path,options.encoding,true,req);
18221838
}
1823-
}
1824-
1825-
functionhandleFilePaths({ result, currentPath, context }){
1826-
for(leti=0;i<result.length;i++){
1827-
constresultPath=pathModule.join(currentPath,result[i]);
1828-
constrelativeResultPath=pathModule.relative(context.basePath,resultPath);
1829-
conststat=binding.internalModuleStat(resultPath);
1830-
ArrayPrototypePush(context.readdirResults,relativeResultPath);
18311839

1832-
if(stat===1){
1833-
ArrayPrototypePush(context.pathsQueue,resultPath);
1834-
}
1835-
}
1840+
read(context.dirs[i],context.prefixes[i++]);
18361841
}
18371842

18381843
/**
@@ -1846,35 +1851,20 @@ function handleFilePaths({ result, currentPath, context }) {
18461851
functionreaddirSyncRecursive(basePath,options){
18471852
constcontext={
18481853
withFileTypes: Boolean(options.withFileTypes),
1849-
encoding: options.encoding,
1850-
basePath,
1851-
readdirResults: [],
1852-
pathsQueue: [basePath],
1854+
results: [],
1855+
dirs: [basePath],
1856+
prefixes: [''],
18531857
};
18541858

1855-
functionread(path){
1856-
constreaddirResult=binding.readdir(
1857-
path,
1858-
context.encoding,
1859-
context.withFileTypes,
1860-
);
1861-
1862-
if(readdirResult===undefined){
1863-
return;
1859+
for(leti=0;i<context.dirs.length;i++){
1860+
constdir=context.dirs[i];
1861+
constresult=binding.readdir(dir,options.encoding,true);
1862+
if(result!==undefined){
1863+
collectRecursiveReaddirResult(dir,context.prefixes[i],result,context);
18641864
}
1865-
1866-
processReaddirResult({
1867-
result: readdirResult,
1868-
currentPath: path,
1869-
context,
1870-
});
1871-
}
1872-
1873-
for(leti=0;i<context.pathsQueue.length;i++){
1874-
read(context.pathsQueue[i]);
18751865
}
18761866

1877-
returncontext.readdirResults;
1867+
returncontext.results;
18781868
}
18791869

18801870
/**

‎lib/internal/fs/promises.js‎

Lines changed: 32 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ const {
3232
O_WRONLY,
3333
S_IFMT,
3434
S_IFREG,
35+
UV_DIRENT_DIR,
36+
UV_DIRENT_LINK,
37+
UV_DIRENT_UNKNOWN,
3538
}=constants;
3639

3740
constbinding=internalBinding('fs');
@@ -63,6 +66,7 @@ const {
6366
kWriteFileMaxChunkSize,
6467
},
6568
copyObject,
69+
getDirent,
6670
getDirents,
6771
getOptions,
6872
getStatFsFromBinding,
@@ -1638,73 +1642,37 @@ async function mkdir(path, options) {
16381642
}
16391643

16401644
asyncfunctionreaddirRecursive(originalPath,options){
1645+
constwithFileTypes=!!options.withFileTypes;
1646+
constreaddirWithTypes=(path)=>PromisePrototypeThen(
1647+
binding.readdir(path,options.encoding,true,kUsePromises),
1648+
undefined,
1649+
handleErrorFromBinding,
1650+
);
16411651
constresult=[];
1642-
constqueue=[
1643-
[
1644-
originalPath,
1645-
awaitPromisePrototypeThen(
1646-
binding.readdir(
1647-
originalPath,
1648-
options.encoding,
1649-
!!options.withFileTypes,
1650-
kUsePromises,
1651-
),
1652-
undefined,
1653-
handleErrorFromBinding,
1654-
),
1655-
],
1656-
];
1657-
1658-
1659-
if(options.withFileTypes){
1660-
while(queue.length>0){
1661-
// If we want to implement BFS make this a `shift` call instead of `pop`
1662-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1663-
for(constdirentofgetDirents(path,readdir)){
1652+
constqueue=[[originalPath,'',awaitreaddirWithTypes(originalPath)]];
1653+
1654+
while(queue.length>0){
1655+
// If we want to implement BFS make this a `shift` call instead of `pop`
1656+
const{0: path,1: prefix,2: {0: names,1: types}}=ArrayPrototypePop(queue);
1657+
for(leti=0;i<names.length;i++){
1658+
constname=names[i];
1659+
constrelative=prefix==='' ? name : `${prefix}${pathModule.sep}${name}`;
1660+
letisDirectory;
1661+
if(withFileTypes){
1662+
constdirent=getDirent(path,name,types[i]);
16641663
ArrayPrototypePush(result,dirent);
1665-
if(dirent.isDirectory()){
1666-
constdirentPath=pathModule.join(path,dirent.name);
1667-
ArrayPrototypePush(queue,[
1668-
direntPath,
1669-
awaitPromisePrototypeThen(
1670-
binding.readdir(
1671-
direntPath,
1672-
options.encoding,
1673-
true,
1674-
kUsePromises,
1675-
),
1676-
undefined,
1677-
handleErrorFromBinding,
1678-
),
1679-
]);
1680-
}
1664+
isDirectory=dirent.isDirectory();
1665+
}else{
1666+
ArrayPrototypePush(result,relative);
1667+
// Entries that are, or may be, symbolic links to directories are followed.
1668+
consttype=types[i];
1669+
isDirectory=type===UV_DIRENT_DIR||
1670+
((type===UV_DIRENT_LINK||type===UV_DIRENT_UNKNOWN)&&
1671+
binding.internalModuleStat(pathModule.join(path,name))===1);
16811672
}
1682-
}
1683-
}else{
1684-
while(queue.length>0){
1685-
const{0: path,1: readdir}=ArrayPrototypePop(queue);
1686-
for(constentofreaddir){
1687-
constdirentPath=pathModule.join(path,ent);
1688-
conststat=binding.internalModuleStat(direntPath);
1689-
ArrayPrototypePush(
1690-
result,
1691-
pathModule.relative(originalPath,direntPath),
1692-
);
1693-
if(stat===1){
1694-
ArrayPrototypePush(queue,[
1695-
direntPath,
1696-
awaitPromisePrototypeThen(
1697-
binding.readdir(
1698-
direntPath,
1699-
options.encoding,
1700-
false,
1701-
kUsePromises,
1702-
),
1703-
undefined,
1704-
handleErrorFromBinding,
1705-
),
1706-
]);
1707-
}
1673+
if(isDirectory){
1674+
constdirentPath=pathModule.join(path,name);
1675+
ArrayPrototypePush(queue,[direntPath,relative,awaitreaddirWithTypes(direntPath)]);
17081676
}
17091677
}
17101678
}

‎test/known_issues/test-fs-readdir-recursive-with-buffer.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const { readdir } = require('node:fs');
1212
const{ join }=require('node:path');
1313

1414
consttestDirPath=join(__dirname,'..','..');
15-
readdir(Buffer.from(testDirPath),{recursive: true},common.mustCall());
15+
readdir(Buffer.from(testDirPath),{recursive: true},common.mustSucceed());

0 commit comments

Comments
 (0)