Skip to content

Adopt timeMedian in remaining sync benchmark timing loops (query-benchmark.ts, incremental-benchmark.ts, benchmark.ts) #1774

Description

@carlos-alm

Summary

Grind phase 28 (issue tracking forge's timeMedian extraction in scripts/token-benchmark.ts) promoted median/round1/timeMedian into a new shared scripts/lib/bench-timing.ts module and wired timeMedian into the 3 call sites that were already unconditionally async with no new async boundary needed:

  • scripts/incremental-benchmark.ts (worker): fullBuildMs, noopRebuildMs
  • scripts/benchmark.ts (worker): noopRebuildMs

5 more sites duplicate the identical "loop N times, time each run, compute median" pattern but are currently synchronous functions called without await:

  • scripts/query-benchmark.ts: benchDepths() (used for fnDeps/fnImpact depth-scaling timings), benchDiffImpact()
  • scripts/incremental-benchmark.ts (parent process): nativeBatchMs computation, jsFallbackMs computation
  • scripts/benchmark.ts: benchQuery()

Adopting timeMedian at these sites requires converting the enclosing functions to async and threading await through their call sites — a control-flow change, not a mechanical adoption, per the titan-grind skill's "never change control flow" rule. It's also worth verifying end-to-end (these are timing-sensitive benchmark scripts with documented cold-start/warmup sensitivity — see comments in query-benchmark.ts around WARMUP_RUNS and incremental-benchmark.ts/benchmark.ts's references to #1076/#1077/#1113) that adding microtask overhead per timed iteration doesn't perturb measured latencies.

Two more sites (incremental-benchmark.ts's and benchmark.ts's "1-file rebuild" measurement) compute a median-by-timestamp over {ms, phases} pairs (need the phases of the median run, not just the numeric median) — timeMedian's current signature (() => unknown returning only a number) can't carry that side data, so adopting it there would require extending the helper's API, which is more of a forge-shaped change.

Suggested follow-up

Either:

  1. Convert the 5 sync sites to async and adopt timeMedian directly (verify no benchmark methodology regression), or
  2. Extend timeMedian's API (e.g. an overload/variant that returns {ms, value} pairs sorted by timing) to also cover the 2 side-data sites, then adopt across all 7.

scripts/lib/bench-timing.ts already has median/round1 available for reuse in either approach.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions