From 73757a5f421be9a658f2bc4a22cf8e1c0a6dc23d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vinicius=20Louren=C3=A7o?= <12551007+H4ad@users.noreply.github.com> Date: Sun, 15 Oct 2023 07:55:27 -0300 Subject: [PATCH] benchmark: fix race condition on fs benchs PR-URL: https://github.com/nodejs/node/pull/50035 Reviewed-By: Yagiz Nizipli Reviewed-By: Benjamin Gruenbaum Reviewed-By: Matteo Collina --- benchmark/fs/readfile-partitioned.js | 38 ++++++++++++++------- benchmark/fs/readfile-permission-enabled.js | 29 +++++++++------- benchmark/fs/readfile-promises.js | 35 +++++++++++-------- benchmark/fs/readfile.js | 35 +++++++++++-------- benchmark/fs/writefile-promises.js | 29 +++++++++------- 5 files changed, 99 insertions(+), 67 deletions(-) diff --git a/benchmark/fs/readfile-partitioned.js b/benchmark/fs/readfile-partitioned.js index b0183709e9f1e9..5a2004873e9fa0 100644 --- a/benchmark/fs/readfile-partitioned.js +++ b/benchmark/fs/readfile-partitioned.js @@ -14,7 +14,6 @@ const filename = path.resolve(__dirname, `.removeme-benchmark-garbage-${process.pid}`); const fs = require('fs'); const zlib = require('zlib'); -const assert = require('assert'); const bench = common.createBenchmark(main, { duration: [5], @@ -35,20 +34,28 @@ function main({ len, duration, concurrent, encoding }) { const zipData = Buffer.alloc(1024, 'a'); + let waitConcurrent = 0; + + // Plus one because of zip + const targetConcurrency = concurrent + 1; + const startedAt = Date.now(); + const endAt = startedAt + (duration * 1000); + let reads = 0; let zips = 0; - let benchEnded = false; + bench.start(); - setTimeout(() => { + + function stop() { const totalOps = reads + zips; - benchEnded = true; bench.end(totalOps); + try { fs.unlinkSync(filename); } catch { // Continue regardless of error. } - }, duration * 1000); + } function read() { fs.readFile(filename, encoding, afterRead); @@ -56,11 +63,6 @@ function main({ len, duration, concurrent, encoding }) { function afterRead(er, data) { if (er) { - if (er.code === 'ENOENT') { - // Only OK if unlinked by the timer from main. - assert.ok(benchEnded); - return; - } throw er; } @@ -68,8 +70,13 @@ function main({ len, duration, concurrent, encoding }) { throw new Error('wrong number of bytes returned'); reads++; - if (!benchEnded) + const benchEnded = Date.now() >= endAt; + + if (benchEnded && (++waitConcurrent) === targetConcurrency) { + stop(); + } else if (!benchEnded) { read(); + } } function zip() { @@ -81,12 +88,17 @@ function main({ len, duration, concurrent, encoding }) { throw er; zips++; - if (!benchEnded) + const benchEnded = Date.now() >= endAt; + + if (benchEnded && (++waitConcurrent) === targetConcurrency) { + stop(); + } else if (!benchEnded) { zip(); + } } // Start reads - while (concurrent-- > 0) read(); + for (let i = 0; i < concurrent; i++) read(); // Start a competing zip zip(); diff --git a/benchmark/fs/readfile-permission-enabled.js b/benchmark/fs/readfile-permission-enabled.js index 6f762a402dfb5f..46f20be6a0b06e 100644 --- a/benchmark/fs/readfile-permission-enabled.js +++ b/benchmark/fs/readfile-permission-enabled.js @@ -5,7 +5,6 @@ const common = require('../common.js'); const fs = require('fs'); -const assert = require('assert'); const tmpdir = require('../../test/common/tmpdir'); tmpdir.refresh(); @@ -36,18 +35,24 @@ function main({ len, duration, concurrent, encoding }) { data = null; let reads = 0; - let benchEnded = false; + let waitConcurrent = 0; + + const startedAt = Date.now(); + const endAt = startedAt + (duration * 1000); + bench.start(); - setTimeout(() => { - benchEnded = true; + + function stop() { bench.end(reads); + try { fs.unlinkSync(filename); } catch { // Continue regardless of error. } + process.exit(0); - }, duration * 1000); + } function read() { fs.readFile(filename, encoding, afterRead); @@ -55,11 +60,6 @@ function main({ len, duration, concurrent, encoding }) { function afterRead(er, data) { if (er) { - if (er.code === 'ENOENT') { - // Only OK if unlinked by the timer from main. - assert.ok(benchEnded); - return; - } throw er; } @@ -67,9 +67,14 @@ function main({ len, duration, concurrent, encoding }) { throw new Error('wrong number of bytes returned'); reads++; - if (!benchEnded) + const benchEnded = Date.now() >= endAt; + + if (benchEnded && (++waitConcurrent) === concurrent) { + stop(); + } else if (!benchEnded) { read(); + } } - while (concurrent--) read(); + for (let i = 0; i < concurrent; i++) read(); } diff --git a/benchmark/fs/readfile-promises.js b/benchmark/fs/readfile-promises.js index b9fcab00333419..f1df92931b35a0 100644 --- a/benchmark/fs/readfile-promises.js +++ b/benchmark/fs/readfile-promises.js @@ -5,7 +5,6 @@ const common = require('../common.js'); const fs = require('fs'); -const assert = require('assert'); const tmpdir = require('../../test/common/tmpdir'); tmpdir.refresh(); @@ -35,19 +34,25 @@ function main({ len, duration, concurrent, encoding }) { fs.writeFileSync(filename, data); data = null; - let writes = 0; - let benchEnded = false; + let reads = 0; + let waitConcurrent = 0; + + const startedAt = Date.now(); + const endAt = startedAt + (duration * 1000); + bench.start(); - setTimeout(() => { - benchEnded = true; - bench.end(writes); + + function stop() { + bench.end(reads); + try { fs.unlinkSync(filename); } catch { // Continue regardless of error. } + process.exit(0); - }, duration * 1000); + } function read() { fs.promises.readFile(filename, encoding) @@ -57,21 +62,21 @@ function main({ len, duration, concurrent, encoding }) { function afterRead(er, data) { if (er) { - if (er.code === 'ENOENT') { - // Only OK if unlinked by the timer from main. - assert.ok(benchEnded); - return; - } throw er; } if (data.length !== len) throw new Error('wrong number of bytes returned'); - writes++; - if (!benchEnded) + reads++; + const benchEnded = Date.now() >= endAt; + + if (benchEnded && (++waitConcurrent) === concurrent) { + stop(); + } else if (!benchEnded) { read(); + } } - while (concurrent--) read(); + for (let i = 0; i < concurrent; i++) read(); } diff --git a/benchmark/fs/readfile.js b/benchmark/fs/readfile.js index 24c2c5401ecf9c..9f74508042205f 100644 --- a/benchmark/fs/readfile.js +++ b/benchmark/fs/readfile.js @@ -5,7 +5,6 @@ const common = require('../common.js'); const fs = require('fs'); -const assert = require('assert'); const tmpdir = require('../../test/common/tmpdir'); tmpdir.refresh(); @@ -29,30 +28,31 @@ function main({ len, duration, concurrent, encoding }) { data = null; let reads = 0; - let benchEnded = false; + let waitConcurrent = 0; + + const startedAt = Date.now(); + const endAt = startedAt + (duration * 1000); + bench.start(); - setTimeout(() => { - benchEnded = true; + + function read() { + fs.readFile(filename, encoding, afterRead); + } + + function stop() { bench.end(reads); + try { fs.unlinkSync(filename); } catch { // Continue regardless of error. } - process.exit(0); - }, duration * 1000); - function read() { - fs.readFile(filename, encoding, afterRead); + process.exit(0); } function afterRead(er, data) { if (er) { - if (er.code === 'ENOENT') { - // Only OK if unlinked by the timer from main. - assert.ok(benchEnded); - return; - } throw er; } @@ -60,9 +60,14 @@ function main({ len, duration, concurrent, encoding }) { throw new Error('wrong number of bytes returned'); reads++; - if (!benchEnded) + const benchEnded = Date.now() >= endAt; + + if (benchEnded && (++waitConcurrent) === concurrent) { + stop(); + } else if (!benchEnded) { read(); + } } - while (concurrent--) read(); + for (let i = 0; i < concurrent; i++) read(); } diff --git a/benchmark/fs/writefile-promises.js b/benchmark/fs/writefile-promises.js index 2f3fd352aa886e..41c029051bc04d 100644 --- a/benchmark/fs/writefile-promises.js +++ b/benchmark/fs/writefile-promises.js @@ -5,7 +5,6 @@ const common = require('../common.js'); const fs = require('fs'); -const assert = require('assert'); const tmpdir = require('../../test/common/tmpdir'); tmpdir.refresh(); @@ -38,11 +37,16 @@ function main({ encodingType, duration, concurrent, size }) { } let writes = 0; - let benchEnded = false; + let waitConcurrent = 0; + + const startedAt = Date.now(); + const endAt = startedAt + (duration * 1000); + bench.start(); - setTimeout(() => { - benchEnded = true; + + function stop() { bench.end(writes); + for (let i = 0; i < filesWritten; i++) { try { fs.unlinkSync(`${filename}-${i}`); @@ -50,8 +54,9 @@ function main({ encodingType, duration, concurrent, size }) { // Continue regardless of error. } } + process.exit(0); - }, duration * 1000); + } function write() { fs.promises.writeFile(`${filename}-${filesWritten++}`, chunk, encoding) @@ -61,18 +66,18 @@ function main({ encodingType, duration, concurrent, size }) { function afterWrite(er) { if (er) { - if (er.code === 'ENOENT') { - // Only OK if unlinked by the timer from main. - assert.ok(benchEnded); - return; - } throw er; } writes++; - if (!benchEnded) + const benchEnded = Date.now() >= endAt; + + if (benchEnded && (++waitConcurrent) === concurrent) { + stop(); + } else if (!benchEnded) { write(); + } } - while (concurrent--) write(); + for (let i = 0; i < concurrent; i++) write(); }