ordo-one / package-benchmark

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

Differing scaling factors across the same benchmark misrepresented in comaprisons. #236

Open loonatick-src opened 4 months ago

loonatick-src commented 4 months ago
Benchmark("my benchmark") { benchmark in
      ...
}

to

Benchmark("my benchmark", configuration: .init(scalingFactor: .kilo)) { benchmark in
    for _ in benchmark.scaledIterations {
        ...
    }
}

This shows up like so when using swift package benchmark baseline compare

╒══════════════════════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│         Time (wall clock) (μs) *         │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞══════════════════════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│                   prev                   │     487 │     522 │     526 │     530 │     539 │     565 │     667 │   10000 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│                 current                  │  514414 │  516686 │  518259 │  519045 │  521929 │  543344 │  543344 │      20 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│                    Δ                     │  513927 │  516164 │  517733 │  518515 │  521390 │  542779 │  542677 │   -9980 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│              Improvement %               │ -105529 │  -98882 │  -98428 │  -97833 │  -96733 │  -96067 │  -81361 │   -9980 │
╘══════════════════════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

╒══════════════════════════════════════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╤═════════╕
│             Malloc (total) *             │      p0 │     p25 │     p50 │     p75 │     p90 │     p99 │    p100 │ Samples │
╞══════════════════════════════════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════╡
│                   prev                   │       0 │       0 │       0 │       0 │       0 │       0 │       0 │   10000 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│                 current                  │       0 │       0 │       0 │       0 │       0 │       0 │       0 │      20 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│                    Δ                     │       0 │       0 │       0 │       0 │       0 │       0 │       0 │   -9980 │
├──────────────────────────────────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────┤
│              Improvement %               │       0 │       0 │       0 │       0 │       0 │       0 │       0 │   -9980 │
╘══════════════════════════════════════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╧═════════╛

It appears that the new benchmark is not scaled and this caused some confusion. Is this the intended behavior?

hassila commented 4 months ago

Thanks for the report, unfortunately away for a couple of weeks and won't have time to check on this until then, sorry for delay.

hassila commented 3 months ago

Had a Quick Look at this now, this is not expected - will have a look at it, in the meantime generating a new baseline with the new factor should make things work.

hassila commented 3 months ago

Note to self: result.timeUnits = base.timeUnits is invalid, we should instead scale the percentile results to base when they differ.