morningman opened a new pull request, #68619:
URL: https://github.com/apache/doris/pull/68619
### What problem does this PR solve?
Issue Number: N/A
Related PR: #68617 (adds the two libraries to the third-party build; must be
merged first), #66511 (moved datasketches-cpp into the BE CMake tree)
**Do not merge before #68617 is merged and the prebuilt third-party archives
from apache/doris-thirdparty contain `libopenblas.a` and
`include/DataSketches`.** Until then BE fails to build: ninja stops right away
with `libopenblas.a` missing. This PR contains #68617's commit and will be
rebased once that merges.
Problem Summary:
**Context.** faiss (the ANN index) links OpenBLAS, and the
`datasketches_hll_union_agg` functions include the datasketches-cpp headers.
Both are git submodules under `contrib/`, fetched by `build.sh --be` and
`run-be-ut.sh` before every build and built inside the BE build directory:
`be/src/storage/index/ann/cmake-protect` `add_subdirectory()`s OpenBLAS, and
`be/CMakeLists.txt` `add_subdirectory()`s datasketches-cpp (header-only).
#68617 adds both to `thirdparty/`, whose prebuilt archives CI and developers
already download.
**1. The problem, and what it cost**
- On a fresh checkout `git submodule update` clones the whole
apache/doris-thirdparty repository (about 650 MB on GitHub) for
`contrib/openblas`, or downloads a 24.7 MB tarball when the clone fails.
- Every cold BE build compiles OpenBLAS: 6,725 build steps on macOS arm64,
where the rest of the BE is 740 unity steps, and 12,577 on Linux x86_64, where
`DYNAMIC_ARCH` builds kernels for every CPU generation.
- In ASAN builds OpenBLAS's configure step runs an ASAN-instrumented
`getarch` probe, which is where macOS ASAN builds hung before #68595.
**2. What this PR does, and why it helps**
- `be/cmake/thirdparty.cmake` imports `lib64/libopenblas.a` as the
`openblas` target, with `NOTADD` because only faiss needs it. `contrib/faiss`
already links a target of that name when one exists (it prints `Using OpenBLAS
target for linking`), so faiss itself is unchanged, and `cmake-protect` no
longer configures OpenBLAS.
- The datasketches-cpp headers come from `include/DataSketches` and are
included as `<DataSketches/hll.hpp>`, the way BE included them before #66511.
The `add_subdirectory()` build, its `DataSketches::HLL` link and the
`BUILD_TESTS` setting it needed go away.
- `build.sh` and `run-be-ut.sh` stop updating the two submodules, and
`.gitmodules` drops them.
- `build.sh` drops `--enable-dynamic-arch` / `--disable-dynamic-arch`: they
only set `DYNAMIC_ARCH` for the in-tree OpenBLAS, and the third-party build
decides that now. (The `ENABLE_DYNAMIC_ARCH` environment variable they
documented was overwritten before it was read.)
- `conf/ubsan_ignorelist.txt` drops `contrib/openblas`: the prebuilt library
is not instrumented, like every other third-party library.
- Comments and docs that named the two submodules are updated
(`be-ut-mac.yml`, `compile-bench`, `fe/.idea/vcs.xml`).
What it buys:
- No fetch for these two submodules, and no OpenBLAS compile in any BE build
directory.
- ASAN builds no longer run OpenBLAS's `getarch` probe.
- BE links the OpenBLAS that #68617 builds for distribution: runtime kernel
selection over a fixed CPU baseline and a thread cap that does not come from
the machine that built it.
**3. How the pieces fit**
```
build.sh --be / run-be-ut.sh
|- update_submodule: contrib/apache-orc, contrib/clucene, contrib/faiss
(openblas, datasketches-cpp removed)
'- cmake be/
|- include(cmake/thirdparty.cmake)
| '- add_thirdparty(openblas LIB64 NOTADD) -> IMPORTED target
`openblas` = installed/lib64/libopenblas.a
|- include_directories(SYSTEM installed/include) ->
<DataSketches/hll.hpp>
'- add_subdirectory(src/storage/index/ann)
|- cmake-protect: add_subdirectory(contrib/faiss)
| '- faiss/CMakeLists.txt: elseif(TARGET openblas) ->
target_link_libraries(faiss PRIVATE openblas)
'- ann_index PUBLIC faiss, OpenMP::OpenMP_CXX
link: doris_be <- ann_index <- faiss <- libopenblas.a (static, OpenMP) +
the OpenMP runtime
```
Not touched: faiss, apache-orc and clucene stay submodules; the BE source
only changes the include path of `hll.hpp` in two files.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- macOS arm64: with the two libraries from `build-thirdparty.sh`,
`build.sh --be` (Release, clang 20) builds and links `doris_be`. faiss is
configured with the imported target (`Using OpenBLAS target for linking`), and
`doris_be` resolves `sgemm_`, `ssyrk_` and `dsyev_` from the prebuilt
`libopenblas.a`. The `libfaiss.a` of that build, linked against the prebuilt
library, passes the smoke test described in #68617 (`sgemm`, `ssyrk` + `dsyev`,
`sgesvd`, IVF-PQ under OpenMP).
- `build-support/check-build-hygiene.sh` passes; clang-format 16
reports no changes to the two C++ files.
- CI cannot build this PR until the prebuilt archives carry the two
libraries.
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. `build.sh` no longer accepts `--enable-dynamic-arch` or
`--disable-dynamic-arch`. A BE build needs a third-party tree that contains
OpenBLAS and datasketches-cpp: refresh the prebuilt package, or run
`thirdparty/build-thirdparty.sh openblas datasketches`.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
