benchmark: fix race condition on fs benchs

PR-URL: #50035 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
nodejs · Oct 15, 2023 · 33c87ec · 33c87ec
1 parent 0f0dd1a
commit 33c87ec
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 67 deletions.
diff --git 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,41 +34,49 @@ 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);
   }
 
   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');
 
     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
@@ -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,40 +35,46 @@ 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);
   }
 
   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');
 
     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
@@ -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
@@ -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,40 +28,46 @@ 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;
     }
 
     if (data.length !== len)
       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
@@ -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,20 +37,26 @@ 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}`);
       } catch {
         // 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();
 }