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

Đang mở
  • Có gì không ổn?
#2,277 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức phù hợp với người mới
22/100
Loại issue
Tái cấu trúc
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
c, github-actions, shell
Lĩnh vực
build-system, ci-cd, testing-qa

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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)

Ngôn ngữ chính
C
Star
3
Fork
7
Merge trung bình
6 giờ 32 phút
Pull request đã merge (30 ngày)
285

Chuẩn bị môi trường

Mở trong Codespaces

Khởi chạy dev container của dự án ngay trên trình duyệt, bằng tài khoản GitHub của bạn.

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của InauguralSystems/EigenScript

Tất cả issue của InauguralSystems/EigenScript

Issue tương tự

Thêm issue về C

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.