Skip to content

feat(browser): add browser concurrency benchmark - #320

Open
kisernl wants to merge 26 commits into
masterfrom
add-browser-concurrency-bench
Open

feat(browser): add browser concurrency benchmark#320
kisernl wants to merge 26 commits into
masterfrom
add-browser-concurrency-bench

Conversation

@kisernl

@kisernl kisernl commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Adds a browser benchmark that measures what happens to a provider as the number of simultaneous browser sessions grows. The existing browser benchmarks (browser, browser-throughput) each drive one session at a time, so nothing in the repo answered "does this provider still work at 50 sessions, and what does it cost you."

The workflow half is already on master, from #300, #301, #305, #307 and #309. This PR is the benchmark code those runs execute.

What it measures

Five concurrency levels: 1, 5, 10, 25, 50 simultaneous sessions. Each level is a separate cell, comparable to the storage benchmark's 1MB/4MB/10MB/16MB dimensions, and the interesting output is the shape of the curve rather than any single number.

Per level:

metric meaning
createMs wall clock to provision all N sessions in parallel
connectMs wall clock to open N CDP connections in parallel
loopMs one session's ten-action loop while N sessions run together
perActionType median/p95 per action kind, so you can see which operation degrades
perSessionActionsPerSecond throughput one session sees, the degradation signal
maxConcurrentActions measured peak simultaneity, not inferred
success rate sessions completing every action they attempted, over sessions attempted

How a level runs

Each level runs a barrier protocol, so the sessions genuinely overlap instead of merely being requested together:

create-all-c50    Promise.allSettled over 50 session.create calls
connect-all-c50   Promise.allSettled over 50 connectOverCDP calls
─────── BARRIER: every surviving session is alive and connected ───────
actions-all-c50   Promise.allSettled over 50 action loops, all starting here
release-all-c50   destroy every session id the provider ever returned

The barrier is the point. Without it, session 1 could finish its work before session 50 was even created, and the run would report "50 concurrent" while never exceeding a handful. Step names carry the level because the platform groups step distributions by name alone, so a shared create-all would merge a 1-session create with a 50-session create into one distribution.

The action loop is the same ten actions as the throughput benchmark, so results are comparable to it: navigate, waitForSelector, screenshot, textContent, click, waitForSelector, screenshot, textContent, goBack, waitForSelector.

Levels run sequentially with a 60s cooldown between them, and every session id is destroyed in a finally. Both matter: leaked sessions hold provider quota until their idle timeout, and an earlier version of this benchmark measured "c1" while up to 91 sessions from other levels were still alive.

Run shape

One platform run per CI run. Each level is a phase with one iteration, and the task reads its level from ctx.phase:

phases: SELECTED_LEVELS.map((level) => ({ name: phaseNameForLevel(level), iterations: 1 })),
concurrency: 1,   // one level at a time, so levels cannot overlap

Nothing depends on task position, so --levels 50 runs c50 alone. Declaring phases also makes the runner reject --iterations, which previously looked like a count of repetitions but actually selected which levels ran.

Sampling

The comparable unit is one loop: ten actions on one session while the level's sessions run together. Sample count is therefore level x loops, and sessions repeat their loop per level to even that out:

c1   20 loops x  1 session  = 20 loop samples
c5    4 loops x  5 sessions = 20
c10   2 loops x 10 sessions = 20
c25   1 loop  x 25 sessions = 25
c50   1 loop  x 50 sessions = 50

Repeating inside a session costs no extra browsers, which is the expensive part. A per-level action budget (240s, env-overridable) bounds slow providers, checked on loop boundaries so every session stops together and the level holds its concurrency for as long as it runs.

Percentiles are reported only when the samples support them: stats carry their count, and a p95 under 20 samples is withheld rather than printed as the median a second time.

Scoring

composite = (0.30 x createMs.median
           + 0.25 x loopMs.median
           + 0.20 x loopMs.p95
           + 0.15 x screenshot.median
           + 0.10 x perSessionAPS.median) x successRate

Latency subscores are linear to a 30,000ms ceiling, throughput linear to 10 actions/sec. The success-rate multiplier is what stops a provider from ranking well by serving 10 of 50 sessions quickly.

