build: source jemalloc through Cargo - #1376
Open
hnwyllmm wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved toolchain, architecture, header, and Bazel integration issues remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR replaces the platform-specific jemalloc RPM with a locked Cargo-sourced jemalloc 5.3.1 build integrated through CMake.
Changes:
- Adds locked Cargo dependency metadata.
- Builds and links a PIC jemalloc archive out of tree.
- Removes the AL8 x86_64 jemalloc RPM dependency.
Review findings:
- Critical: Clean CI may lack the pinned Rust toolchain.
- Moderate: Darwin architecture flags are not propagated.
- Moderate: Generated jemalloc headers are missing from sanity builds.
- Moderate: Bazel jemalloc dependencies are not migrated.
File summaries
| File | Description |
|---|---|
src/oblib/lib/CMakeLists.txt |
Links allocator targets to jemalloc. |
deps/init/oceanbase.al8.x86_64.deps |
Removes the jemalloc RPM. |
CMakeLists.txt |
Includes the jemalloc integration. |
cmake/native-deps/src/lib.rs |
Provides the Cargo manifest anchor. |
cmake/native-deps/README.md |
Documents native dependency handling. |
cmake/native-deps/Cargo.toml |
Declares the pinned dependency. |
cmake/native-deps/Cargo.lock |
Locks the package version and checksum. |
cmake/Jemalloc.cmake |
Resolves, builds, and exposes jemalloc. |
Review details
Suppressed comments (2)
cmake/Jemalloc.cmake:42
- The external macOS build receives only
-O2 -fPIC(plusSDKROOT), so it does not inheritCMAKE_OSX_ARCHITECTURES. On an Apple Silicon host configured with-DCMAKE_OSX_ARCHITECTURES=x86_64or a universal list, the normal CMake objects use the requested architecture(s) but jemalloc is compiled for the host default, producing an incompatiblelibjemalloc_pic.aat link;CMAKE_CROSSCOMPILINGdoes not cover this Darwin architecture selection. Pass the selected-archflags (and matching deployment target) to this command, or explicitly reject such configurations.
set(_jemalloc_env "CC=${CMAKE_C_COMPILER}" "AR=${CMAKE_AR}" "CFLAGS=${_jemalloc_cflags}")
if(APPLE AND CMAKE_OSX_SYSROOT)
list(APPEND _jemalloc_env "SDKROOT=${CMAKE_OSX_SYSROOT}")
cmake/Jemalloc.cmake:62
- This replacement is only registered with CMake, but the removed AL8 x86 package is also consumed by the Bazel dependency setup. Bazel still directly requires @seekdb_3rd_headers//:lib/libjemalloc_pic.a for sanity builds and exposes jemalloc_headers; after this profile entry is deleted, an AL8 x86 Bazel sanity build cannot resolve the archive or headers. Keep the package until Bazel is migrated, or add the Cargo-built archive and configured headers to the Bazel repository/link graph.
add_library(seekdb_jemalloc STATIC IMPORTED GLOBAL)
set_target_properties(seekdb_jemalloc PROPERTIES IMPORTED_LOCATION "${JEMALLOC_STATIC_LIBRARY}")
add_dependencies(seekdb_jemalloc seekdb_jemalloc_build)
- Files reviewed: 7/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "${_native_manifest}" "${CMAKE_CURRENT_LIST_DIR}/native-deps/Cargo.lock") | ||
| execute_process( | ||
| COMMAND "${CARGO}" metadata --locked --format-version 1 --manifest-path "${_native_manifest}" | ||
| WORKING_DIRECTORY "${CMAKE_BINARY_DIR}" |
Comment on lines
+61
to
+62
| set_target_properties(seekdb_jemalloc PROPERTIES IMPORTED_LOCATION "${JEMALLOC_STATIC_LIBRARY}") | ||
| add_dependencies(seekdb_jemalloc seekdb_jemalloc_build) |
wangyunlai-seekdb
force-pushed
the
codex/build-jemalloc-from-cargo-source
branch
from
September 11, 2026 08:26
5c8cb0d to
3d282b1
Compare
wangyunlai-seekdb
force-pushed
the
codex/build-jemalloc-from-cargo-source
branch
from
September 11, 2026 08:29
3d282b1 to
088cd20
Compare
wangyunlai-seekdb
force-pushed
the
codex/build-jemalloc-from-cargo-source
branch
from
September 11, 2026 08:34
088cd20 to
6121f95
Compare
wangyunlai-seekdb
force-pushed
the
codex/build-jemalloc-from-cargo-source
branch
from
September 11, 2026 08:45
6121f95 to
ade7ed2
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.
The CMake build currently requires platform-specific prebuilt jemalloc packages. This change resolves the official jemalloc 5.3.1 source through the locked
seekdb-jemalloc-sysCargo registry package, builds it out of tree, and links the generated PIC archive through an imported CMake target.It preserves the existing
je_*ABI and stats support, and removes jemalloc packages from all Linux dependency init manifests.Validation:
seekdbbuild and linkseekdb --version