Skip to content

test(perf): measure the offload heartbeat against its own idle baseline, not a 7.5ms clock (#15221) - #15248

Merged
mrveiss merged 8 commits into
Dev_new_guifrom
issue-15221-offload-contention
Aug 29, 2026
Merged

mrveiss merged 8 commits into
Dev_new_guifrom
issue-15221-offload-contention

Conversation

@mrveiss

@mrveiss mrveiss commented Aug 28, 2026 •

Copy link
Copy Markdown
Owner

Thinking Path

test_a_scan_in_a_process_does_not_delay_the_event_loop ended with
assert in_process < 0.005 * 1.5 — a 7.5 ms ceiling on a 5 ms heartbeat, 1.5x headroom,
on the CI runner itself. Its own message said "something is still contending for this
process"
, which is simultaneously the defect it guards (the scan not being offloaded) and
the runner's ordinary weather. Through an absolute clock the two are the same event, which
is why it fired at 50.6 ms on a PR whose entire diff was two session-ownership test files.

Does #15207's work unit transfer? No — and this is the substance of the change. The
quantity here is contention, not duration, and a CPU-throughput yardstick fails it twice:

  • Timed while the scan runs, reference_workload() is starved by exactly the
    contention under test. Numerator and denominator inflate together, the ratio holds at
    ~1.0 whether or not the scan is offloaded, and the detector is gone. That is the trap
    the issue asked about, and it is fatal.
  • Timed outside the scan, it describes a different moment and a different physical
    quantity — CPU throughput, not event-loop wakeup latency. Measured here across six
    consecutive samples it swung 1.43 ms – 3.27 ms: a 2.3x spread in the denominator
    alone
    , wider than the 2.0x signal the assertion has to resolve. It would manufacture
    flake rather than absorb it.

What replaces it. The heartbeat's own idle baseline: the same 5 ms sleep, on the same
loop, in the same process, in windows a fraction of a second either side of the scan. Same
quantity, same moment, differing only in whether a scan was in flight — the #14930 move
(measure the delta the assertion always meant), not the #15207 one. Over the same six
samples the baseline held to ±1.4% where the CPU unit swung 2.3x.

Same discipline as #15207 in form: a ratio, a recorded measurement on every run, an
explicit vacuity guard, and a down-only budget. Different yardstick, because the quantity
is different. The new helper is a sibling of assert_within_work_budget in the same module
so the next contention site does not re-derive it.

What Changed

autobot_shared/perf_work_budget.py

  • New assert_within_baseline_ratio(measured_ms, baseline_ms, budget_ratio, what): asserts
    a measurement against a caller-supplied baseline of the same quantity. Reports on every
    run by both test(perf): startup benchmarks assert absolute wall-clock budgets and fail on runner load — passed and failed on the same code minutes apart #15055 routes — a [perf #15221] stdout line and a perf_baseline_ratio
    junit property.
  • Its docstring records why the CPU work unit does not transfer here, the premise the
    load-tolerance algebra rests on and the residual false-failure path it leaves open
    (see
    Verification), and two things the caller owns rather than the helper: a settling gap
    before a trailing baseline window, and the choice of statistic over each window.
  • Fails on four distinct conditions: over budget, no measurement, no baseline, zero
    baseline.

autobot-backend/code_intelligence/shared/process_offload_test.py

  • _median_tick_delay_during_scan → _tick_latency_during_scan, returning a _TickLatency
    with the busy median, the idle median, both tick counts, and the pool's availability.
    Idle ticks are collected in two 0.3 s windows bracketing the scan.
  • The absolute ceiling is gone; assert_within_baseline_ratio(busy, idle, 1.5, …) replaces it.
    The existing in-process-vs-in-thread comparison is unchanged.
  • Budget _TICK_BUDGET_VS_IDLE = 1.5, derived from measurement and documented as down-only:
    offloaded readings 0.992–1.023, in-thread 1.994–2.024.
  • New test_the_responsiveness_budget_cannot_pass_without_a_measurement — the vacuity probe.
  • Minimum-sample guards: each window must contribute ≥10 ticks or the median means nothing.
  • The junit sink fixture (recording_work_units) added, as in the benchmarks file.
  • A comment at the trailing baseline window recording that it is resampled with no
    settling gap
    deliberately — this scan leaves nothing taxing the loop behind it, and a
    workload that did would need one, or its tail would land in the baseline and shrink the
    ratio.
  • Adjacent fix, called out for review: the pytest.skip for "process pool unavailable
    here" could never fire — the helper reset _pool_unavailable_reason to None before the
    caller read it. On a host that forbids process creation the scan runs in-thread silently
    and the test reported a regression that was really an environment — the same
    environment-vs-defect confusion this issue is about. The reason is now captured before
    the reset and returned.

Verification

A genuine regression still fails. run_directory_scan was mutated to force the scan
in-process (pool = None after _get_pool()), reintroducing #12866:

E   AssertionError: a scan still delays the event loop: 10.9ms median tick in-process
    vs 10.7ms in-thread — expected a clear improvement

The relative assertion fires first, so a second run with it neutralised isolates the new one:

[perf #15221] 5ms heartbeat during an offloaded scan (30 ticks vs 111 idle):
  1.994x its own idle baseline (budget 1.5) = 10.857ms / 5.444ms idle
E   AssertionError: … 1.994x its own idle baseline (budget 1.5) … This is a ratio against
    a baseline measured on the same loop moments earlier, not a wall-clock ceiling —
    runner load moves both terms, so investigate the code, do not raise it.

Both mutations reverted from pristine copies and confirmed with diff -q; git status
shows only the two intended files.

Load tolerance — a bounded claim, not an absolute one. Not established by generating
load: this machine runs live CI and the issue forbids it. Established by the algebra of the
measure plus repeated observation, with its premise stated.

Under an external delay L the baseline reads 5+L and the measurement 5+L+tax, so the
ratio is 1 + tax/(5+L), decreasing in L. That holds only where L is comparable
across the baseline and measurement windows
— and the three windows are temporally
disjoint, spanning roughly one to two seconds end to end, on the CI runner itself. A load
burst from a sibling job that lands inside the busy window and is gone before the trailing
idle window inflates the numerator with no matching rise in the denominator, and pushes the
ratio up. That is a real false-failure path this change does not exclude.

What it buys is a much narrower one. The old absolute ceiling tripped on any load bump;
this requires a spike timed to miss both bracketing windows, and to move the median of a
~0.5 s window without moving either adjacent ~0.3 s one. The baseline is taken as two
windows bracketing the measurement rather than one before it precisely so a monotone drift
is shared by both terms. So the honest guarantee is "much less likely to fire on load",
not "impossible" — a red here is evidence to investigate, not proof of a regression on its
own. Both the helper docstring and the test say so, so the next person to hit one does not
go hunting for a defect that is not there.

Five consecutive runs reported 0.992, 1.001, 1.009, 1.010, 1.012 against a budget of 1.5:
a 2% spread with 47% headroom, while the regression sits at 1.99.

Vacuity probe — test_the_responsiveness_budget_cannot_pass_without_a_measurement
asserts AssertionError for a zero measurement, a negative measurement, an absent
measurement, a zero baseline, and both terms absent; then a positive control
(5.1 / 5.0 → 1.020x) proves the assertion is not one that can never pass. The
≥10 ticks per window guards cover the "heartbeat never ran" case upstream of the median.

Sweep — every *_test.py under autobot-backend/code_intelligence/ was searched for
time.time/time.monotonic/time.perf_counter/timeit/elapsed/duration/latency/_ms
and each hit classified. Exactly one absolute wall-clock assertion exists in the tree —
the one this PR replaces. One file in the tree takes a real clock delta at all
(process_offload_test.py). Everything else matching those tokens is a parsed or
constructed literal, a config default, or a key-presence check. The benchmarks file's own
remaining drifting budgets are #15235's and are untouched here.

Tests — process_offload_test.py: 9 passed (8 pre-existing + the vacuity probe), five
further repeats of the load-bearing test green. system_benchmarks_performance_test.py
still collects its 14 tests against the extended module.

Not verified / known limits

  • Behaviour on the CI runner under real concurrent shards; that arrives with this PR's own
    checks. The local interpreter sits below 37 declared dependency floors, so a local pass is
    weaker evidence than CI.
  • Pre-existing, inherited unchanged and now documented: the per-window statistic is a
    median, so a regression that delays only a minority of ticks still will not move it.
    Not introduced here, but it is now the documented behaviour of a shared helper, and the
    docstring tells the next caller to pick a high percentile where a tail is what matters.

Refs #15055, #15207, #15235. Closes #15221.

Model Used

Claude Opus 5 (1M context)

@github-actions

Copy link
Copy Markdown
Contributor

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

@mrveiss
mrveiss merged commit 9f08f79 into Dev_new_gui Aug 29, 2026
69 checks passed
@mrveiss
mrveiss deleted the issue-15221-offload-contention branch August 29, 2026 05:15
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