Nit: any reason for using fs.readFileSync(__filename) instead of an arbitrary fill character? Is it for making the chunks to gzip/gunzip realistic?

@lpinca Yes, exactly that. If you have other suggestions, I’ll take them too :)

Nah, that's ok and I have no better ideas.

These options have to be reflected in the benchmark test as well (parallel/test-benchmark-zlib.js).

@BridgeAR they are no? See line 14.

As far as I can tell, inputLen and duration have to be added to the test.

Oh, true.

Actually inputLen and duration are used in the test.

Right now we just have:

runBenchmark('zlib',
             [
               'method=deflate',
               'n=1',
               'options=true',
               'type=Deflate'
             ]);

It seems this benchmark is like net benchmarks where throughput is calculated after x sec. I don't think this will be run in CI. Or I am not understanding the issue :)

Right but we have tests that run these benchmarks to confirm that they work and also to confirm that we didn't break some functionality. See test/parallel/test-benchmark-zlib.js.

I see now, I didn't read the original comment carefully. Sorry for the noise.

Done!

Should not better write be replaced instead of input.write?

I’d find it a bit surprising to override something defined with function foo(){}, tbh…?

I'm -0 on this. It's heuristic (so still might fail on a really slow or clogged machine), probably safer to call bench.end(gb) with

// Give result in GBit/s, like the net benchmarks do.
// Never return 0, so that tests won't fail on really slow machines.
const gb = (readFromOutput * 8 / (1024 ** 3)) || 1

ohh and #20128 - settings duration=1 will add a 1sec test to the suite.

Do we do this elsewhere? Otherwise it seems like it would falsify test results – what if the test is actually broken and returns 0?

Would you be be okay with the current code if it passes a stress test run?

Regarding the duration: Yeah, that’s not great. It’s not quite as bad as if it were a sequential test though…

Let me read through the test and harness a little bit more. AFAIK the test-benchmark-* set are there "mostly" for syntax, API and regression testing. Designing correctness tests for the benchmarks might be out-of-scope.
Maybe we can just relax the Error: called end() with operation count <= 0 rule anyway 🤔

Found it:

runBenchmark('zlib',
             [
               'method=deflate',
               'n=1',
               'options=true',
               'type=Deflate',
               'inputLen=1024',
               'duration=0.001'
             ],
             {
               'NODEJS_BENCHMARK_ZERO_ALLOWED': 1
             });

should get the test to pass.

Thanks, done!