From 77f850969433b14769ade4281899373f3ebabf86 Mon Sep 17 00:00:00 2001 From: Jonathan Stites Date: Wed, 6 May 2020 12:55:35 +0000 Subject: [PATCH] puts jemalloc allocator behind a cargo feature flag Retrieved from: https://github.com/BurntSushi/ripgrep/pull/1569 Moves jemalloc behind a feature for musl builds, where it is not supported by the upstream project, so ripgrep will fail to build. Signed-off-by: Sam Voss [Antoine: update for 14.1.0] Signed-off-by: Antoine Coutant --- .github/workflows/ci.yml | 6 ++++++ .github/workflows/release.yml | 8 +++++++- Cargo.toml | 8 +++++++- README.md | 9 +++++++++ crates/core/main.rs | 8 ++++++-- 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d21b85a..0c9ecb9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -172,6 +172,12 @@ jobs: if: matrix.target != '' run: ${{ env.CARGO }} test --verbose --workspace ${{ env.TARGET_FLAGS }} + - name: Run tests with jemalloc (Musl) + # We only use the jemalloc allocator when building with musl. + # The system allocator is good enough for other platforms. + if: matrix.os == 'nightly-musl' + run: ${{ env.CARGO }} test --verbose --all --features jemalloc ${{ env.TARGET_FLAGS }} + - name: Test zsh shell completions (Unix, sans cross) # We could test this when using Cross, but we'd have to execute the # 'rg' binary (done in test-complete) with qemu, which is a pain and diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f6ea3d9..ac18129 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -171,7 +171,13 @@ jobs: echo "target flag is: ${{ env.TARGET_FLAGS }}" echo "target dir is: ${{ env.TARGET_DIR }}" - - name: Build release binary + - name: Build release binary (linux) + if: matrix.build == 'linux' + # Use jemalloc allocator for much better performance over the musl default allocator + run: ${{ env.CARGO }} build --verbose --release --features "pcre2 jemalloc" ${{ env.TARGET_FLAGS }} + + - name: Build release binary (non-linux) + if: matrix.build != 'linux' shell: bash run: | ${{ env.CARGO }} build --verbose --release --features pcre2 ${{ env.TARGET_FLAGS }} diff --git a/Cargo.toml b/Cargo.toml index da350bc..1a0a48f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -59,8 +59,9 @@ serde_json = "1.0.23" termcolor = "1.1.0" textwrap = { version = "0.16.0", default-features = false } -[target.'cfg(all(target_env = "musl", target_pointer_width = "64"))'.dependencies.jemallocator] +[dependencies.jemallocator] version = "0.5.0" +optional = true [dev-dependencies] serde = "1.0.77" @@ -70,6 +71,11 @@ walkdir = "2" [features] simd-accel = ["grep/simd-accel"] pcre2 = ["grep/pcre2"] +# The jemalloc allocator is used for improved +# performance on x86 musl builds. +# Cargo does not yet support platform-specific features +# https://github.com/rust-lang/cargo/issues/1197 +jemalloc = ["jemallocator"] [profile.release] debug = 1 diff --git a/README.md b/README.md index 0821fab..fdb9fb5 100644 --- a/README.md +++ b/README.md @@ -478,6 +478,15 @@ build a static executable with MUSL and with PCRE2, then you will need to have `musl-gcc` installed, which might be in a separate package from the actual MUSL library, depending on your Linux distribution. +When building with the MUSL target on Linux, it is recommended to use the +jemalloc allocator for performance: + +``` +$ rustup target add x86_64-unknown-linux-musl +$ cargo build --release --target x86_64-unknown-linux-musl --features jemalloc +``` + + ### Running tests diff --git a/crates/core/main.rs b/crates/core/main.rs index 64f35ce..9aa6663 100644 --- a/crates/core/main.rs +++ b/crates/core/main.rs @@ -27,7 +27,7 @@ mod search; // have the fastest version of everything. Its goal is to be small and amenable // to static compilation.) Even though ripgrep isn't particularly allocation // heavy, musl's allocator appears to slow down ripgrep quite a bit. Therefore, -// when building with musl, we use jemalloc. +// we expose a feature for using jemalloc when building with musl. // // We don't unconditionally use jemalloc because it can be nice to use the // system's default allocator by default. Moreover, jemalloc seems to increase @@ -35,7 +35,11 @@ mod search; // // Moreover, we only do this on 64-bit systems since jemalloc doesn't support // i686. -#[cfg(all(target_env = "musl", target_pointer_width = "64"))] +#[cfg(all( + target_env = "musl", + target_pointer_width = "64", + feature = "jemalloc" +))] #[global_allocator] static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc; -- 2.25.1