Skip to content

fix(walltime): normalize minimum time only once#127

Open
lukapeschke wants to merge 1 commit into
CodSpeedHQ:masterfrom
lukapeschke:fix-formatting-sub-ms-benchmarks
Open

fix(walltime): normalize minimum time only once#127
lukapeschke wants to merge 1 commit into
CodSpeedHQ:masterfrom
lukapeschke:fix-formatting-sub-ms-benchmarks

Conversation

@lukapeschke

Copy link
Copy Markdown

In the local walltime mode results table, the "Time (best)" column divides by iter_per_round twice: once in BenchmarkStats.from_list, and again in _print_benchmark_table.

This is only an issue for fast benchmarks: since iter_per_round defaults to 1ms, the time for benchmarks taking more than 1ms get divided by 1 twice.

MRE

repro.py:

import time


def test_sleep_100us(benchmark):
    benchmark(time.sleep, 0.0001)


def test_sleep_5ms(benchmark):
    benchmark(time.sleep, 0.005)
# pytest-benchmark correctly displays ~100us for for the 100us test and ~5000us for the 5ms test
❯ uv run pytest bench_repro.py

[...]


---------------------------------------------------------------------------------------- benchmark: 2 tests ----------------------------------------------------------------------------------------
Name (time in us)            Min                   Max                  Mean            StdDev                Median               IQR            Outliers         OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_sleep_100us        100.8900 (1.0)        130.8170 (1.0)        101.8938 (1.0)      1.8603 (1.0)        101.5320 (1.0)      0.6710 (1.0)       336;434  9,814.1441 (1.0)        8908           1
test_sleep_5ms        5,002.3020 (49.58)    5,064.6200 (38.72)    5,015.6336 (49.22)    6.9418 (3.73)     5,016.1335 (49.40)    1.3230 (1.97)        37;50    199.3766 (0.02)        200           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

# pytest-codspeed is correct for the 5ms test, but displays only 10us for the 100us test
❯ uv run pytest bench_repro.py --codspeed

[...]

                         Benchmark Results                          
┏━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━┓
┃        Benchmark ┃ Time (best) ┃ Rel. StdDev ┃ Run time ┃  Iters ┃
┡━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━┩
│ test_sleep_100us │     10.11µs │        1.4% │    3.00s │ 29,410 │
│   test_sleep_5ms │       5.0ms │        1.3% │    3.00s │    596 │

In the local walltime mode results table, the "Time (best)" column
divides by `iter_per_round` twice: once in `BenchmarkStats.from_list`,
and again in `_print_benchmark_table`.

This is only an issue for fast benchmarks: since `iter_per_round`
defaults to 1ms, the time for benchmarks taking more than 1ms get
divided by 1 twice.

Signed-off-by: Luka Peschke <mail@lukapeschke.com>
@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a double-division bug in the walltime benchmark results table where Time (best) was incorrectly divided by iter_per_round twice for fast benchmarks.

  • BenchmarkStats.from_list already normalizes all timing fields (including min_ns) by iter_per_round at construction time (line 107), so the extra / bench.stats.iter_per_round in _print_benchmark_table was redundant and caused incorrect values for any benchmark that needed more than one iteration per round (i.e., benchmarks faster than ~1ms).
  • The fix is a single-line removal of the redundant division. For slow benchmarks where iter_per_round == 1, behavior is unchanged; for fast benchmarks, the displayed "Time (best)" now correctly reflects per-iteration time.

Confidence Score: 5/5

Safe to merge. The change is a targeted single-line removal of a redundant division that was already performed during stats construction.

The fix correctly identifies that BenchmarkStats.from_list already divides all timing values by iter_per_round at line 107, making the downstream division in _print_benchmark_table redundant. For slow benchmarks (iter_per_round == 1) the behavior is identical. For fast benchmarks the displayed value now matches the actual per-iteration time. No other fields in the table were affected by this bug, and no other callers of min_ns perform this extra division.

No files require special attention.

Important Files Changed

Filename Overview
src/pytest_codspeed/instruments/walltime.py Single-line fix removing the double division of min_ns by iter_per_round in the benchmark results table; BenchmarkStats.from_list already normalizes all stats fields per-iteration.

Sequence Diagram

sequenceDiagram
    participant M as measure()
    participant S as BenchmarkStats.from_list()
    participant T as _print_benchmark_table()

    M->>S: times_per_round_ns, iter_per_round
    Note over S: times_ns = [t / iter_per_round for t in times_per_round_ns]
    Note over S: min_ns = min(times_ns)  ← already per-iteration
    S-->>M: BenchmarkStats (min_ns already normalized)

    M->>T: bench.stats.min_ns
    Note over T: BEFORE: format_time(bench.stats.min_ns / iter_per_round) ← double division ❌
    Note over T: AFTER:  format_time(bench.stats.min_ns)                  ← correct ✅
Loading

Reviews (1): Last reviewed commit: "fix(walltime): normalize minimum time on..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant