ordo-one / package-benchmark

Swift benchmark runner with many performance metrics and great CI support
Apache License 2.0
326 stars 25 forks source link

Time is not scaled with scaling factor, but the result is #277

Open MahdiBM opened 2 months ago

MahdiBM commented 2 months ago

See #276 for root.

The Benchmark:

let benchmarks = {
    Benchmark.defaultConfiguration = .init(
        metrics: [.cpuTotal],
        timeUnits: .nanoseconds,
        warmupIterations: 1,
        scalingFactor: .one,
        maxDuration: .seconds(1000),
        maxIterations: 1
    )

    func defaultCounter() -> Int { 10 }

    func dummyCounter(_ count: Int) {
        for index in 0 ..< count {
            blackHole(index)
        }
    }

    Benchmark(
        "CountNumbersBenchmark",
        configuration: .init(
            scalingFactor: .kilo,
            thresholds: [
                .cpuTotal: .init(
                    /// Tolerate up to 1% of difference compared to the threshold.
                    relative: [.p90: 1],
                    /// Tolerate up to 80ms of difference compared to the threshold.
                    absolute: [.p90: 80_000_000]
                )
            ]
        )
    ) { _ in
        for _ in 0 ... 1_000_000 {
            dummyCounter(defaultCounter() * 1000)
        }
    }
}

The Result

✅ Pull request has no significant performance differences ✅

Benchmark check running at 2024-09-18 02:48:03 UTC

Baseline mmbm-no-twice-benchmark-run is EQUAL to the defined absolute baseline thresholds. (--check-absolute)

Baseline mmbm-no-twice-benchmark-run

Host 0d12d771afd8 with 64 aarch64 processors with 125 GB memory, running:
#24~22.04.1-Ubuntu SMP Sat Jul 20 10:48:15 UTC 2024

MyBenchmarks

CountNumbersBenchmark

Metric p0 p25 p50 p75 p90 p99 p100 Samples
Time (total CPU) (ns) * 7710000 7710000 7710000 7710000 7710000 7710000 7710000 1

What's wrong?

Those 7710000 numbers are NOT in nanoseconds (1e-9), but in microseconds (1e-6). This is likely because i've set the scalingFactor to .kilo, aka 1e3. So that time label should have also been multiplied by the 1e3. So nanos x kilos == micros == 1e-9 x 1e3 == 1e-6.

hassila commented 2 months ago

By default the output is scaled with the scalingFactor to provide the per-real-iteration units - any output marked with * signifies scaled output with the scaling factor. If you want to get the raw output, you need to provide the --scale option;

--scale Show the metrics in the scale of the outer loop only (without applying the inner loop scalingFactor to the output)

MahdiBM commented 2 months ago

The problem with raw output is that the numbers won't fit in the chart and will be truncated. That's the reason why I'm using scalingFactor in the first place.

hassila commented 2 months ago

Right, but that is a different issue?

I am responding to "What's wrong" - that output is scaled, so not sure what the problem is (truncation or output would be a different thing)?

MahdiBM commented 2 months ago

Ok but then what the table says is still incorrect ... it shouldn't misinform by reporting the wrong amount of nanoseconds.

I'm also open to doing something to make the output fit the columns. Perhaps if there are trailing zeros, it should just use the e notation instead of truncating. Would that be reasonable?