ci: switch fast-path jobs to system OpenBLAS and split-image tags#2120
Open
wxyucs wants to merge 1 commit into
Open
ci: switch fast-path jobs to system OpenBLAS and split-image tags#2120wxyucs wants to merge 1 commit into
wxyucs wants to merge 1 commit into
Conversation
wxyucs
added
kind/improvement
Contributor
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Require kind labelWonderful, this rule succeeded.
🟢 Require version labelWonderful, this rule succeeded.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the Docker setup for VSAG on x86_64 by introducing a common base image (Dockerfile.x86.base) and layering specific BLAS backend variants (Dockerfile.x86.mkl and Dockerfile.x86.openblas) on top of it. Additionally, the CircleCI configuration is updated to install liblapacke-dev and opt into using the system OpenBLAS during parallel tests. The review feedback suggests optimizing the Dockerfiles by combining multiple apt commands into single RUN instructions, using apt-get with --no-install-recommends, and cleaning up package lists to minimize image sizes.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR repoints x86 CI workflows to split MKL/OpenBLAS CI images and uses the existing EXTRA_DEFINED Makefile hook to prefer system OpenBLAS on fast-path OpenBLAS jobs, while preserving daily coverage for the bundled OpenBLAS source build.
Changes:
- Adds/splits x86 Docker images into base, MKL, and OpenBLAS variants.
- Updates CI workflows to use
ci-x86-mklorci-x86-openblas. - Adds
-DUSE_SYSTEM_OPENBLAS=ONto most OpenBLAS-backed build steps.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
docker/Dockerfile.x86.base |
Adds shared x86 CI base image. |
docker/Dockerfile.x86.mkl |
Rebuilds MKL image on top of the shared base. |
docker/Dockerfile.x86.openblas |
Adds OpenBLAS image with system OpenBLAS/LAPACKE packages. |
.github/workflows/update-ci-image.yml |
Builds base and variant CI Docker images. |
.github/workflows/pr-ci.yml |
Routes PR CI to OpenBLAS image and system OpenBLAS builds. |
.github/workflows/daily_test.yml |
Routes daily OpenBLAS/MKL jobs to split images and keeps source OpenBLAS guard. |
.github/workflows/coverage.yml |
Uses OpenBLAS image and system OpenBLAS for coverage builds. |
.github/workflows/lint.yml |
Uses OpenBLAS image and system OpenBLAS for lint build setup. |
.github/workflows/check_compatibility.yml |
Uses OpenBLAS image and system OpenBLAS for compatibility tools. |
.github/workflows/performance.yml |
Uses OpenBLAS image and system OpenBLAS for performance builds. |
.github/workflows/generate_old_version_index.yml |
Switches old-version index workflow to OpenBLAS image. |
.github/workflows/asan_build_and_test.yml |
Routes ASan workflow to MKL image. |
.github/workflows/asan_with_simd_option.yml |
Routes SIMD ASan workflow to MKL image. |
.github/workflows/tsan_build_and_test.yml |
Routes TSan workflow to MKL image. |
.circleci/continue_config.yml |
Installs LAPACKE and enables system OpenBLAS in CircleCI builds. |
wxyucs
added a commit
to wxyucs/vsag
that referenced
this pull request
May 28, 2026
…+variants on PRs Per Copilot review on antgroup#2119/antgroup#2120: with the previous fan-out layout the variant builds ran in a separate job from the base build, so on pull_request runs (where `push: false`) the variants would try to pull `vsaglib/vsag:ci-x86-base` from Docker Hub — either failing before the first main publish or, worse, silently validating against the already-published base instead of the PR's base changes. Collapse the workflow into a single job that builds base first with buildx's plain `docker` driver, which loads images into the host daemon automatically, so the subsequent variant builds resolve `FROM vsaglib/vsag:ci-x86-base` against the just-built base regardless of whether the run pushes to the registry. Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com> Assisted-by: OpenCode:claude-opus-4.7
wxyucs
force-pushed
the
improve/ci-switch-to-system-openblas
branch
from
ffa6bdc to
7539eae
Compare
wxyucs
force-pushed
the
improve/ci-switch-to-system-openblas
branch
from
7539eae to
369bca0
Compare
wxyucs
force-pushed
the
improve/ci-switch-to-system-openblas
branch
from
369bca0 to
3b8bbad
Compare
Copilot stopped reviewing on behalf of
wxyucs due to an error
May 29, 2026 03:34
Repoint every x86 CI job at the BLAS-variant image introduced in the previous PR and pass `-DUSE_SYSTEM_OPENBLAS=ON` (via the existing `EXTRA_DEFINED` Makefile knob) to the OpenBLAS-backed jobs so they reuse the pre-installed `libopenblas-dev` instead of rebuilding OpenBLAS from source on every run. Routing: * `pr-ci`, `coverage`, `lint`, `check_compatibility`, `performance`, `generate_old_version_index` → `ci-x86-openblas`. All except `generate_old_version_index` also set `USE_SYSTEM_OPENBLAS=ON`; the latter builds historical release tags whose CMake does not understand the flag, so the source-build path is kept there. * `asan_build_and_test`, `tsan_build_and_test`, `asan_with_simd_option`, daily SIMD / TSAN → `ci-x86-mkl` (these jobs already build with `VSAG_ENABLE_INTEL_MKL=ON`). * Daily x86 ASan → `ci-x86-openblas` but deliberately leaves `USE_SYSTEM_OPENBLAS` unset, so `ExternalProject_Add(openblas)` keeps getting exercised end-to-end as the dedicated guard for the from-source build path. CircleCI's `prepare_env_and_create_swap_file` step also installs `liblapacke-dev` so CMake's `find_path(LAPACKE_INCLUDE NAMES lapacke.h)` probe succeeds against the system OpenBLAS. Forwarding `USE_SYSTEM_OPENBLAS` via `EXTRA_DEFINED` keeps `Makefile` untouched and avoids colliding with the in-flight `VSAG_USE_SYSTEM_OPENBLAS` CMake option in antgroup#2117. Release artifacts (`scripts/release/dist.sh`, `docker/Dockerfile.dist_*x86`) and `make pyvsag` are unchanged and still build OpenBLAS from source. Refs: antgroup#2118 Signed-off-by: Xiangyu Wang <wxy407827@antgroup.com> Assisted-by: OpenCode:claude-opus-4.7
wxyucs
force-pushed
the
improve/ci-switch-to-system-openblas
branch
from
3b8bbad to
fa58f3d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ci-x86-mkl/ci-x86-openblasimage tags introduced in chore(docker): split ci-x86 image into base / mkl / openblas variants #2119.-DUSE_SYSTEM_OPENBLAS=ON(forwarded via the existingEXTRA_DEFINEDMakefile knob, noMakefilechange required) on the OpenBLAS-backed jobs so they reuse the pre-installed system OpenBLAS instead of rebuilding it from source on every run.ExternalProject_Add(openblas)inextern/openblas/openblas.cmake).make pyvsagare unchanged and still build OpenBLAS from source.Why
See #2118 for the full rationale. Short version: every PR CI / coverage / lint / compatibility / performance / CircleCI run currently spends 2–3 min building OpenBLAS even though
libopenblas-devis already in the image. Switching to the system package eliminates that rebuild on the fast path while the daily ASan job keeps the source-build path under daily coverage.Image routing after this PR
USE_SYSTEM_OPENBLASpr-ci(asan build, unittests, functests, examples, compatibility)ci-x86-openblasON(build job only)coverage,lint,check_compatibility,performanceci-x86-openblasON(build steps)generate_old_version_indexci-x86-openblasasan_build_and_test,tsan_build_and_test,asan_with_simd_option, daily SIMD / TSANci-x86-mklci-x86-openblasON(build step)Prerequisite
#2119 has merged and
update-ci-imagehas published the newci-x86-base/ci-x86-mkl/ci-x86-openblastags to Docker Hub, so the workflows in this PR can now pull their new container images.Notes on related work
USE_SYSTEM_OPENBLAS=ONis forwarded viaEXTRA_DEFINEDrather than a newmakeflag, deliberately avoiding any change toMakefile. This keeps this PR independent of #2117, which introduces a differentVSAG_USE_SYSTEM_OPENBLASCMake option with stricter semantics. The two changes can land in either order.Test plan
mainto confirm the OpenBLAS build step has dropped out of the timeline.Labels
kind/improvement,version/1.0Refs: #2118