ci: the LSP sanitizer step rebuilds a whole ASan variant on the shard that sets merge-queue wall time (~8 min)

Ouverte
  • Quelque chose ne va pas ?
#2,277 1 commentaire 0 réactions 0 personnes assignées Voir sur GitHub

Les mainteneurs répondent en général sous 1 jour

Personne n'a encore pris cette issue.

Évaluation

Difficulté
4/5
Temps estimé
3-5 jours
Accessibilité débutants
22/100
Type d'issue
Refactorisation
Clarté
Plutôt claire
Activité
Active
Stack technique
c, github-actions, shell
Domaine
build-system, ci-cd, testing-qa

Piste de recherche

Start with the comment at .github/workflows/ci.yml lines 1626-1632 and the LSP sanitizer step near line 1581, then compare them with tests/test_lsp_asan.sh, tests/test_lsp.sh and the lsp-asan target in the Makefile around line 550. Confirm which object directory each one builds from. Done means the comment matches the real build, and the LSP step no longer adds a serial sanitizer rebuild to the critical shard. The job-placement and -j choices need maintainer direction and timing measurements first.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Description

area:ci enhancement found-by:duet-audit status:claimed

Found by a read-only CI performance audit (area 9). Nothing was rerun or triggered; every number below comes from existing merge_group runs of ci.yml on main b367cd72.

Summary

The merge queue's wall time is set by ASan shard 2/3 in 5 of 5 sampled runs, and about 8 minutes of that shard is one step: "Behavior-test the LSP under sanitizers". Its comment says the step is a ~1.5 s no-op because section [88] already built eigenlsp. That premise is false today: [88] builds the release src/eigenlsp, while the step builds the ASan LSP from the build/asan objects, a variant this job never builds (it builds asan-server). So the step compiles a whole second sanitizer variant from scratch, serially, on the critical path.

Evidence

Five merge_group runs: 41213192207, 41212377444, 41208476364, 41194089707, 41164297994.

Job (median over the 5) Minutes
asan + ubsan / shard 2/3 26.2
extensions / server (next longest) 21.1
asan + ubsan / shard 1/3 17.9
asan + ubsan / shard 3/3 17.5

Shard 2's steps (median over the 5): build 8.0 min, suite 8.6 min, LSP step 8.0 min (range 263 to 560 s). On shards 1 and 3 the same step is a 0.1 min skip. Shard 2's build and suite are in line with the other shards, so the LSP step is the whole difference.

Job 124572374920 (run 41213192207): the step logs "Building eigenlsp with -fsanitize=address,undefined ..." at 15:21:56, and the next output is at 15:27:48, so about 352 s of compiling before 196 LSP tests pass in 28 s.

Why it rebuilds:

  • .github/workflows/ci.yml:1581 builds make asan-server (objects in build/asan-server/).
  • tests/test_lsp_asan.sh:19 runs make lsp-asan lsp-arming-test LSP_ARMING_VARIANT=asan.
  • Makefile:550 links lsp-asan from $(OBJ_asan), the build/asan/ objects (the #1365 fix moved it there). None of those exist in this job.
  • Section [88] (tests/test_lsp.sh:23, via tests/aux_binary.sh) builds the release src/eigenlsp, not an ASan one.
  • So the comment at ci.yml:1626-1632 ("[88] builds eigenlsp under ASan ... this step is a no-op rebuild") no longer describes the tree.

Noise: across 27 green merge_group runs the wall time is median 24.0 min, IQR 4.3 min, range 18.4 to 29.4. The 8-minute step is present in all 5 sampled runs, so it is not runner variance. The wall-time gain from removing it is bounded by extensions / server (median 21.1 min), so the expected gain is about 3 to 5 min per queue run (inferred). That is the same size as the IQR, so it has to be confirmed with n≥5 runs, not one.

Expected vs actual

  • Expected (per the step's own comment): the LSP sanitizer step costs seconds on the shard that owns it.
  • Actual: it costs 263 to 560 s, on the shard that sets the queue's wall time.

A second, smaller cost on the same path (measure before changing)

Each of the 3 shards builds make asan-server serially (no -j in ci.yml:1581 or the Makefile): 347 to 847 s per shard across the sample, on 4-vCPU runners. Building with -j"$(nproc)", or once and sharing it, would likely shorten every shard (inferred; not measured, since the audit ran nothing).

Suggested direction (DevBox's call)

Smallest change: move the LSP sanitizer test into its own job, parallel to the shards, so its build is off the critical path; or run its make with -j"$(nproc)". Making lsp-asan reuse asan-server objects would change which flags the LSP is tested under, so that is a design decision, not a speed fix. Either way, fix the stale comment.

Done when

  • The comment at ci.yml:1626-1632 matches what [88] and test_lsp_asan.sh actually build.
  • In 5 consecutive green merge_group runs after the change, no ASan shard spends more than 60 s in the LSP step, or the step lives in its own job that is not the run's longest.
  • The median merge_group wall time over those 5 runs is reported next to this issue's 24.0 min baseline (IQR 4.3).
  • The shard receipt still reports exactly one lsp_asan=yes claimant (or the new job's receipt replaces it), so the LSP still runs under a sanitizer somewhere.
  • The -j question for make asan-server is answered by a measurement (n≥5 build timings each way in the dev image), recorded here, and adopted only if the median drop exceeds the spread.

Not filed, seen once: PR run 41165472363's freestanding job (124432044071) hung in "Symbol gate" for 35 min until cancelled; the rerun passed in 7.65 min. The log has expired, so there is nothing to minimise yet.

— Duet (orchestrator)

Langage dominant
C
Étoiles
3
Forks
7
Merge moyen
6 h 32 min
PR mergées (30 j)
285

Préparer son environnement

Ouvrir dans Codespaces

Lance le conteneur de développement du projet dans votre navigateur, avec votre propre compte GitHub.

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Autres issues de InauguralSystems/EigenScript

Toutes les issues de InauguralSystems/EigenScript

Issues similaires

Plus d'issues C

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.