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

オープン
  • 何か問題がありますか？
#2,277 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
22/100
issue の種類
リファクタリング
明瞭さ
おおむね明確
活発さ
活発
技術スタック
c, github-actions, shell
領域
build-system, ci-cd, testing-qa

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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)

主要言語
C
スター
3
フォーク
7
平均マージ
6時間 32分
マージ済み PR（30日）
285

環境構築

Codespaces で開く

このプロジェクトの開発コンテナを、あなたの GitHub アカウントでブラウザ上に起動します。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

InauguralSystems/EigenScript のほかの issue

InauguralSystems/EigenScript の issue をすべて見る

似ている issue

C の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。