Skip to content

Add profiling_libunwind feature#159

Merged
BusyJay merged 2 commits into
tikv:mainfrom
ndr-ds:ndr-ds/add-profiling-libunwind-feature
May 24, 2026
Merged

Add profiling_libunwind feature#159
BusyJay merged 2 commits into
tikv:mainfrom
ndr-ds:ndr-ds/add-profiling-libunwind-feature

Conversation

@ndr-ds
Copy link
Copy Markdown

@ndr-ds ndr-ds commented Mar 25, 2026

Configure jemalloc with --enable-prof-libunwind to use libunwind for heap profiling
backtraces. This avoids a known livelock in gcc's _Unwind_Backtrace in multi-threaded
programs (jemalloc/jemalloc#2282).

The feature implies profiling since jemalloc requires --enable-prof alongside
--enable-prof-libunwind. On Linux, the build emits cargo:rustc-link-lib=unwind to
link against libunwind. On Apple platforms no extra link is needed since unwind symbols
are part of libSystem.

As invited by @BusyJay in
#146 (comment).

Closes #146

Summary by CodeRabbit

  • New Features
    • Added a new feature flag, profiling_libunwind, to enable libunwind-based backtracing for heap profiling; it also enables profiling automatically.
  • Bug Fixes
    • Avoids a known livelock issue with unwind-based backtraces in multi-threaded programs.
  • Documentation
    • Updated docs to describe the feature and platform requirements (libunwind packages on Linux; system unwind symbols on macOS/iOS).

Review Change Stack

Configure jemalloc with --enable-prof-libunwind to use libunwind for
heap profiling backtraces. This avoids a known livelock in gcc's
_Unwind_Backtrace in multi-threaded programs
(jemalloc/jemalloc#2282).

The feature implies profiling since jemalloc requires --enable-prof
alongside --enable-prof-libunwind. On Linux, the build emits
cargo:rustc-link-lib=unwind. On Apple platforms no extra link is
needed since unwind symbols are part of libSystem.

Closes tikv#146

Signed-off-by: Andre da Silva <andre.dasilva@linera.io>
@ti-chi-bot
Copy link
Copy Markdown

ti-chi-bot Bot commented Mar 25, 2026

Welcome @ndr-ds! It looks like this is your first PR to tikv/jemallocator 🎉

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6c23083e-9ce4-4a67-8905-3c866f951b3d

📥 Commits

Reviewing files that changed from the base of the PR and between 5cd1753 and cf42d64.

📒 Files selected for processing (2)
  • jemalloc-sys/Cargo.toml
  • jemalloc-sys/README.md
✅ Files skipped from review due to trivial changes (1)
  • jemalloc-sys/README.md

📝 Walkthrough

Walkthrough

Adds a new profiling_libunwind Cargo feature across crates to enable jemalloc heap-profiling with libunwind (configures with --enable-prof-libunwind), and documents platform linking and dependency notes.

Changes

Profiling Libunwind Feature

Layer / File(s) Summary
Cargo Feature Declarations
jemalloc-ctl/Cargo.toml, jemalloc-sys/Cargo.toml, jemallocator/Cargo.toml
Added profiling_libunwind feature flags across crates. In jemalloc-ctl and jemallocator, the feature enables tikv-jemalloc-sys/profiling_libunwind and profiling. In jemalloc-sys, the feature aliases to profiling.
Build Configuration
jemalloc-sys/build.rs
Build script now respects CARGO_FEATURE_PROFILING_LIBUNWIND, passing --enable-prof-libunwind to configure and linking unwind on non-Apple/non-Windows targets.
Documentation
jemalloc-sys/README.md
Documented profiling_libunwind: uses --enable-prof-libunwind to force libunwind-based backtraces for profiling, notes a livelock workaround for _Unwind_Backtrace, and lists platform dependency guidance (Linux: libunwind-dev/devel; macOS/iOS: system unwind symbols).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • BusyJay

Poem

🐰 I hopped through Cargo.toml with glee,
Enabled libunwind for your heap's decree,
Build flags set and docs now sing,
Profiling backtraces take to wing,
A rabbit cheers—no more deadlocking spree! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: adding a new Cargo feature called profiling_libunwind across multiple crates.
Linked Issues check ✅ Passed The PR adds the profiling_libunwind feature to address the musl + profiling segfault by using libunwind-based backtracing, directly implementing the solution approach described in issue #146.
Out of Scope Changes check ✅ Passed All changes are scoped to adding the profiling_libunwind feature: updated Cargo.toml files, README documentation, and build.rs logic, with no unrelated modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@BusyJay
Copy link
Copy Markdown
Member

BusyJay commented Mar 25, 2026

Cool, but this requires libunwind installed on system, right? I remember rust itself also use libunwind, is it possible to reuse the package?

@ndr-ds
Copy link
Copy Markdown
Author

ndr-ds commented Mar 25, 2026

Rust uses libgcc_s for unwinding on glibc Linux (source), not libunwind. And either way, jemalloc's configure needs libunwind.h at build time, so the dev package seems to be unavoidable for this opt-in feature, AFAICT.

@ndr-ds
Copy link
Copy Markdown
Author

ndr-ds commented Apr 6, 2026

@BusyJay Happy to adjust the approach if you have any concerns!

@BusyJay
Copy link
Copy Markdown
Member

BusyJay commented Apr 7, 2026

Can you verify whether it's linked dynamically or statically?

@BusyJay BusyJay merged commit 3bc06e9 into tikv:main May 24, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

x86_64-unknown-linux-musl + profiling = segmentation fault

2 participants