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

Open
  • Is something wrong?
#2,277 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
22/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Active
Tech stack
c, github-actions, shell
Domain
build-system, ci-cd, testing-qa

Research direction

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.

Written by the indexing model from the issue text.

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)

Dominant language
C
Stars
3
Forks
7
Avg merge
6h 32m
Merged PRs (30d)
285

Getting set up

Open in Codespaces

Starts the project's dev container in your browser, under your own GitHub account.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from InauguralSystems/EigenScript

All issues in InauguralSystems/EigenScript

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.