From 07d075adee3771d2a7f1880bc4e0c5eab99d1ab6 Mon Sep 17 00:00:00 2001 From: sonsurim Date: Fri, 30 Aug 2024 03:52:01 +0900 Subject: [PATCH 01/12] fs: refactoring declaratively with the fromAsync function - Refs: https://github.com/nodejs/node/pull/51912 --- lib/fs.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 53e4d5b829b5f1..541a186bbcb97f 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -3115,10 +3115,7 @@ function glob(pattern, options, callback) { // TODO: Use iterator helpers when available (async () => { try { - const res = []; - for await (const entry of new Glob(pattern, options).glob()) { - ArrayPrototypePush(res, entry); - } + const res = await Array.fromAsync(new Glob(pattern, options).glob()); callback(null, res); } catch (err) { callback(err); From e2d8a690223e16e527e29ea61f205a1b90e8a2c0 Mon Sep 17 00:00:00 2001 From: sonsurim Date: Fri, 30 Aug 2024 04:15:37 +0900 Subject: [PATCH 02/12] fs: replace global Array to primordials ArrayFromAsync --- lib/fs.js | 3 ++- typings/primordials.d.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index 541a186bbcb97f..fba77b6a99781e 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -41,6 +41,7 @@ const { StringPrototypeIndexOf, StringPrototypeSlice, uncurryThis, + ArrayFromAsync } = primordials; const { fs: constants } = internalBinding('constants'); @@ -3115,7 +3116,7 @@ function glob(pattern, options, callback) { // TODO: Use iterator helpers when available (async () => { try { - const res = await Array.fromAsync(new Glob(pattern, options).glob()); + const res = await ArrayFromAsync(new Glob(pattern, options).glob()); callback(null, res); } catch (err) { callback(err); diff --git a/typings/primordials.d.ts b/typings/primordials.d.ts index e5426b85b5c29b..2da1a45f9e1a39 100644 --- a/typings/primordials.d.ts +++ b/typings/primordials.d.ts @@ -111,6 +111,7 @@ declare namespace primordials { export const ArrayPrototype: typeof Array.prototype export const ArrayIsArray: typeof Array.isArray export const ArrayFrom: typeof Array.from + export const ArrayFromAsync: typeof Array.fromAsync export const ArrayOf: typeof Array.of export const ArrayPrototypeConcat: UncurryThis export const ArrayPrototypeCopyWithin: UncurryThis From dde434cc17fa47fae61ff69012f1260deac85ae8 Mon Sep 17 00:00:00 2001 From: sonsurim Date: Fri, 30 Aug 2024 04:20:59 +0900 Subject: [PATCH 03/12] fix lint error --- lib/fs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fs.js b/lib/fs.js index fba77b6a99781e..32bcbb4f417689 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -25,6 +25,7 @@ 'use strict'; const { + ArrayFromAsync, ArrayPrototypePush, BigIntPrototypeToString, Boolean, @@ -41,7 +42,6 @@ const { StringPrototypeIndexOf, StringPrototypeSlice, uncurryThis, - ArrayFromAsync } = primordials; const { fs: constants } = internalBinding('constants'); From f4025e5df16f928001466ad24127d52f120a158e Mon Sep 17 00:00:00 2001 From: sonsurim Date: Mon, 9 Sep 2024 17:16:56 +0900 Subject: [PATCH 04/12] benchmark: add fs glob benchmark PR: https://github.com/nodejs/node/pull/54644#discussion_r1737178059 --- benchmark/fs/bench-glob.js | 43 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 benchmark/fs/bench-glob.js diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js new file mode 100644 index 00000000000000..38774709b33de7 --- /dev/null +++ b/benchmark/fs/bench-glob.js @@ -0,0 +1,43 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const path = require('path'); + +const benchmarkDirectory = path.resolve(__dirname, '..', '..'); + +const configs = { + n: [1, 10, 100], + dir: ['lib', 'test/parallel', 'benchmark'], + pattern: ['**/*', '*.js', '**/**.js'], + mode: ['async', 'sync'], + recursive: ['true', 'false'] +}; + +const bench = common.createBenchmark(main, configs); + +async function main(conf) { + const fullPath = path.resolve(benchmarkDirectory, conf.dir); + const globPattern = conf.pattern; + const recursive = conf.recursive === 'true'; + + bench.start(); + + let counter = 0; + for (let i = 0; i < conf.n; i++) { + if (conf.mode === 'async') { + const matches = await new Promise((resolve, reject) => { + fs.glob(globPattern, { cwd: fullPath, recursive }, (err, files) => { + if (err) reject(err); + else resolve(files); + }); + }); + counter += matches.length; + } else if (conf.mode === 'sync') { + const matches = fs.globSync(globPattern, { cwd: fullPath, recursive }); + counter += matches.length; + } + } + + bench.end(counter); +} From 1a9ae155229f5cf66a9504793d7c24032d1c7dc5 Mon Sep 17 00:00:00 2001 From: sonsurim Date: Mon, 9 Sep 2024 17:20:16 +0900 Subject: [PATCH 05/12] lint js --- benchmark/fs/bench-glob.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js index 38774709b33de7..567e68c34f297c 100644 --- a/benchmark/fs/bench-glob.js +++ b/benchmark/fs/bench-glob.js @@ -11,7 +11,7 @@ const configs = { dir: ['lib', 'test/parallel', 'benchmark'], pattern: ['**/*', '*.js', '**/**.js'], mode: ['async', 'sync'], - recursive: ['true', 'false'] + recursive: ['true', 'false'], }; const bench = common.createBenchmark(main, configs); From db16e20437a954303a72410bce0fb0e6ca14cae4 Mon Sep 17 00:00:00 2001 From: sonsurim Date: Thu, 12 Sep 2024 22:16:29 +0900 Subject: [PATCH 06/12] refactor glob benchmark --- benchmark/fs/bench-glob.js | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js index 567e68c34f297c..82e7308bfdd528 100644 --- a/benchmark/fs/bench-glob.js +++ b/benchmark/fs/bench-glob.js @@ -16,25 +16,29 @@ const configs = { const bench = common.createBenchmark(main, configs); -async function main(conf) { - const fullPath = path.resolve(benchmarkDirectory, conf.dir); - const globPattern = conf.pattern; - const recursive = conf.recursive === 'true'; +async function main(config) { + const fullPath = path.resolve(benchmarkDirectory, config.dir); + const { pattern, recursive, mode } = config; bench.start(); let counter = 0; - for (let i = 0; i < conf.n; i++) { - if (conf.mode === 'async') { + + for (let i = 0; i < config.n; i++) { + if (mode === 'async') { const matches = await new Promise((resolve, reject) => { - fs.glob(globPattern, { cwd: fullPath, recursive }, (err, files) => { - if (err) reject(err); - else resolve(files); + fs.glob(pattern, { cwd: fullPath, recursive }, (err, files) => { + if (err) { + reject(err); + } else { + resolve(files); + }; }); }); + counter += matches.length; - } else if (conf.mode === 'sync') { - const matches = fs.globSync(globPattern, { cwd: fullPath, recursive }); + } else { + const matches = fs.globSync(pattern, { cwd: fullPath, recursive }); counter += matches.length; } } From 43ef1e4d2d82730a8c389f1b5daf749db6d8259c Mon Sep 17 00:00:00 2001 From: sonsurim Date: Thu, 12 Sep 2024 23:30:08 +0900 Subject: [PATCH 07/12] reflected review in glob benchmark --- benchmark/fs/bench-glob.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js index 82e7308bfdd528..5c1285bf48e6b6 100644 --- a/benchmark/fs/bench-glob.js +++ b/benchmark/fs/bench-glob.js @@ -7,25 +7,23 @@ const path = require('path'); const benchmarkDirectory = path.resolve(__dirname, '..', '..'); const configs = { - n: [1, 10, 100], + n: [1e3], dir: ['lib', 'test/parallel', 'benchmark'], pattern: ['**/*', '*.js', '**/**.js'], - mode: ['async', 'sync'], - recursive: ['true', 'false'], + useAsync: [true, false], + recursive: [true, false], }; const bench = common.createBenchmark(main, configs); async function main(config) { const fullPath = path.resolve(benchmarkDirectory, config.dir); - const { pattern, recursive, mode } = config; + const { pattern, recursive, useAsync } = config; bench.start(); - let counter = 0; - for (let i = 0; i < config.n; i++) { - if (mode === 'async') { + if (useAsync) { const matches = await new Promise((resolve, reject) => { fs.glob(pattern, { cwd: fullPath, recursive }, (err, files) => { if (err) { @@ -35,13 +33,10 @@ async function main(config) { }; }); }); - - counter += matches.length; } else { const matches = fs.globSync(pattern, { cwd: fullPath, recursive }); - counter += matches.length; } } - bench.end(counter); + bench.end(config.n); } From cb2591aa9e1a203c33636c823114941268d77d2b Mon Sep 17 00:00:00 2001 From: sonsurim Date: Fri, 13 Sep 2024 00:52:15 +0900 Subject: [PATCH 08/12] remove an unused variable --- benchmark/fs/bench-glob.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js index 5c1285bf48e6b6..86405f0008e18a 100644 --- a/benchmark/fs/bench-glob.js +++ b/benchmark/fs/bench-glob.js @@ -24,7 +24,7 @@ async function main(config) { for (let i = 0; i < config.n; i++) { if (useAsync) { - const matches = await new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { fs.glob(pattern, { cwd: fullPath, recursive }, (err, files) => { if (err) { reject(err); @@ -34,7 +34,7 @@ async function main(config) { }); }); } else { - const matches = fs.globSync(pattern, { cwd: fullPath, recursive }); + fs.globSync(pattern, { cwd: fullPath, recursive }); } } From 1ff400e4d3722ad835e80e7fb3f19103b03bf4b2 Mon Sep 17 00:00:00 2001 From: sonsurim Date: Fri, 13 Sep 2024 00:56:00 +0900 Subject: [PATCH 09/12] applied review on glob benchmark --- benchmark/fs/bench-glob.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js index 86405f0008e18a..eaec6ea08e354a 100644 --- a/benchmark/fs/bench-glob.js +++ b/benchmark/fs/bench-glob.js @@ -3,6 +3,7 @@ const common = require('../common'); const fs = require('fs'); const path = require('path'); +const assert = require('node:assert'); const benchmarkDirectory = path.resolve(__dirname, '..', '..'); @@ -20,11 +21,12 @@ async function main(config) { const fullPath = path.resolve(benchmarkDirectory, config.dir); const { pattern, recursive, useAsync } = config; + let noDead; bench.start(); for (let i = 0; i < config.n; i++) { if (useAsync) { - await new Promise((resolve, reject) => { + noDead = await new Promise((resolve, reject) => { fs.glob(pattern, { cwd: fullPath, recursive }, (err, files) => { if (err) { reject(err); @@ -34,9 +36,10 @@ async function main(config) { }); }); } else { - fs.globSync(pattern, { cwd: fullPath, recursive }); + noDead = fs.globSync(pattern, { cwd: fullPath, recursive }); } } bench.end(config.n); + assert.ok(noDead); } From c9764467602b803dfecc3a3a501cb79d8b534ed3 Mon Sep 17 00:00:00 2001 From: sonsurim Date: Sat, 14 Sep 2024 07:02:34 +0900 Subject: [PATCH 10/12] fix error where benchmarking type does not match in CI modify config property because boolean cannot be used --- benchmark/fs/bench-glob.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js index eaec6ea08e354a..3864900375c9ba 100644 --- a/benchmark/fs/bench-glob.js +++ b/benchmark/fs/bench-glob.js @@ -11,21 +11,21 @@ const configs = { n: [1e3], dir: ['lib', 'test/parallel', 'benchmark'], pattern: ['**/*', '*.js', '**/**.js'], - useAsync: [true, false], - recursive: [true, false], + mode: ['async', 'sync'], + recursive: ['true', 'false'], }; const bench = common.createBenchmark(main, configs); async function main(config) { const fullPath = path.resolve(benchmarkDirectory, config.dir); - const { pattern, recursive, useAsync } = config; + const { pattern, recursive, mode } = config; let noDead; bench.start(); for (let i = 0; i < config.n; i++) { - if (useAsync) { + if (mode === 'async') { noDead = await new Promise((resolve, reject) => { fs.glob(pattern, { cwd: fullPath, recursive }, (err, files) => { if (err) { From 7dbb93fcde5ff6b2113943f2c9f0b138359db4ea Mon Sep 17 00:00:00 2001 From: sonsurim Date: Sat, 14 Sep 2024 15:39:39 +0900 Subject: [PATCH 11/12] remove parts that affect benchmarking --- benchmark/fs/bench-glob.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js index 3864900375c9ba..9c6f947a95858c 100644 --- a/benchmark/fs/bench-glob.js +++ b/benchmark/fs/bench-glob.js @@ -26,15 +26,7 @@ async function main(config) { for (let i = 0; i < config.n; i++) { if (mode === 'async') { - noDead = await new Promise((resolve, reject) => { - fs.glob(pattern, { cwd: fullPath, recursive }, (err, files) => { - if (err) { - reject(err); - } else { - resolve(files); - }; - }); - }); + noDead = await fs.promises.glob(pattern, { cwd: fullPath, recursive }); } else { noDead = fs.globSync(pattern, { cwd: fullPath, recursive }); } From defe49213673606aa85c2129bf85b2e9dce5637b Mon Sep 17 00:00:00 2001 From: sonsurim Date: Tue, 24 Sep 2024 23:39:49 +0900 Subject: [PATCH 12/12] fix too many cases for benchmarking --- benchmark/fs/bench-glob.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/fs/bench-glob.js b/benchmark/fs/bench-glob.js index 9c6f947a95858c..02ecf929269054 100644 --- a/benchmark/fs/bench-glob.js +++ b/benchmark/fs/bench-glob.js @@ -9,7 +9,7 @@ const benchmarkDirectory = path.resolve(__dirname, '..', '..'); const configs = { n: [1e3], - dir: ['lib', 'test/parallel', 'benchmark'], + dir: ['lib'], pattern: ['**/*', '*.js', '**/**.js'], mode: ['async', 'sync'], recursive: ['true', 'false'],