Skip to content

feat: dispatch warning event on timer saturation - #569

Closed
jerome-benoit wants to merge 1 commit into
tinylibs:mainfrom
jerome-benoit:feat/timer-saturation-warning
Closed

feat: dispatch warning event on timer saturation#569
jerome-benoit wants to merge 1 commit into
tinylibs:mainfrom
jerome-benoit:feat/timer-saturation-warning

Conversation

@jerome-benoit

Copy link
Copy Markdown
Collaborator

Summary

Add a 'warning' event to BenchEvents and TaskEvents that is dispatched on both the Bench and the Task instances when the latency samples of a task are dominated by the timer resolution.

const bench = new Bench()
bench.add('hot fn', () => regex.test(line))

bench.addEventListener('warning', evt => {
  // evt.task is typed as Task (warning is in BenchEventsWithTask)
  console.warn(`[${evt.task.name}] timer saturation detected`)
})

await bench.run()

Implementation

Saturation is detected by a new internal detectTimerSaturation(samples, mad) helper in src/utils.ts. It returns true on any of:

  • more than half of the samples are zero
  • the number of distinct sample values is below max(3, min(10, n / 1000))
  • the median absolute deviation is zero with more than 100 samples

The event is dispatched in Task#processRunResult right after this.#result is set with a 'completed' or 'aborted-with-statistics' state, before the 'cycle' event so that cycle listeners can observe the warning state if they need to.

Risk

Runtime-compatible — the event is purely informative and has no listener by default. The only observable change is widening the BenchEvents string union with 'warning', which is a soft compile-time signal for users who exhaustively switch on event types and treat unknown variants as errors. No bump beyond feat: is warranted because no existing API contract is removed or changed in shape.

Add a 'warning' event to BenchEvents and TaskEvents that is dispatched on
both the Bench and the Task instances when the latency samples of a task
are dominated by the timer resolution.

Saturation is detected by a new internal detectTimerSaturation helper in
src/utils.ts. It reports true on any of:
- more than half of the samples are zero
- the number of distinct sample values is below max(3, min(10, n / 1000))
- the median absolute deviation is zero with more than 100 samples

The event is purely informative and has no listener by default, so the
change is runtime-compatible. It only widens the BenchEvents string union,
which is a soft compile-time signal for users who exhaustively switch on
event types.
@jerome-benoit
jerome-benoit requested a review from Uzlopak as a code owner May 30, 2026 00:35
@pkg-pr-new

pkg-pr-new Bot commented May 30, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/tinylibs/tinybench@569

commit: 5159de5

@jerome-benoit

Copy link
Copy Markdown
Collaborator Author

Closed in favor of #571 which consolidates this feature with #568 and #570. The audit found that the saturation detection in this PR must operate on the raw latency samples and the raw MAD; running it after the in-place overhead correction from #568 inflates zeroCount with artificially clamped samples and zeroes the MAD on truncated distributions, both producing false positives. The consolidated PR also tightens the heuristic with an n >= 10 minimum-sample guard (avoiding systematic false positives on iterations: 1 benchmarks) and replaces the new Set(samples).size distinct-count with an O(1)-extra-space scan that exploits the sorted-ascending invariant.

@jerome-benoit
jerome-benoit deleted the feat/timer-saturation-warning branch June 25, 2026 22:00
jerome-benoit added a commit that referenced this pull request Jul 22, 2026
…on diagnostic (#571)

* feat: add timer-overhead correction, saturation warning, and resolution diagnostic

Three cooperating diagnostics for sub-microsecond benchmarking, integrated in
a single coherent code path so that each one can rely on the others:

- BenchOptions.subtractTimerOverhead (default false): when enabled, the cost
  of one timestamp provider call is calibrated once at construction time via
  the new exported calibrateTimerOverhead helper, then subtracted from each
  raw latency sample (clamped to zero) before statistics are computed.
  Samples returned by the task function via overriddenDuration are
  intentional user values and are skipped by the correction.
- 'warning' event on BenchEvents and TaskEvents, dispatched on both the
  Bench and the Task instances when the latency samples of a task are
  dominated by the timer resolution. Detection uses three OR'd criteria
  computed by detectTimerSaturation: more than half zero samples, fewer
  than max(3, min(10, n/1000)) distinct values, or zero MAD with n > 100.
  An n < 10 guard prevents false positives on unit-style benchmarks.
- Task.detectedResolution getter, populated after each run with the smallest
  strictly-positive sample value that appears at least twice (smallest
  reproducibly observed increment). Falls back to the strict minimum when
  no positive value repeats. A new estimateResolution helper is exported.

calibrateTimerOverhead (utils.ts):
- Subtracts in the provider's native type before converting to milliseconds
  (toMs(b - a) rather than toMs(b) - toMs(a)), preserving bigint precision
  on long-uptime hosts.
- Discards a configurable warmup phase (default 64 pairs) so the JIT
  reaches its steady-state tier before measurements begin.
- Returns 0 when fewer than half the back-to-back pairs produce a positive
  delta — in that regime the timer resolution exceeds the call cost and
  the positive deltas measure a tick boundary, not the call cost.
- Configurable estimator: 'median' (default), 'min', or 'p05'.

Task#processRunResult orders the diagnostics so they always reflect the raw,
uncorrected measurements:
1. sortSamples on raw latencies
2. estimateResolution on raw sorted samples
3. when overhead correction is active, compute raw statistics, evaluate
   detectTimerSaturation against the raw distribution, then apply the
   correction in-place (skipping overridden samples) and re-sort only when
   overridden samples were skipped
4. compute the final (possibly corrected) statistics
5. when no correction was applied, evaluate detectTimerSaturation against
   the final samples (raw == final in this path)

This consolidates three previously separate proposals (PRs #568/#569/#570)
into a single coherent change: composing them naively would have caused the
diagnostics to operate on the corrected-and-clamped sample set, producing
artificially small detected-resolution values and false-positive saturation
warnings on benchmarks that activate overhead subtraction.

* fix: align overridden samples and harden subtractTimerOverhead

Apply audit-driven fixes to PR #571:

* fix(task): correct overhead before sort to keep latencySamples aligned
  with isOverridden (collection order). Previous logic indexed
  isOverridden after sortSamples, corrupting both overriddenDuration
  preservation and measured-sample skip in mixed-mode tasks.
* fix(task): run timer-saturation detection on a measured-only subset so
  constant overriddenDuration values cannot trigger a spurious
  low-distinct-count warning.
* fix(bench): assert subtractTimerOverhead is incompatible with
  concurrency: 'task' (sequential calibration would not reflect
  per-iteration cost under concurrency).
* fix(bench): normalize subtractTimerOverhead with ?? false instead of
  === true to accept any falsy default consistently.
* fix(index): export detectTimerSaturation alongside the other timer
  diagnostics helpers.
* docs(types): rewrite subtractTimerOverhead JSDoc with an honest
  treatment of the max(0, x) clamp and the two caveats (concurrency,
  overriddenDuration). Remove an orphan /** block.
* test: rewrite the overriddenDuration warning test to assert
  warningCount === 0, matching the measured-only saturation behavior.

* refactor(utils): expose saturation classifier and tighten timer typing

* Add `classifyTimerSaturation` returning a `TimerSaturationReason`
  (`'zero-dominated' | 'low-distinct' | 'zero-mad'`) and re-implement
  `detectTimerSaturation` as a boolean wrapper.
* Tighten `detectTimerSaturation`/`classifyTimerSaturation` parameter
  type from `Samples` to `SortedSamples`.
* Short-circuit the distinct-value loop once the threshold is reached.
* Add `medianAbsoluteDeviation(SortedSamples)` helper.
* Rename `TimerOverheadEstimator` to `TimerOverheadEstimatorKind`.
* Replace `as unknown as number` with `as bigint` in
  `calibrateTimerOverhead`; document operator polymorphism.
* Re-export `classifyTimerSaturation`, `medianAbsoluteDeviation`,
  `TimerSaturationReason`, `TimerOverheadEstimatorKind` from the
  package entry point.

* feat(event): carry timer saturation reason on warning events

Extend `BenchEvent` with an optional `reason` payload symmetrical to
`error`. The `reason` getter is typed as
`TimerSaturationReason | undefined` for `'warning'` events and
`undefined` for every other event type.

* Move `TimerSaturationReason` from `utils.ts` to `types.ts` to align
  with the `Statistics`/`Samples` convention (types in `types.ts`,
  helpers in `utils.ts`).
* Add a `'warning'` constructor overload accepting an optional reason.
* Re-export `TimerSaturationReason` from the `./types` block in the
  package entry point.

* fix(task): align resolution and saturation diagnostics with measured-only samples

* Compute `detectedResolution` from the measured-only subset (excluding
  `overriddenDuration` samples). A constant override value is no longer
  reported as the timer grain.
* Allocate `isOverridden` unconditionally so the measured-only filter is
  also active when `subtractTimerOverhead` is disabled.
* Replace Phase 6 `computeStatistics` recomputation with the dedicated
  `medianAbsoluteDeviation` helper.
* Use `classifyTimerSaturation` and propagate the
  `TimerSaturationReason` onto the `'warning'` event payload.
* Update `Task.detectedResolution` JSDoc to reflect the measured-only
  semantics; update the `#processRunResult` ordering description.

* fix(bench): enforce subtractTimerOverhead invariant at run() and tighten options coercion

* Coerce `subtractTimerOverhead` with `=== true`, matching the sibling
  `retainSamples` form. Truthy non-boolean values from JS callers are
  now rejected.
* Re-state the constructor assert message in remediation form (action
  the user can take, not the internal cause).
* Add the same assert at the start of `run()`. `concurrency` is
  documented as a post-construction-mutable field, so the constructor
  check alone leaves the mutation path uncovered.
* Note the constraint and the dual enforcement in the
  `subtractTimerOverhead` field JSDoc.

* fix(types): make BenchLike.timerOverhead optional and readonly

Third-party `BenchLike` implementers can omit the field (semantically
equivalent to the existing `undefined` sentinel that `Task` already
handles). The `readonly` modifier matches the concrete
`Bench.timerOverhead` declaration and forbids mutation through the
interface, which `Task` reads on every cycle.

* docs(types): document subtractTimerOverhead clamp consequences honestly

Rewrite the `subtractTimerOverhead` JSDoc with a mathematically grounded
treatment:

* Statistics list refers to all fields of `Statistics`; previously
  enumerated only seven of eighteen fields.
* The `rme` inflation factor `M / (M − Ĉ)` is stated deterministically
  in the clean-shift regime, not hedged with 'potentially'.
* The collapse of `p50`, `mad`, and `aad` to zero in the
  sub-overhead regime is named explicitly with the threshold.
* Three observable consequences of the `max(0, …)` clamp are listed
  (`latency.min` may be 0; throughput substitutes the mean for clamped
  samples; criterion `'zero-dominated'` cannot distinguish clamped
  samples from genuine zeros).

* test: cover alignment, p05 estimator, run() invariant, and saturation classifier

* New `test/subtract-timer-overhead-alignment.test.ts` — exercises the
  Phase 1/2 alignment invariant on a heterogeneous run (alternating
  overridden + measured iterations) using a deterministic timestamp
  provider. Pins exact multiset counts so an off-by-one in the
  `isOverridden`/`latencySamples` index alignment fails the test.
* `test/calibrate-timer-overhead.test.ts`:
  - Replace the loose `min ≤ median * 2` assertion with a deterministic
    estimator-ordering test using a scripted ascending-pair provider.
  - Add a deterministic `'p05'` test pinning the
    `max(0, ⌈n·0.05⌉ − 1)` index math at three sample sizes.
  - Add tests for the `subtractTimerOverhead` + `concurrency: 'task'`
    constructor assert and the equivalent `run()` runtime check.
* `test/detected-resolution.test.ts`: replace the conditional
  `if (resolution !== undefined)` block with unconditional assertions.
* `test/utils-detect-timer-saturation.test.ts`: add `classifyTimerSaturation`
  parallel coverage for each criterion (returning the precise reason
  string) plus the n<10 and healthy-spread negative cases.
* New `test/warning-event-reason.test.ts` — verifies
  `BenchEvent.reason` carries the saturation reason for `'warning'`
  events and is `undefined` for other event types.

* docs(readme): document timer overhead correction, per-sample override, and timer diagnostics

* New 'Timer Overhead Correction' section covers
  `subtractTimerOverhead`, the calibration helper, and the
  `concurrency: 'task'` and sub-overhead caveats.
* New 'Per-Sample Override' section documents `overriddenDuration`
  (previously absent from the README despite being supported in code).
* New 'Timer Diagnostics' section covers `Task.detectedResolution`
  and the `'warning'` event with its `TimerSaturationReason` payload,
  plus pointers to the standalone helpers.
* Extend the `BenchEvents` listener example with a `'warning'`
  listener that reads `evt.reason`.

* fix(utils): use backticked refs for non-exported symbols in JSDoc

`computeStatistics` and `absoluteDeviationMedian` are not re-exported
from the package entry point, so `{@link …}` references to them
trigger `typedoc --treatWarningsAsErrors`. Switch them to plain
backticked code references; `{@link}` is preserved only for symbols
listed in the public exports.

* fix(index): export hrtimeNow and performanceNow timestamp providers

The README timer-overhead example and `calibrateTimerOverhead` require a
`TimestampProvider` object, but none was exported from the package entry.

* perf(task): derive detectedResolution from sorted samples

`estimateResolution` scans the sorted sample array for the first repeated
positive value instead of building a value-keyed Map over every sample,
removing an O(distinct) allocation on the default post-processing path.
Its signature is tightened to `SortedSamples` (matching its
`classifyTimerSaturation` / `detectTimerSaturation` siblings) and
resolution is computed after the working-array sort.

Also corrects the `detectedResolution` getter doc (strict-min fallback;
corrected-samples note under `subtractTimerOverhead`) and the
`#processRunResult` phase-ordering doc (`'warning'` is dispatched before
`'cycle'`/`'complete'`).

* test(utils): lock timer-saturation classifier thresholds

Cover the `zero-mad` n=100/101 boundary and the distinct-count ceiling of
10 at n=10000.

* docs(readme): clarify the coarse-timer no-op condition

Calibration returns 0 when fewer than half the pairs yield a positive
delta (C < R/2), not at a fixed 1 ms resolution.

* refactor(bench): hoist duplicated subtractTimerOverhead/concurrency assert message

The identical guard message in the constructor and `run()` is now a single
module-level constant.

* refactor(task): use hasAnyOverridden instead of reference-identity check

Branch on the named `hasAnyOverridden` boolean rather than the
`measuredOnly === latencySamples` array identity; behaviour-identical,
self-documenting.

* refactor(utils): rename calibrate options to pairs/warmupPairs

The `CalibrateTimerOverheadOptions` fields count back-to-back call pairs,
not latency samples; rename disambiguates from the codebase-wide `Samples`
meaning (unreleased option, no published break). Also document the
deliberate p05 nearest-rank choice vs `quantileSorted`.

* docs(readme): configure concurrency and threshold via constructor options

`concurrency` and `threshold` are `readonly`; the example now sets them at
construction instead of mutating them after the fact.

* docs(bench): clarify the run() re-assert guards JS-side mutation

The `run()` re-check exists to catch untyped mutation of the `readonly`
`concurrency` field after construction, not a supported reconfiguration path.

* refactor(task): unify isOverridden guard and correct its param JSDoc

Use optional chaining at both guard sites (dropping a non-null-assertion
eslint-disable), and fix the #processRunResult param doc: isOverridden is
undefined on the error / no-valid-samples path, not when overhead
correction is disabled.

* refactor(utils): guard pairs===0 explicitly in calibrateTimerOverhead

Return early on the degenerate zero-pairs input so the coarse-timer guard
(deltas.length * 2 < pairs) covers every remaining case, removing the
redundant empty-deltas check.

* fix(utils): guard non-positive pairs in calibrateTimerOverhead

`pairs < 0` fell through the coarse-timer guard and produced `NaN` (median)
or `undefined` (min/p05); `pairs <= 0` now returns 0, restoring the graceful
behaviour of the removed empty-deltas check. Covered by a regression test.

* docs(readme): clarify overriddenDuration does not bypass the timer

The timestamp provider is still invoked around the task function; only the
measured value is discarded and overhead correction skipped.

* fix(utils): reject non-finite/non-integer pair counts in calibrateTimerOverhead

`{ pairs: Infinity }` / `{ warmupPairs: Infinity }` (and NaN/non-integer) hung
or produced NaN; `Number.isInteger` guards coerce them to the existing
no-op (return 0 / skip warmup). Covered by regression tests.

* perf(task): track overridden samples by index Set, not a parallel boolean[]

Replace the always-allocated `isOverridden: boolean[]` (one slot per sample,
even with no override) with a `Set<number>` of overridden collection-order
indices, populated only on override. Eliminates an O(n) parallel array on the
default path; alignment invariant and concurrency safety preserved (index is
taken synchronously right after the sample push).

* refactor(task): extract BenchmarkResult union for #benchmark/#benchmarkSync

The identical error-XOR-samples return union was duplicated across both
methods; hoist it to a single module-local `BenchmarkResult` type (mirrors
the local-type convention in utils.ts). Type-only; no behavior change.

* docs(task): finish isOverridden -> overriddenIndices rename in #processRunResult

The 'Ordering' JSDoc block still referenced the removed `isOverridden`
array (and array-indexed it, which a Set is not); align the wording with
the `overriddenIndices` Set introduced in 15f30d9.

* test(task): rename stale isOverridden test title to overridden samples

The tracked field was renamed to `overriddenIndices` (a Set) in 15f30d9;
this aligns the last remaining `isOverridden` reference (a test title).
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