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

Aberta
  • Algo errado?
#2,277 1 comentário 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 1 dia

Ninguém assumiu esta issue ainda.

Avaliação

Dificuldade
4/5
Tempo estimado
3-5 dias
Facilidade para iniciantes
22/100
Tipo de issue
Refatoração
Clareza
Razoavelmente clara
Status de atividade
Ativa
Stack de tecnologia
c, github-actions, shell
Domínio
build-system, ci-cd, testing-qa

Direção de pesquisa

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.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

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)

Linguagem predominante
C
Estrelas
3
Forks
7
Merge médio
6h 32min
PRs com merge (30d)
285

Preparar o ambiente

Abrir no Codespaces

Inicia o contêiner de desenvolvimento do projeto no navegador, com a sua própria conta do GitHub.

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de InauguralSystems/EigenScript

Todas as issues de InauguralSystems/EigenScript

Issues semelhantes

Mais issues de C

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.