Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fs: refactoring declaratively with the Array.fromAsync function #54644

Merged
merged 12 commits into from
Sep 27, 2024
Merged

fs: refactoring declaratively with the Array.fromAsync function #54644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
07d075a
fs: refactoring declaratively with the fromAsync function
sonsurim Aug 29, 2024
e2d8a69
fs: replace global Array to primordials ArrayFromAsync
sonsurim Aug 29, 2024
dde434c
fix lint error
sonsurim Aug 29, 2024
f4025e5
benchmark: add fs glob benchmark
sonsurim Sep 9, 2024
1a9ae15
lint js
sonsurim Sep 9, 2024
db16e20
refactor glob benchmark
sonsurim Sep 12, 2024
43ef1e4
reflected review in glob benchmark
sonsurim Sep 12, 2024
cb2591a
remove an unused variable
sonsurim Sep 12, 2024
1ff400e
applied review on glob benchmark
sonsurim Sep 12, 2024
c976446
fix error where benchmarking type does not match in CI
sonsurim Sep 13, 2024
7dbb93f
remove parts that affect benchmarking
sonsurim Sep 14, 2024
defe492
fix too many cases for benchmarking
sonsurim Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions benchmark/fs/bench-glob.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict';

const common = require('../common');
const fs = require('fs');
const path = require('path');
const assert = require('node:assert');

aduh95 marked this conversation as resolved.
Show resolved Hide resolved
const benchmarkDirectory = path.resolve(__dirname, '..', '..');

const configs = {
n: [1e3],
dir: ['lib'],
pattern: ['**/*', '*.js', '**/**.js'],
mode: ['async', 'sync'],
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
recursive: ['true', 'false'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
recursive: ['true', 'false'],
recursive: [true, false],

Copy link
Contributor Author

@sonsurim sonsurim Sep 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't seem to use boolean in benchmark (error on CI), so I fixed that in this commit!

Many Thanks!

};

const bench = common.createBenchmark(main, configs);

async function main(config) {
const fullPath = path.resolve(benchmarkDirectory, config.dir);
const { pattern, recursive, mode } = config;

let noDead;
bench.start();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bench.start();
let noDead;
bench.start();


for (let i = 0; i < config.n; i++) {
if (mode === 'async') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this statement affect benchmarking speeds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedYetiDev
Thank you for the review!
I think wrapping a Promise with each iteration affects benchmarking, So I fixed it.
After benchmarking again, there seems to be a decrease in performance..🥲

So, I'm trying to close the current PR and upload a PR that only adds benchmarking code. Is that okay?

mode: async, recursive: true

                                                                                             confidence improvement accuracy (*)   (**)  (***)
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='benchmark' n=1000                        -0.55 %       ±1.61% ±2.14% ±2.79%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='lib' n=1000                              -0.16 %       ±1.88% ±2.51% ±3.27%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='test/parallel' n=1000                     1.76 %       ±3.88% ±5.19% ±6.83%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='benchmark' n=1000                 **     -3.39 %       ±2.20% ±2.93% ±3.83%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='lib' n=1000                              -0.69 %       ±2.17% ±2.89% ±3.77%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='test/parallel' n=1000                    -0.20 %       ±1.24% ±1.65% ±2.14%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='benchmark' n=1000                    -2.56 %       ±2.61% ±3.49% ±4.57%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='lib' n=1000                          -0.49 %       ±2.24% ±2.98% ±3.87%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='test/parallel' n=1000                -1.24 %       ±1.48% ±1.97% ±2.56%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 9 comparisons, you can thus expect the following amount of false-positive results:
  0.45 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.09 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedYetiDev, Sorry for the frequent comments.
I look forward to hearing from you.
Many thanks!

noDead = await fs.promises.glob(pattern, { cwd: fullPath, recursive });
} else {
noDead = fs.globSync(pattern, { cwd: fullPath, recursive });
}
}

bench.end(config.n);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bench.end(config.n);
bench.end(config.n);
assert.ok(noDead);

assert.ok(noDead);
}
6 changes: 2 additions & 4 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
'use strict';

const {
ArrayFromAsync,
ArrayPrototypePush,
BigIntPrototypeToString,
Boolean,
Expand Down Expand Up @@ -3115,10 +3116,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 ArrayFromAsync(new Glob(pattern, options).glob());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if there is a benchmark for this, but this should probably be benchmarked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedYetiDev Thanks for the review!
There are currently no benchmarks, so I'll add them!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedYetiDev
Sorry for being late!
I added the fs glob benchmark f4025e5.

The benchmark results are as follows:

node-benchmark-compare compare-glob-result.csv
                                                                                             confidence improvement accuracy (*)   (**)  (***)
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='benchmark' n=1                           0.68 %       ±1.50% ±2.01% ±2.62%
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='benchmark' n=10                          0.47 %       ±1.59% ±2.11% ±2.75%
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='benchmark' n=100                  *      0.68 %       ±0.63% ±0.84% ±1.10%
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='lib' n=1                                -0.40 %       ±1.61% ±2.15% ±2.82%
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='lib' n=10                                0.23 %       ±1.31% ±1.74% ±2.27%
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='lib' n=100                               1.67 %       ±1.67% ±2.24% ±2.96%
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='test/parallel' n=1                       0.77 %       ±0.83% ±1.11% ±1.46%
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='test/parallel' n=10             ***      3.75 %       ±1.90% ±2.55% ±3.37%
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='test/parallel' n=100              *      1.51 %       ±1.16% ±1.54% ±2.01%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='benchmark' n=1                           0.13 %       ±1.53% ±2.04% ±2.67%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='benchmark' n=10                          1.42 %       ±1.64% ±2.20% ±2.91%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='benchmark' n=100                         0.27 %       ±0.53% ±0.71% ±0.93%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='lib' n=1                        ***      1.39 %       ±0.75% ±1.00% ±1.31%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='lib' n=10                               -3.22 %       ±5.11% ±6.88% ±9.13%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='lib' n=100                               0.05 %       ±0.78% ±1.04% ±1.35%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='test/parallel' n=1                       1.09 %       ±1.37% ±1.82% ±2.38%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='test/parallel' n=10             ***      5.67 %       ±1.26% ±1.69% ±2.23%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='test/parallel' n=100                     1.22 %       ±1.55% ±2.07% ±2.72%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='benchmark' n=1                       0.39 %       ±3.01% ±4.03% ±5.27%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='benchmark' n=10                      0.72 %       ±1.31% ±1.74% ±2.27%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='benchmark' n=100                     0.35 %       ±1.11% ±1.49% ±1.97%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='lib' n=1                             1.28 %       ±1.48% ±1.99% ±2.62%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='lib' n=10                            0.33 %       ±1.02% ±1.35% ±1.76%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='lib' n=100                           0.03 %       ±0.71% ±0.94% ±1.22%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='test/parallel' n=1            *      1.49 %       ±1.44% ±1.92% ±2.51%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='test/parallel' n=10         ***      7.41 %       ±2.81% ±3.79% ±5.03%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='test/parallel' n=100        ***      1.56 %       ±0.75% ±1.00% ±1.31%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='benchmark' n=1                           -0.43 %       ±1.26% ±1.68% ±2.19%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='benchmark' n=10                          -0.12 %       ±1.12% ±1.49% ±1.94%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='benchmark' n=100                         -0.10 %       ±0.91% ±1.21% ±1.58%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='lib' n=1                                  2.79 %       ±4.15% ±5.59% ±7.39%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='lib' n=10                                 0.12 %       ±2.12% ±2.83% ±3.70%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='lib' n=100                                0.07 %       ±3.03% ±4.04% ±5.26%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='test/parallel' n=1                        0.41 %       ±1.25% ±1.66% ±2.17%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='test/parallel' n=10                       0.79 %       ±2.11% ±2.81% ±3.66%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='test/parallel' n=100                      0.08 %       ±0.92% ±1.22% ±1.59%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='benchmark' n=1                           -1.47 %       ±4.43% ±5.96% ±7.90%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='benchmark' n=10                           0.49 %       ±0.51% ±0.68% ±0.89%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='benchmark' n=100                          1.15 %       ±1.23% ±1.66% ±2.19%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='lib' n=1                                  0.57 %       ±0.91% ±1.21% ±1.58%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='lib' n=10                                -1.87 %       ±4.44% ±5.97% ±7.90%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='lib' n=100                                2.01 %       ±3.92% ±5.29% ±7.01%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='test/parallel' n=1                       -0.10 %       ±0.88% ±1.18% ±1.54%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='test/parallel' n=10                      -0.12 %       ±0.83% ±1.11% ±1.45%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='test/parallel' n=100                     -0.17 %       ±0.79% ±1.06% ±1.38%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='benchmark' n=1                 *      1.12 %       ±1.08% ±1.44% ±1.88%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='benchmark' n=10                       1.85 %       ±2.73% ±3.65% ±4.80%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='benchmark' n=100                      0.16 %       ±1.07% ±1.42% ±1.85%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='lib' n=1                              0.54 %       ±1.06% ±1.42% ±1.84%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='lib' n=10                            -0.62 %       ±1.21% ±1.62% ±2.14%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='lib' n=100                            0.66 %       ±1.14% ±1.52% ±2.00%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='test/parallel' n=1                   -2.67 %       ±5.08% ±6.85% ±9.09%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='test/parallel' n=10                  -0.12 %       ±0.73% ±0.97% ±1.26%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='test/parallel' n=100                  0.55 %       ±0.60% ±0.80% ±1.05%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='benchmark' n=1                            0.22 %       ±1.42% ±1.90% ±2.49%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='benchmark' n=10                           0.08 %       ±2.60% ±3.50% ±4.64%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='benchmark' n=100                          0.08 %       ±0.78% ±1.03% ±1.35%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='lib' n=1                                 -0.03 %       ±1.26% ±1.68% ±2.20%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='lib' n=10                                 0.38 %       ±2.30% ±3.07% ±4.03%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='lib' n=100                                2.81 %       ±3.46% ±4.65% ±6.16%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='test/parallel' n=1               ***      1.77 %       ±0.50% ±0.66% ±0.86%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='test/parallel' n=10              ***      4.75 %       ±1.45% ±1.93% ±2.52%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='test/parallel' n=100                      0.26 %       ±1.28% ±1.72% ±2.27%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='benchmark' n=1                            0.45 %       ±1.22% ±1.63% ±2.13%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='benchmark' n=10                           0.31 %       ±0.81% ±1.09% ±1.42%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='benchmark' n=100                         -0.18 %       ±0.84% ±1.13% ±1.47%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='lib' n=1                                  0.21 %       ±1.48% ±1.97% ±2.56%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='lib' n=10                                -0.23 %       ±0.57% ±0.76% ±1.00%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='lib' n=100                                0.05 %       ±1.43% ±1.92% ±2.52%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='test/parallel' n=1                **      2.56 %       ±1.63% ±2.18% ±2.88%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='test/parallel' n=10              ***      4.31 %       ±1.28% ±1.71% ±2.23%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='test/parallel' n=100               *      1.00 %       ±0.76% ±1.02% ±1.34%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='benchmark' n=1                        2.96 %       ±4.42% ±5.95% ±7.89%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='benchmark' n=10                       0.75 %       ±1.01% ±1.35% ±1.78%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='benchmark' n=100                      0.04 %       ±0.75% ±1.00% ±1.32%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='lib' n=1                              1.02 %       ±1.31% ±1.74% ±2.27%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='lib' n=10                            -0.29 %       ±1.13% ±1.52% ±1.99%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='lib' n=100                            0.26 %       ±0.70% ±0.93% ±1.21%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='test/parallel' n=1           ***      2.64 %       ±1.26% ±1.69% ±2.23%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='test/parallel' n=10          ***      5.59 %       ±1.19% ±1.59% ±2.10%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='test/parallel' n=100          **      1.59 %       ±1.12% ±1.49% ±1.96%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='benchmark' n=1                             1.62 %       ±2.70% ±3.61% ±4.72%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='benchmark' n=10                           -0.09 %       ±1.25% ±1.67% ±2.20%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='benchmark' n=100                           0.03 %       ±0.60% ±0.80% ±1.04%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='lib' n=1                                   0.87 %       ±1.34% ±1.78% ±2.32%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='lib' n=10                                  0.81 %       ±1.74% ±2.32% ±3.03%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='lib' n=100                                 0.71 %       ±1.92% ±2.56% ±3.35%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='test/parallel' n=1                        -0.14 %       ±0.75% ±1.01% ±1.33%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='test/parallel' n=10                        0.16 %       ±0.61% ±0.81% ±1.06%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='test/parallel' n=100                       0.09 %       ±1.20% ±1.60% ±2.08%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='benchmark' n=1                             0.35 %       ±1.17% ±1.57% ±2.06%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='benchmark' n=10                            0.77 %       ±0.79% ±1.05% ±1.38%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='benchmark' n=100                           1.09 %       ±1.19% ±1.60% ±2.12%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='lib' n=1                                   0.57 %       ±1.04% ±1.38% ±1.80%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='lib' n=10                                 -0.57 %       ±2.63% ±3.52% ±4.65%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='lib' n=100                                 2.33 %       ±3.84% ±5.17% ±6.86%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='test/parallel' n=1                        -0.26 %       ±0.60% ±0.80% ±1.04%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='test/parallel' n=10                        0.28 %       ±0.66% ±0.88% ±1.15%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='test/parallel' n=100                       0.05 %       ±0.93% ±1.24% ±1.61%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='benchmark' n=1                  *      1.61 %       ±1.44% ±1.94% ±2.55%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='benchmark' n=10                        0.77 %       ±3.60% ±4.82% ±6.33%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='benchmark' n=100                       1.17 %       ±1.52% ±2.05% ±2.71%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='lib' n=1                               0.88 %       ±0.93% ±1.23% ±1.60%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='lib' n=10                             -0.21 %       ±0.92% ±1.22% ±1.59%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='lib' n=100                             0.86 %       ±1.28% ±1.72% ±2.27%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='test/parallel' n=1                     0.28 %       ±0.50% ±0.66% ±0.87%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='test/parallel' n=10                    1.55 %       ±2.77% ±3.71% ±4.90%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='test/parallel' n=100            *      0.86 %       ±0.86% ±1.15% ±1.52%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 108 comparisons, you can thus expect the following amount of false-positive results:
  5.40 false positives, when considering a   5% risk acceptance (*, **, ***),
  1.08 false positives, when considering a   1% risk acceptance (**, ***),
  0.11 false positives, when considering a 0.1% risk acceptance (***)

Performance is significantly improved when asynchronous compared to synchronous.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedYetiDev
Please confirm when you have time. Many Thanks!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that benchmark is accurate. See my review comments.

Copy link
Contributor Author

@sonsurim sonsurim Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedYetiDev
Many Thanks, we were able to improve the benchmark code.
Here are the results of the improved benchmark!

The results of recursive=true useAsync=true are:

                                                                                                confidence improvement accuracy (*)   (**)  (***)
fs/bench-glob.js recursive='true' useAsync='true' pattern='*.js' dir='benchmark' n=1000                         1.03 %       ±2.95% ±3.95% ±5.21%
fs/bench-glob.js recursive='true' useAsync='true' pattern='*.js' dir='lib' n=1000                               1.14 %       ±1.67% ±2.23% ±2.90%
fs/bench-glob.js recursive='true' useAsync='true' pattern='*.js' dir='test/parallel' n=1000              *      1.53 %       ±1.32% ±1.75% ±2.28%
fs/bench-glob.js recursive='true' useAsync='true' pattern='**/*' dir='benchmark' n=1000                         0.70 %       ±1.27% ±1.70% ±2.23%
fs/bench-glob.js recursive='true' useAsync='true' pattern='**/*' dir='lib' n=1000                               0.97 %       ±2.06% ±2.75% ±3.58%
fs/bench-glob.js recursive='true' useAsync='true' pattern='**/*' dir='test/parallel' n=1000                     0.73 %       ±1.55% ±2.07% ±2.69%
fs/bench-glob.js recursive='true' useAsync='true' pattern='**/**.js' dir='benchmark' n=1000                    -0.88 %       ±1.31% ±1.74% ±2.27%
fs/bench-glob.js recursive='true' useAsync='true' pattern='**/**.js' dir='lib' n=1000                           1.23 %       ±2.51% ±3.35% ±4.36%
fs/bench-glob.js recursive='true' useAsync='true' pattern='**/**.js' dir='test/parallel' n=1000                 0.66 %       ±1.42% ±1.89% ±2.47%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 9 comparisons, you can thus expect the following amount of false-positive results:
  0.45 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.09 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

The results of recursive=true useAsync=false are:

                                                                                                 confidence improvement accuracy (*)   (**)  (***)
fs/bench-glob.js recursive='true' useAsync='false' pattern='*.js' dir='benchmark' n=1000                         0.33 %       ±2.25% ±3.00% ±3.92%
fs/bench-glob.js recursive='true' useAsync='false' pattern='*.js' dir='lib' n=1000                               2.97 %       ±4.11% ±5.52% ±7.27%
fs/bench-glob.js recursive='true' useAsync='false' pattern='*.js' dir='test/parallel' n=1000                     1.08 %       ±1.16% ±1.54% ±2.01%
fs/bench-glob.js recursive='true' useAsync='false' pattern='**/*' dir='benchmark' n=1000                         0.59 %       ±1.82% ±2.43% ±3.16%
fs/bench-glob.js recursive='true' useAsync='false' pattern='**/*' dir='lib' n=1000                               0.40 %       ±1.85% ±2.47% ±3.22%
fs/bench-glob.js recursive='true' useAsync='false' pattern='**/*' dir='test/parallel' n=1000                     1.25 %       ±1.52% ±2.02% ±2.63%
fs/bench-glob.js recursive='true' useAsync='false' pattern='**/**.js' dir='benchmark' n=1000                     0.33 %       ±1.56% ±2.08% ±2.71%
fs/bench-glob.js recursive='true' useAsync='false' pattern='**/**.js' dir='lib' n=1000                           0.42 %       ±1.68% ±2.24% ±2.91%
fs/bench-glob.js recursive='true' useAsync='false' pattern='**/**.js' dir='test/parallel' n=1000                 1.03 %       ±1.15% ±1.53% ±1.99%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 9 comparisons, you can thus expect the following amount of false-positive results:
  0.45 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.09 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looks like a bit of improvement

callback(null, res);
} catch (err) {
callback(err);
Expand Down
1 change: 1 addition & 0 deletions typings/primordials.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof Array.prototype.concat>
export const ArrayPrototypeCopyWithin: UncurryThis<typeof Array.prototype.copyWithin>
Expand Down
Loading