Skip to content

build: source jemalloc through Cargo - #1376

Open
hnwyllmm wants to merge 1 commit into
oceanbase:masterfrom
wangyunlai-seekdb:codex/build-jemalloc-from-cargo-source
Open

build: source jemalloc through Cargo#1376
hnwyllmm wants to merge 1 commit into
oceanbase:masterfrom
wangyunlai-seekdb:codex/build-jemalloc-from-cargo-source

Conversation

@hnwyllmm

@hnwyllmm hnwyllmm commented Sep 11, 2026

Copy link
Copy Markdown
Member

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-sys Cargo 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:

  • GitHub Actions SeekDB CI run 34580759006: full release compile, all four mysqltest slices, and collect passed
  • GitHub Actions License header run 34580758878 passed
  • Local full RelWithDebInfo seekdb build and link
  • seekdb --version
  • C++ malloc/realloc/aligned allocation/mallctl smoke check

Copilot AI lite review requested due to automatic review settings September 11, 2026 07:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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 (plus SDKROOT), so it does not inherit CMAKE_OSX_ARCHITECTURES. On an Apple Silicon host configured with -DCMAKE_OSX_ARCHITECTURES=x86_64 or a universal list, the normal CMake objects use the requested architecture(s) but jemalloc is compiled for the host default, producing an incompatible libjemalloc_pic.a at link; CMAKE_CROSSCOMPILING does not cover this Darwin architecture selection. Pass the selected -arch flags (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)
Copilot AI review requested due to automatic review settings September 11, 2026 08:26
@wangyunlai-seekdb
wangyunlai-seekdb force-pushed the codex/build-jemalloc-from-cargo-source branch from 5c8cb0d to 3d282b1 Compare September 11, 2026 08:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@wangyunlai-seekdb
wangyunlai-seekdb force-pushed the codex/build-jemalloc-from-cargo-source branch from 3d282b1 to 088cd20 Compare September 11, 2026 08:29
Copilot AI review requested due to automatic review settings September 11, 2026 08:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@wangyunlai-seekdb
wangyunlai-seekdb force-pushed the codex/build-jemalloc-from-cargo-source branch from 088cd20 to 6121f95 Compare September 11, 2026 08:34
Copilot AI review requested due to automatic review settings September 11, 2026 08:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

Copilot AI review requested due to automatic review settings September 11, 2026 08:45
@wangyunlai-seekdb
wangyunlai-seekdb force-pushed the codex/build-jemalloc-from-cargo-source branch from 6121f95 to ade7ed2 Compare September 11, 2026 08:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants