Skip to content

fix(test): stabilize Markdown parser perf budgets under parallel load - #3848

Merged
cixzhang merged 1 commit into
facebook:mainfrom
Han5991:fix/markdown-perf-test-flake
Jul 12, 2026
Merged

cixzhang merged 1 commit into
facebook:mainfrom
Han5991:fix/markdown-perf-test-flake

Conversation

@Han5991

@Han5991 Han5991 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Stabilizes the Markdown parser perf benchmarks, which assert fixed wall-clock budgets from a single measurement and therefore flap under parallel-suite CPU contention.

Evidence

While verifying #3847, one full-suite run failed exactly one test out of 6,177:

FAIL |ui| packages/core/src/Markdown/parser.perf.test.ts
     > full re-parse Small (10 paragraphs) under 20ms
AssertionError: expected 30.87816699999985 to be less than 20
  • Two other full-suite runs on the same toolchain passed 6,177/6,177.
  • The file passes 3/3 in isolation.
  • The benchmarks run alongside 331 other test files in parallel forks; scheduler contention can roughly double a millisecond-scale sample, which a 20ms budget cannot absorb.

Fix

Take the fastest of three runs (measureBest) instead of a single sample. Contention can only inflate a measurement, so the minimum approximates the un-contended cost — transient load can no longer fail the assertion, while a real parser regression raises the minimum too and is still caught. Budgets are unchanged.

Applied to:

  • the five full re-parse benchmarks (tight budgets: 20–1000ms)
  • the incremental-vs-full speedup comparison (a ratio of two samples — noise on either side could flip it)

Deliberately not applied to the streaming budget tests: their budgets carry 10x+ headroom (5s/30s against sub-second actuals), and repeating multi-second simulations would only slow the file down.

Verification

The perf benchmarks asserted fixed wall-clock budgets against a single
measurement. With the whole suite running in parallel forks, scheduler
contention can inflate a millisecond-scale sample well past a tight
budget — observed once during an unrelated PR's verification runs:
30.9ms against the Small full re-parse budget of 20ms (1 failure in
6,177 tests, non-reproducible in isolation or on re-run).

Measure the fastest of three runs instead. The minimum approximates the
un-contended cost, so transient load cannot fail the assertion, while a
real parser regression still raises the minimum and gets caught. The
budgets themselves stay unchanged.

Applied to the five full re-parse benchmarks and to the incremental vs
full speedup comparison (a ratio of two samples, where noise on either
side could flip the outcome). The streaming budget tests keep a single
run: their budgets carry 10x+ headroom, and best-of-N there would just
slow the file down.
@vercel

vercel Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 11, 2026 10:15pm

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 11, 2026
@github-actions github-actions Bot added the community Authored by a community contributor (not on the eng/design team) label Jul 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR Analysis Report

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.6KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | View full report

@github-actions github-actions Bot added the needs:code-review High-risk change (new package/component/API) — needs human code review before merge label Jul 12, 2026

@cixzhang cixzhang 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.

Thanks for optimizing all of these <3

@cixzhang cixzhang removed the needs:code-review High-risk change (new package/component/API) — needs human code review before merge label Jul 12, 2026
@cixzhang
cixzhang merged commit 7a376b6 into facebook:main Jul 12, 2026
20 of 21 checks passed
cixzhang added a commit that referenced this pull request Jul 12, 2026
…runs (#3860)

Two reliability fixes for the review gate:

1. Stale check-run neutralization. PRs gated before the switch to a commit-status gate still carry an action_required review-required CHECK RUN. Since the gate is now a STATUS, nothing updated those check-runs, and a leftover action_required drags the status rollup to FAILURE even when the status is success — so approved PRs stayed blocked (e.g. #3847). Both the flag job and review-clear now update any stale review-required check-run to neutral once the status is set.

2. Direct fork-safe clearing. review-clear previously re-dispatched the flag job (approve → Review signal completes → workflow_run → dispatch → flag), and any dropped hop left approved fork PRs stuck at pending (e.g. #3848, #3803). It now clears DIRECTLY in the workflow_run context (write-capable, fork-safe): resolve the PR, confirm an entitled CODEOWNER approved, then drop the label, set the status to success, and neutralize stale check-runs — one hop, no re-dispatch.
@Han5991
Han5991 deleted the fix/markdown-perf-test-flake branch July 12, 2026 05:32
Han5991 added a commit to Han5991/astryx that referenced this pull request Jul 20, 2026
…he enforced budgets

The measureBest comment still claimed the streaming budgets carry 10x+ headroom that contention cannot eat — disproved by the 5s-default-timeout incident this branch fixes, and stale now that the XL budget is 15s (~4x observed CI cost) with explicit per-test timeouts.
Han5991 added a commit to Han5991/astryx that referenced this pull request Jul 20, 2026
The Table render benchmarks asserted tight budgets (100-200ms, 750ms CI) against a single wall-clock sample — the exact pattern facebook#3848 diagnosed as flaky under parallel forks and fixed in the Markdown perf suite, but Table.perf was left out. Apply the same best-of-3 technique: the fastest of three runs approximates the un-contended cost while a real regression still raises the minimum. Mount/unmount happen outside the timed section so only the measured work is sampled.

This branch was successfully deployed

1 active deployment
Preview — ddb8cd39 Deployed Jul 11, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. community Authored by a community contributor (not on the eng/design team)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants