feat: expose detected timer resolution per task - #570
Closed
jerome-benoit wants to merge 1 commit into
Closed
Conversation
Add a Task.detectedResolution getter (number | undefined) populated after each run with the smallest strictly positive latency sample observed. It is undefined before the first successful run, when all samples are zero (timer too coarse to resolve any iteration), or after Task#reset(). A new estimateResolution helper is exported from src/utils.ts and from the public entry point. The implementation is purely additive: no new option, no behavior change for existing benchmarks.
commit: |
This was referenced May 30, 2026
Collaborator
Author
|
Closed in favor of #571 which consolidates this feature with #568 and #569. The audit found that |
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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a
Task.detectedResolutiongetter (number | undefined) populated after each run with the smallest strictly positive latency sample observed. A newestimateResolutionhelper is exported fromsrc/utils.tsand from the public entry point.Semantics
task.detectedResolutionis:undefinedbefore the first successful runundefinedwhen all samples of the last run were zero (timer too coarse to resolve any iteration)undefinedaftertask.reset()The smallest non-zero sample is a conservative upper bound on the effective timer resolution: a timer of resolution
Rcannot return a non-zero delta smaller thanR. It is more reliable than a nominal value carried by theTimestampProviderbecause it observes the actual resolution at the moment of measurement (subject to OS scheduler jitter, power-saving modes, etc.).Implementation
estimateResolutionis a single linear scan; the cost is negligible relative to a benchmark run. The diagnostic is always populated — no new option, no opt-in. Useful for users who want to interpretlatency.meanhonestly for sub-µs functions, or to feed downstream guards (e.g. a CI that fails whendetectedResolution > Xdue to a degraded environment).Risk
None — purely additive: new public getter on
Task, new export in the entry point. No change to existing options, types, or measurement behavior.