Two guards on top:

  • Denominator is sessions attempted. A round where the provider refused every session records no sessions at all, so counting recorded sessions would make wholesale failures invisible.
  • Latency is withheld when the load was not sustained. A provider that held under 90% of the requested sessions measured a smaller experiment, so its timings are dashed out and footnoted rather than charted. Quota refusals (429, quota, limit exceeded) are detected and marked separately from capacity limits, because "your plan caps you at 10" is not the same finding as "it fell over at 25".

Outputs

  • results/browser-concurrent/c{1,5,10,25,50}/latest.json plus a dated file per level
  • browser-concurrent.svg, a leaderboard for the highest level with results
  • browser-concurrent-degradation.svg, per-session throughput across levels, which is the curve this benchmark exists to draw
  • a per-level table in the CI job summary

Also included

  • packages/benchsdk-runner/src/cli.ts prints the API response body on a BenchmarkApiError. A rejected run previously logged a bare 400 Bad Request with no indication of which field the platform refused; changeset included.
  • benchmarks/browser/concurrent-sampling.test.ts, 13 checks over the concurrency tracker, the action-budget boundary, loop chunking and the percentile gates. Run with pnpm test:browser-concurrent:sampling.

Validation

Typecheck and the sampling tests pass. Beyond that, every correctness fix in this branch came from replaying real CI artifacts rather than from reasoning, and several were found that way:

  • Two of fifty shared Wikipedia articles were unusable, timing out waitForSelector or click at 30s for all seven providers alike. Every action failure in run 31626532250 mapped to article index 6 or 44. That capped success at 96% for everyone and set a 63s task p95 against a 4s median. Fixed in the setup job (ci(browser-concurrency): require usable article pages in the setup job #309) and by censoring timed-out actions out of latency here.
  • A round ends when its slowest session ends, so one stalled session set the whole round's wall clock. Censored rounds are rebuilt from the slowest unaffected session rather than dropped, since each level runs once.
  • Kernel reports Peak 10/50 on replayed data, which is its account limit made visible by the concurrency counter instead of inferred from a regex over error strings.

Notes for review

  • Master's scheduled browser-concurrent-benchmarks.yml currently fails, because the workflow is there and the benchmark it invokes is not. Merging this fixes that.
  • The runner's phase tag lands in record.data and is not an aggregation key, so the platform dashboard cannot group by level today. The per-level breakdown comes from step names, which is why they are suffixed. A dimension axis on the platform would replace that convention.

Open in Devin Review

kisernl added 23 commits August 11, 2026 15:24
Add a new browser benchmark that measures per-action latency under
concurrent load. Unlike the existing lifecycle and throughput benchmarks
(which run one session at a time), this benchmark creates N browser
sessions in parallel, holds them all alive (barrier protocol), runs a
fixed 10-action Wikipedia loop on every session simultaneously, then
releases all.

The custom --concurrency-level flag (parsed from argv like storage's
--file-size) controls N. Results are organized by concurrency level in
subdirectories (c1, c5, c10, c25, c50), mirroring the storage benchmark's
per-file-size layout. The SVG generator produces a ranked leaderboard
table from the highest concurrency level and a degradation curve chart
showing median per-action latency vs. concurrent sessions for each
provider.

New files:
- concurrent-types.ts: types (SessionResult, RoundResult, etc.)
- concurrent-scoring.ts: composite scoring (0-100, higher = better)
- concurrent-benchmark.ts: summarization + JSON writer
- concurrent-legacy-results.ts: CLI records to legacy JSON bridge
- browser-concurrent.bench.ts: main benchmark with barrier protocol
- generate-concurrent-svg.ts: leaderboard + degradation curve SVG
- .github/workflows/browser-concurrent-benchmarks.yml: CI workflow

Modified:
- package.json: 12 new bench:browser-concurrent:* scripts
- benchmarks/src/merge-results.ts: browser-concurrent merge mode
workflow_dispatch requires the workflow file to exist on the selected
branch, not just on the default branch.
Every matrix job shared one run key, so each provider tried to register
as a participant five times (once per concurrency level) in the same
platform run, which the API rejects with 409 Conflict.

The benchmark slug is level-independent, so the level belongs in the
key, matching how storage-benchmarks.yml keys on file size.
The first full run surfaced defects that corrupted the data rather than
just its presentation.

Levels no longer run as parallel matrix jobs. All five started within
two seconds of each other against the same provider account, so a c1
measurement actually observed up to 91 concurrent sessions. Steel's c1
job failed rounds 0-6 and then ran 43 clean rounds, exactly tracking the
window where its sibling jobs were live. Levels now run sequentially
within a per-provider job, with a cooldown between them.

Sessions no longer leak. Release sat inside the try block after the
barrier, so any earlier throw skipped destroy entirely and the finally
block only closed local CDP sockets. Every id the provider returns is
now tracked and destroyed in finally, including creates that resolve
after their timeout, which previously held quota until idle expiry and
corrupted later rounds.

Success rate counted recorded sessions, but a round where every create
failed records none, so wholesale failures were invisible: steel
reported 100% at c5 while delivering 10 of 50 sessions. The denominator
is now sessions attempted.

Failure diagnostics survive. Provider rejection reasons were replaced
with a fixed string, and throwing discarded the sessions array outright
because the client keeps only an error message. Expected provider
failures are now returned as data; only unexpected exceptions throw.

Also: each session draws its own article, so a c50 round no longer puts
50 sessions on one CDN-warmed page while c1 pays cold-fetch cost;
per-session create and connect times are measured instead of recorded
as zero; and create phases censored by the timeout are excluded from
latency stats rather than reported as a 120s provisioning time.
A BenchmarkApiError's message carries only the status line, so a run
rejected by the platform logged a bare '400 Bad Request' with no
indication of which field was at fault. The body was already captured on
the error, just never surfaced.
The platform now rejects a caller-supplied run name; master already
dropped it from the createRun payload.
Master removed benchmarkKind from the SDK and every bench file (#302);
this benchmark was branched before that and was the last occurrence.
notte rejects with a plain object, so String(err) recorded 127 of its
c50 failures as "[object Object]" — leaving it the only provider whose
capacity degradation couldn't be attributed. Unwrap the common SDK
shapes (message/error/detail), then fall back to JSON.
The four phase steps record timing but not partial failure, which is
this benchmark's normal outcome: a round where 40 of 50 sessions are
refused still reports create-all as a success. Adopt the ai-gateway
pattern (ctx.log with meta) to report created/refused counts and a
breakdown of the provider's own rejection reasons, so a capacity
collapse is attributable from the worker log alone.

Reasons are counted rather than sampled — kernel refused sessions for
both a concurrency cap and a rate limit in the same round, and reporting
only the first would mis-attribute it.
…ever sustained

Latency percentiles are computed over surviving sessions, so a provider
that refuses most of the load measures itself on a nearly idle account.
In run 31544221216 kernel posted the best task latency of any provider
at c50 (2.4s vs browserbase's 11.1s) while delivering 21 of 150
sessions — its samples never experienced concurrency. Charting that
next to a provider that ran all 50 compares two different experiments.

A level is now charted only if the sampled rounds ran at >= 90% of the
requested concurrency, measured over rounds that produced sessions;
rounds refused outright contribute no samples, so averaging them in as
zeros would wrongly exclude a provider like tilion whose c50 timings
came from two full rounds and one total failure.

Also reports the session ceiling and, where the provider's own 429 names
a plan or org limit, marks the cap as billing rather than capacity:
kernel is capped at 10 concurrent sessions and steel at 5, so their
score decline across levels is arithmetic on cap/level.
…ummary too

The merge job's console table is the summary a human reads in the CI log,
so it needs the same treatment as the SVGs: latency dashed out for levels
the provider never sustained, the sustained count rather than the median,
and the provider's own limit message printed under the table.
The five concurrency levels were five platform runs, because the level was a
custom `--concurrency-level` flag and the run key carried it. A participant
holds one task count for the entire run, so the levels could not become
separate participants without also splitting each provider into five rows and
losing the provider ranking.

Spend a participant's tasks on the levels instead: task 0 runs every round at
1 session, task 4 every round at 50. One participant per provider, one run for
the sweep, and `task_index` becomes the concurrency axis, so the platform's
per-iteration view is the degradation curve.

Three details this depends on:

- Task latency is the median round, not the level's wall clock. A level's
  duration tracks its round count (c1 runs 10 rounds, c50 only 3), and on the
  last run's data wall clock would have put c5 above c50 for browseruse
  (68.2s vs 38.6s), inverting the curve.
- Step names carry the level (`create-all-c50`). The platform groups step
  distributions by name alone, so a shared `create-all` would merge a
  1-session create with a 50-session create.
- The platform payload holds per-round aggregates only. A c50 level covers
  1,500 actions; on real data the trim takes a level from 124.6KB to 0.5KB,
  and the local artifact keeps the full per-action detail.

The 60s inter-level cooldown moves from the workflow loop into the end of each
task, before the failure throw, since a level that died part way through is the
one most likely to have left sessions behind.
Ours was the last benchmark carrying the transitional `-local` suffix from
#235, when self-contained benchmarks ran alongside the legacy run.ts scripts.
Every other benchmark is `<domain>-<aspect>` with a noun aspect
(browser-lifecycle, browser-throughput, storage-lifecycle, ai-gateway-latency),
so this becomes browser-concurrency / "Browser Concurrency".

The runner upserts the benchmark from the file's own slug and name before
creating a run, so the new slug materializes on the first run. Historical
`browser-concurrent-local` runs stay under the old slug, which suits them:
they predate the success-rate, quota-gating and single-run fixes.

Local paths keep the browser-concurrent spelling (results directory, artifact
names, SVGs, merge mode). Slug and directory already differ elsewhere, as with
browser-lifecycle writing to results/browser.
…ations

`--iterations 3` meant "run the first 3 levels", which reads as a repeat count
for the task or its steps. It also hid a positional bug: the level came from the
task index, so `--iterations 1` ran c1, and there was no way to ask for c50 on
its own — any single-task run resolved to c1 regardless of intent.

Declare one phase per level instead, which is what the SDK offers for exactly
this ("phases let the task branch on identity instead of index arithmetic"):

  phases: SELECTED_LEVELS.map((level) => ({ name: phaseNameForLevel(level), iterations: 1 }))

The task now reads its level from `ctx.phase`, so nothing depends on task
position, and `--levels 50` runs c50 rather than c1. Declaring phases also makes
the runner reject `--iterations` with a warning of its own, so the confusing
spelling can no longer be used at all.

`--levels` takes a comma list validated against CONCURRENCY_LEVELS, normalized
ascending so the sweep always climbs, and unknown values fail with the choices
listed rather than being silently dropped. Records also gain a `phase` tag for
free, naming the level each one came from.

The rounds at each level stay fixed in ROUNDS_PER_LEVEL, since repeating a level
means more rounds, which is a property of the level and not of the invocation.
The level stopped being a task index when levels became phases; the comment next
to RUN_KEY still described the old scheme.
Run 31626532250 reported the same 96% success and the same ~63s task p95
for six independent providers. Both numbers came from the shared page set,
not from the providers: mapping every action failure to its article index
(roundIndex * level + sessionIndex) puts all of them on index 6 or 44,
modulo the 50-URL list, across all five levels and all seven providers.

Article 6 timed out waitForSelector and then textContent, leaving 8/10
actions. Article 44 timed out click, cascading into five skipped actions
and leaving 4/10.

Tighten the setup validation. It accepted any /wiki/ link anywhere in the
document, so a page whose only qualifying links sat in the navigation
chrome passed and then stalled in the browser. It now requires five
distinct non-namespaced article links inside #mw-content-text, which is
where the action loop looks and clicks. All three href forms have to be
matched: the content area is served with absolute and ./-relative links
while the chrome uses /wiki/, so matching the last form alone counts zero
links on even a 1,100-link article. Verified against real pages, where
File:Example.jpg, Special:BlankPage and Wikipedia:Sandbox now fail and
eleven real articles pass, the thinnest with 9 links.

Censor actions cut off at the timeout. A round ends when its slowest
session ends, so one session stalled on an unusable page set the whole
round's wall clock, and with 20% of the composite score riding on the
task p95 that page defect moved the ranking. Rounds are rebuilt from the
slowest unaffected session rather than dropped, because each level now
runs one round whose sessions each draw a different page: at c50 a single
bad article would otherwise erase the level's only latency observation.
On c5 the p95 falls from 61-68s to 3-11s and medians barely move; on c50
browserbase goes 63.5s to 4.9s with 6 of 150 sessions censored. Success
rates are unchanged, so the failures are still counted as failures.

Run one round per level. A level's sessions are its sample: c50 pools 50
sessions and 500 actions from a single round. The round-level wall clocks
get one observation per level as a consequence, so their median and p95
are now the same number.
Devin flagged the post-loop warning on #309: links is assigned only after the
resolution guard, and the guard broke out of the attempt loop, so a slot that
never reached the counting step was judged on the previous slot's number.
Reproduced: for the first slot links is unset, [ "" -lt 5 ] exits 2 with
"integer expression expected" and the warning is skipped; for later slots a
leftover count of 1106 suppressed the warning entirely, and a leftover count
of 2 emitted one naming the wrong page.

url and links now reset per slot, so the post-loop checks read the slot they
describe.

The warning was there to surface the fallback, and the fallback was the real
problem. It put the literal Special:Random URL into the shared list, so that
slot was resolved separately by each provider's browser: neither validated nor
shared, in a list whose whole purpose is that all seven providers navigate
identical pages. A transient redirect failure also broke out of the attempt
loop instead of retrying, abandoning the four remaining attempts.

Resolution failures now continue to the next attempt, and a slot that cannot
resolve an article in five tries fails the step rather than silently degrading
the page set. If Wikipedia is unreachable five times over, the run cannot
produce comparable numbers anyway.

Also guards the link count with || true, so a zero-match page cannot abort the
step if it ever runs under pipefail.

Verified by extracting the step from the workflow and running it against a
stubbed curl: unresolvable and stuck-on-Special:Random both exit 1 with an
::error::, a transient failure on attempts 1-2 recovers on attempt 3, an
exhausted thin page warns with the correct slot and count in either ordering,
and no "integer expression expected" remains. A live run of four slots
resolved four concrete articles.
Adopts three practices from the storage concurrency benchmark (#319), which
faced the same problem of making levels comparable to each other.

Report percentiles only when the samples back them. A p95 drawn from one
observation is the median again, so printing both claimed knowledge the run did
not have, and the score weighted that single number at 25% for the median plus
20% for the p95. Stats now carry their sample count, and consumers withhold a
p95 under 20 samples rather than repeating the median. #319 does this by
returning null for p99 below 1,000 samples.

Measure latency per loop, not per action phase. A level's action phase covers
as many loops as that level runs, so round wall clocks were never comparable
between levels; per loop, one session's ten actions while the level's sessions
run together, is. On replayed data the levels land within 2.6s to 3.2s of each
other, where the round wall clocks spread from 4s to 63s. The leaderboard and
the CI table now read Loop and Loop (p95), and no longer fall back to taskMs,
since presenting a whole action phase under a per-loop heading would relabel
the number rather than report it.

Take samples from the workload instead of from repetition. Sessions repeat their
loop per level, 20/4/2/1/1, which evens the budget out at 20 to 50 loop samples
where c1 previously had one session, ten actions, and a success rate that could
only be 0% or 100%. Repeating inside the session costs no extra browsers, which
is the expensive part, and mirrors how #319 pushes 1,200 operations through a
fixed pool. A level action budget bounds the cost for slow providers: notte
averaged 3.6s per action, which would be 12 minutes for c1 alone. The budget is
checked on loop boundaries so every session stops together and the level keeps
its concurrency while it runs.

Count concurrency instead of inferring it. sessionsAlive measures survival, so
sessions taking turns reported the same number as sessions running together.
A tracker now counts sessions in flight and records the peak per round, both for
live sessions and for sessions running actions, and the round logs a line when a
level never reached its own session count or exceeded it. On replayed kernel
data the new Peak column reads 10/50, which is its account limit made visible
rather than inferred from a regex over error strings. This is the metric that
would have caught c1 running against 91 live sessions directly.

A session now counts as successful when every action it attempted succeeded,
rather than against a fixed total, so a session stopped by the action budget is
not recorded as a provider failure.

Covered by benchmarks/browser/concurrent-sampling.test.ts, 13 checks over the
tracker, the budget boundary, loop chunking and the percentile gates, following
the tsx test-script convention in #319. Validated end to end by replaying real
rounds from run 31626532250 through the writer, merge and both SVGs: every level
reports 18 to 48 loop samples, c10 withholds its p95 at 18 samples while c50
reports it at 48, and the peaks read N/N.
b0dbe2d landed only concurrent-sampling.test.ts: the changes it describes were
never staged, so the tests it added were committed without the code they cover.
This is that code, unchanged from what was validated.

Per-loop stats with sample counts and the p95 gate, per-level loop counts with
an action budget checked on loop boundaries, the concurrency tracker with its
per-round watch, success measured against actions attempted, and the Loop,
Loop (p95) and Peak columns in the table and leaderboard.
@open-cla

open-cla Bot commented Aug 14, 2026

Copy link
Copy Markdown

Contributor License Agreement

All contributors are covered by a CLA.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment on lines +338 to +349
if (!clickSucceeded) {
for (const idx of [6, 7, 8, 9, 10]) {
results.push({
index: baseIdx + idx,
type: idx <= 8 ? (idx === 6 || idx === 10 ? 'waitForSelector' : idx === 7 ? 'screenshot' : 'textContent') : 'goBack',
durationMs: 0,
success: false,
error: 'skipped: click failed',
});
}
continue;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Skipped tenth step of a failed browser loop is recorded under the wrong action name

When a page has no usable article link, the tenth skipped step is filed under the wrong action name (type: expression at benchmarks/browser/browser-concurrent.bench.ts:342) because the idx <= 8 guard sends index 10 down the 'goBack' branch, so the per-action failure breakdown reports one extra navigation-back failure and one missing wait failure.
Impact: Failure counts per action kind in the saved results are attributed to the wrong operation, which is misleading when diagnosing which browser action broke.

Index-to-type mapping in the skip block

The real loop order is 6 waitForSelector, 7 screenshot, 8 textContent, 9 goBack, 10 waitForSelector (see benchmarks/browser/browser-concurrent.bench.ts:351-379). The skip block's ternary only checks idx === 6 || idx === 10 inside the idx <= 8 arm, so for idx === 10 the outer condition is false and it falls to 'goBack'. Same pattern was copied from benchmarks/browser/browser-throughput.bench.ts:168.

Suggested change
if (!clickSucceeded) {
for (const idx of [6, 7, 8, 9, 10]) {
results.push({
index: baseIdx + idx,
type: idx <= 8 ? (idx === 6 || idx === 10 ? 'waitForSelector' : idx === 7 ? 'screenshot' : 'textContent') : 'goBack',
durationMs: 0,
success: false,
error: 'skipped: click failed',
});
}
continue;
}
if (!clickSucceeded) {
const skippedTypes: Record<number, ActionResult['type']> = {
6: 'waitForSelector',
7: 'screenshot',
8: 'textContent',
9: 'goBack',
10: 'waitForSelector',
};
for (const idx of [6, 7, 8, 9, 10]) {
results.push({
index: baseIdx + idx,
type: skippedTypes[idx],
durationMs: 0,
success: false,
error: 'skipped: click failed',
});
}
continue;
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +284 to +286
const { loops, deadline, track } = options;
const release = track.enter();
try {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The reported "peak simultaneous sessions" figure can never differ from the number of connected sessions

The peak-simultaneity counter is incremented for every session before any waiting happens (track.enter() at benchmarks/browser/browser-concurrent.bench.ts:285), so the recorded peak always equals the number of connected sessions and the warning meant to catch a level that never reached its own session count can never fire.
Impact: A level that only appears to run its sessions together cannot be detected, and the "measured peak" shown in tables and result files is just the surviving-session count under a different name.

Why the counter is tautological

runActionLoop is invoked for every page inside Promise.allSettled(...) at benchmarks/browser/browser-concurrent.bench.ts:632-642; each invocation runs synchronously up to its first await, and track.enter() is the first statement (benchmarks/browser/browser-concurrent.bench.ts:285). All N entries therefore land before any session can release, so actionWatch.stop() (line 690) always returns pages.length, which is exactly sessionsAlive (line 610). The guard sessionsAlive > 0 && maxConcurrentActions < sessionsAlive at lines 746-752 is consequently dead code, and the Peak column in benchmarks/src/merge-results.ts:839-847 reports nothing new. Measuring genuine overlap would require entering the tracker only once a session's first action is actually in flight (and/or sampling the count over time).

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +679 to +687
const releaseStart = performance.now();
const releaseAll = () => Promise.allSettled([...createdSessionIds].map(destroySession));
if (harnessFailure) {
await releaseAll();
} else {
await step(`release-all-c${concurrencyLevel}`, releaseAll, { reportConcurrency: false });
}
releaseMs = performance.now() - releaseStart;
cleanupComplete = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 A browser session handed back during the cleanup step is never released

Sessions that the provider reports after the round has already collected its list for teardown ([...createdSessionIds] at benchmarks/browser/browser-concurrent.bench.ts:680) are neither destroyed there nor destroyed on arrival, because the flag that triggers on-arrival cleanup is only set after teardown finishes, so such a session stays open on the provider account.
Impact: A leaked session holds provider capacity until its idle timeout and inflates the live-session count for later concurrency levels, distorting their measurements.

Race window between the teardown snapshot and cleanupComplete

createTrackedSession (benchmarks/browser/browser-concurrent.bench.ts:466-488) registers late-arriving sessions and destroys them only if (cleanupComplete). cleanupComplete = true is set at line 687, after the release step awaits Promise.allSettled([...createdSessionIds].map(destroySession)) (line 680-685). A create whose underlying promise resolves after the spread but before line 687 — a window that can span the whole release phase, up to the 15s destroy timeout — is added to createdSessionIds too late to be destroyed and sees cleanupComplete === false, so no destroy is ever issued. Its liveSessionTracker.enter() release also never runs, permanently inflating the module-level live counter used for the cross-level overlap warning (lines 753-759). Setting a "cleanup started" flag before taking the snapshot, and re-draining any ids that arrived during the release, would close the window.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +286 to +290
try {
for (let loop = 0; loop < loops; loop++) {
// Checked between loops so every session stops on a loop boundary and the
// level holds its concurrency for as long as it runs.
if (!shouldStartLoop(loop, performance.now(), deadline)) break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 New benchmark loop body is left unindented, breaking the project's formatting style

The main action loop body is left at the wrong indentation level after being wrapped in a try block (for (let loop = ...) at benchmarks/browser/browser-concurrent.bench.ts:287), which does not match the Prettier-formatted style the repository requires.
Impact: The file reads inconsistently with the rest of the codebase and any formatting run will produce a large unrelated diff.

Formatting rule

CONTRIBUTING.md (Code Style) requires Prettier formatting. Lines 287-380 of benchmarks/browser/browser-concurrent.bench.ts sit at the same indentation as the enclosing try { on line 286, and the closing } on line 380-381 is likewise misaligned.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

github-actions Bot and others added 3 commits August 14, 2026 18:36
The benchmark scored each level independently, so there was no number for a
provider's concurrency behaviour as a whole and the leaderboard ranked on
whichever level happened to be highest. Adds a sweep score weighting c1 through
c50 at 5/10/15/30/40, so c25 and c50 carry 70% between them while the cheap
levels still count.

Weighting matters more than it sounds. Scored equally on run 31626532250, a
provider capped at 10 sessions placed fifth of seven on the strength of cells
that barely test anything, and a provider that degrades at c50 ranked above one
that holds. Weighted, both invert. The weights sit on the per-level composites
rather than on pooled raw metrics, so the sweep inherits the success-rate
multiplier and the withheld-latency guard instead of averaging a 1-session
median with a 50-session one.

A level with no result scores zero and keeps its weight. Renormalising over the
levels that did run would mean failing at c50 removes the hardest test from the
denominator, so a provider capped at 25 would be scored as though 25 were all it
was ever asked for. Dropping tilion's c25 from the real run costs it 13.0 points
and moves it below hyperbrowser, which is the intended incentive. The table
prints -- for a level that never ran and 0.0 for one that ran and scored nothing,
since both cost the weight but only one was attempted.

The per-level tables stay, printed before the sweep table. The curve is the
finding; one number cannot show where a provider stopped keeping up.

Also fixes a crash this surfaced: roundStats read .median off a stat that
artifacts written before loopMs existed do not carry, so merging a mix of old and
new results threw instead of serializing. Scoring already fell back to taskMs for
those; the writer now returns an empty triple with zero samples, which the
percentile gates then withhold downstream.

Six checks added over the weights, the missing-level rule, and that holding up
beats being quick then collapsing. Verified against all seven providers from run
31626532250, including levels deleted to exercise the zero-fill path.
A session that stops because its browser died is a failure, and none of the work
it managed first earns credit. That is already how computeConcurrentSuccessRate
reads it, so this locks the rule in and fixes the one place that disagreed.

The CI table's fallback success count still tested actionsCompleted === 10,
which predates sessions running more than one loop. It now applies the same rule
as the scorer, against the actions the session attempted, rather than counting a
session that completed all 200 of its actions as a failure. The fallback only
runs for artifacts with no recorded success rate, which is why this went unseen.

Two checks, built from notte's real c1 session in the 2026-08-14 run: 132 of 200
actions succeeded before the browser closed, and both its success rate and its
composite score are zero. The companion check keeps the distinction that makes
this rule fair, since a session stopped early by our own action budget attempted
fewer actions and succeeded at all of them, so it is not scored as a provider
failure.
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