From fa59aa4535cb3a30fe08d69e9bfd0ca8c5f4564d Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Fri, 31 Mar 2023 11:00:33 +0100 Subject: [PATCH 1/8] jemalloc experiment --- Cargo.lock | 21 +++++++++++++++++++++ apollo-router/Cargo.toml | 4 ++++ apollo-router/src/main.rs | 8 ++++++++ 3 files changed, 33 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index f95402bb75..d23aba8969 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -337,6 +337,7 @@ dependencies = [ "test-log", "test-span", "thiserror", + "tikv-jemallocator", "tokio", "tokio-rustls", "tokio-stream", @@ -5726,6 +5727,26 @@ dependencies = [ "tower", ] +[[package]] +name = "tikv-jemalloc-sys" +version = "0.5.3+5.3.0-patched" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a678df20055b43e57ef8cddde41cdfda9a3c1a060b67f4c5836dfb1d78543ba8" +dependencies = [ + "cc", + "libc", +] + +[[package]] +name = "tikv-jemallocator" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20612db8a13a6c06d57ec83953694185a367e16945f66565e8028d2c0bd76979" +dependencies = [ + "libc", + "tikv-jemalloc-sys", +] + [[package]] name = "time" version = "0.3.17" diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index af88ae5db5..45f11f0ee3 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -256,3 +256,7 @@ tonic-build = "0.8.4" [[test]] name = "integration_tests" path = "tests/integration_tests.rs" + +[target.'cfg(not(target_env = "msvc"))'.dependencies] +tikv-jemallocator = "0.5" + diff --git a/apollo-router/src/main.rs b/apollo-router/src/main.rs index 9e5ad32e50..3648e808b1 100644 --- a/apollo-router/src/main.rs +++ b/apollo-router/src/main.rs @@ -1,5 +1,13 @@ //! Main entry point for CLI command to start server. +# main.rs +#[cfg(not(target_env = "msvc"))] +use tikv_jemallocator::Jemalloc; + +#[cfg(not(target_env = "msvc"))] +#[global_allocator] +static GLOBAL: Jemalloc = Jemalloc; + fn main() { match apollo_router::main() { Ok(_) => {} From 8e188e9d5de4adf3ac77e35b11c8f4ae54ad410c Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Fri, 31 Mar 2023 11:06:20 +0100 Subject: [PATCH 2/8] check before I commit --- apollo-router/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/apollo-router/src/main.rs b/apollo-router/src/main.rs index 3648e808b1..685f203dec 100644 --- a/apollo-router/src/main.rs +++ b/apollo-router/src/main.rs @@ -1,6 +1,5 @@ //! Main entry point for CLI command to start server. -# main.rs #[cfg(not(target_env = "msvc"))] use tikv_jemallocator::Jemalloc; From 66f339e04f78aaa1721d68b5fd9cdd4fa0cfc0f5 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Fri, 31 Mar 2023 12:10:20 +0100 Subject: [PATCH 3/8] polish up the PR a little and add a changeset --- .changesets/maint_garypen_jemalloc.md | 5 +++++ apollo-router/Cargo.toml | 4 +--- apollo-router/src/executable.rs | 10 ++++++++++ apollo-router/src/main.rs | 7 ------- 4 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 .changesets/maint_garypen_jemalloc.md diff --git a/.changesets/maint_garypen_jemalloc.md b/.changesets/maint_garypen_jemalloc.md new file mode 100644 index 0000000000..0664cd9075 --- /dev/null +++ b/.changesets/maint_garypen_jemalloc.md @@ -0,0 +1,5 @@ +### use jemalloc on linux + +Performance testing and flamegraph analysis suggests that jemalloc on linux is about 35% faster than the default allocator. + +By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/2882 \ No newline at end of file diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index 45f11f0ee3..2c3c0d2dc7 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -209,6 +209,7 @@ uname = "0.1.1" [target.'cfg(unix)'.dependencies] uname = "0.1.1" +tikv-jemallocator = "0.5" [dev-dependencies] ecdsa = { version = "0.15.1", features = ["signing", "pem", "pkcs8"] } @@ -257,6 +258,3 @@ tonic-build = "0.8.4" name = "integration_tests" path = "tests/integration_tests.rs" -[target.'cfg(not(target_env = "msvc"))'.dependencies] -tikv-jemallocator = "0.5" - diff --git a/apollo-router/src/executable.rs b/apollo-router/src/executable.rs index 5b38513cb7..d98a68363f 100644 --- a/apollo-router/src/executable.rs +++ b/apollo-router/src/executable.rs @@ -33,6 +33,16 @@ use crate::router::SchemaSource; use crate::router::ShutdownSource; use crate::EntitlementSource; +// Note: We want to use jemalloc on unix, but we don't enable it if dhat-heap is in use because we +// can only have one global allocator +#[cfg(target = "unix")] +use tikv_jemallocator::Jemalloc; + +#[cfg(target = "unix")] +#[cfg(not(feature = "dhat-heap"))] +#[global_allocator] +static GLOBAL: Jemalloc = Jemalloc; + // Note: the dhat-heap and dhat-ad-hoc features should not be both enabled. We name our functions // and variables identically to prevent this from happening. diff --git a/apollo-router/src/main.rs b/apollo-router/src/main.rs index 685f203dec..9e5ad32e50 100644 --- a/apollo-router/src/main.rs +++ b/apollo-router/src/main.rs @@ -1,12 +1,5 @@ //! Main entry point for CLI command to start server. -#[cfg(not(target_env = "msvc"))] -use tikv_jemallocator::Jemalloc; - -#[cfg(not(target_env = "msvc"))] -#[global_allocator] -static GLOBAL: Jemalloc = Jemalloc; - fn main() { match apollo_router::main() { Ok(_) => {} From a9f8af18d6cb12aed40c444c5c337ff28b9b6faa Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Fri, 31 Mar 2023 15:30:32 +0100 Subject: [PATCH 4/8] fix xtask lint complaint --- apollo-router/src/executable.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/apollo-router/src/executable.rs b/apollo-router/src/executable.rs index d98a68363f..b87bf9cbfa 100644 --- a/apollo-router/src/executable.rs +++ b/apollo-router/src/executable.rs @@ -19,6 +19,10 @@ use clap::Subcommand; use directories::ProjectDirs; #[cfg(any(feature = "dhat-heap", feature = "dhat-ad-hoc"))] use once_cell::sync::OnceCell; +// Note: We want to use jemalloc on unix, but we don't enable it if dhat-heap is in use because we +// can only have one global allocator +#[cfg(target = "unix")] +use tikv_jemallocator::Jemalloc; use url::ParseError; use url::Url; @@ -33,11 +37,6 @@ use crate::router::SchemaSource; use crate::router::ShutdownSource; use crate::EntitlementSource; -// Note: We want to use jemalloc on unix, but we don't enable it if dhat-heap is in use because we -// can only have one global allocator -#[cfg(target = "unix")] -use tikv_jemallocator::Jemalloc; - #[cfg(target = "unix")] #[cfg(not(feature = "dhat-heap"))] #[global_allocator] From 64c1ca5c2ec3db520a250f46424cf23ffd3d99d4 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Thu, 6 Apr 2023 12:18:38 +0100 Subject: [PATCH 5/8] Code review comments --- apollo-router/src/executable.rs | 9 --------- apollo-router/src/main.rs | 10 ++++++++++ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/apollo-router/src/executable.rs b/apollo-router/src/executable.rs index b87bf9cbfa..5b38513cb7 100644 --- a/apollo-router/src/executable.rs +++ b/apollo-router/src/executable.rs @@ -19,10 +19,6 @@ use clap::Subcommand; use directories::ProjectDirs; #[cfg(any(feature = "dhat-heap", feature = "dhat-ad-hoc"))] use once_cell::sync::OnceCell; -// Note: We want to use jemalloc on unix, but we don't enable it if dhat-heap is in use because we -// can only have one global allocator -#[cfg(target = "unix")] -use tikv_jemallocator::Jemalloc; use url::ParseError; use url::Url; @@ -37,11 +33,6 @@ use crate::router::SchemaSource; use crate::router::ShutdownSource; use crate::EntitlementSource; -#[cfg(target = "unix")] -#[cfg(not(feature = "dhat-heap"))] -#[global_allocator] -static GLOBAL: Jemalloc = Jemalloc; - // Note: the dhat-heap and dhat-ad-hoc features should not be both enabled. We name our functions // and variables identically to prevent this from happening. diff --git a/apollo-router/src/main.rs b/apollo-router/src/main.rs index 9e5ad32e50..9c60323215 100644 --- a/apollo-router/src/main.rs +++ b/apollo-router/src/main.rs @@ -1,4 +1,14 @@ //! Main entry point for CLI command to start server. +// Note: We want to use jemalloc on unix, but we don't enable it if dhat-heap is in use because we +// can only have one global allocator +#[cfg(target = "unix")] +#[cfg(not(feature = "dhat-heap"))] +use tikv_jemallocator::Jemalloc; + +#[cfg(target = "unix")] +#[cfg(not(feature = "dhat-heap"))] +#[global_allocator] +static GLOBAL: Jemalloc = Jemalloc; fn main() { match apollo_router::main() { From 9424d797c4c1d98d6533f30aa179e2410ca5f089 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Mon, 17 Apr 2023 10:14:14 +0100 Subject: [PATCH 6/8] improve changelog contents --- .changesets/maint_garypen_jemalloc.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.changesets/maint_garypen_jemalloc.md b/.changesets/maint_garypen_jemalloc.md index 0664cd9075..d166fb95ea 100644 --- a/.changesets/maint_garypen_jemalloc.md +++ b/.changesets/maint_garypen_jemalloc.md @@ -1,5 +1,9 @@ ### use jemalloc on linux -Performance testing and flamegraph analysis suggests that jemalloc on linux is about 35% faster than the default allocator. +Detailed memory investigations of the router in use have revealed that there is a significant amount of memory fragmentation when using the default allocator, glibc, on linux. Performance testing and flamegraph analysis suggests that jemalloc on linux can yield significant performance improvements. In our tests, this figure shows performance to be about 35% faster than the default allocator. The improvement in performance being due to less time spent managing memory fragmentation. -By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/2882 \ No newline at end of file +Not everyone will see a 35% performance improvement in this release of the router. Depending on your usage pattern, you may see more or less than this. If you see a regression, please file an issue with details. + +We have no reason to believe that there are allocation problems on other platforms, so this change is confined to linux. + +By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/2882 From 59c248d4600c06a07ff4bc201e9df7bbd8f1778f Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Mon, 17 Apr 2023 11:10:22 +0100 Subject: [PATCH 7/8] make sure it really is just linux that is changed and not "unix-like" OSs. i.e.: Linux, BSD, OSX... --- apollo-router/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/main.rs b/apollo-router/src/main.rs index 9c60323215..e777d8c6b3 100644 --- a/apollo-router/src/main.rs +++ b/apollo-router/src/main.rs @@ -1,11 +1,11 @@ //! Main entry point for CLI command to start server. // Note: We want to use jemalloc on unix, but we don't enable it if dhat-heap is in use because we // can only have one global allocator -#[cfg(target = "unix")] +#[cfg(target_os = "linux")] #[cfg(not(feature = "dhat-heap"))] use tikv_jemallocator::Jemalloc; -#[cfg(target = "unix")] +#[cfg(target_os = "linux")] #[cfg(not(feature = "dhat-heap"))] #[global_allocator] static GLOBAL: Jemalloc = Jemalloc; From 34aa70e0d77436483695631b2f6e758f5bb781b6 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Mon, 17 Apr 2023 11:13:11 +0100 Subject: [PATCH 8/8] update comment from unix -> linux --- apollo-router/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/main.rs b/apollo-router/src/main.rs index e777d8c6b3..413189a753 100644 --- a/apollo-router/src/main.rs +++ b/apollo-router/src/main.rs @@ -1,5 +1,5 @@ //! Main entry point for CLI command to start server. -// Note: We want to use jemalloc on unix, but we don't enable it if dhat-heap is in use because we +// Note: We want to use jemalloc on linux, but we don't enable it if dhat-heap is in use because we // can only have one global allocator #[cfg(target_os = "linux")] #[cfg(not(feature = "dhat-heap"))]