Merge pull request #27067 from benesch/prof-sanity

Allow force enabling jemalloc on macOS

.config/hakari.toml

@@ -6,3 +6,8 @@ platforms = [
     "x86_64-apple-darwin",
 ]
 exact-versions = true
+
+[final-excludes]
+third-party = [
+    { "name" = "tikv-jemalloc-sys" }
+]

Cargo.toml

@@ -3,6 +3,7 @@ members = [
     "src/adapter",
     "src/adapter-types",
     "src/alloc",
+    "src/alloc-default",
     "src/arrow-util",
     "src/audit-log",
     "src/avro",

ci/test/lint-deps.sh

@@ -24,19 +24,27 @@ set -euo pipefail
 
 . misc/shlib/shlib.bash
 
-# Explicitly name targets to check dependencies. We support Apple on ARM64 and x86_64 and Linux on x86_64.
+# The crates whose dependency graphs we want to lint.
+entrypoints=(
+    mz-clusterd
+    mz-environmentd
+)
+
+# Explicitly name targets to check dependencies. We support Apple and Linux on ARM64 and x86_64.
 targets=(
     aarch64-apple-darwin
     x86_64-apple-darwin
+    aarch64-unknown-linux-gnu
     x86_64-unknown-linux-gnu
 )
 
 # List of crates to include in the dependency lint, including an explanation why they're listed.
 crates=(
     # Checks that the default allocator is jemalloc on supported platforms, but can
-    # be disabled using --no-default-features
+    # be disabled using --no-default-features or explicitly enabled with --features=jemalloc
     tikv_jemalloc_ctl
     tikv_jemallocator
+    tikv_jemalloc_sys
 )
 
 rewrite=false
@@ -53,7 +61,10 @@ function deps() {
     # output if there is no dependency to any of the specified packages.
     for crate in "${crates[@]}"; do
         # Gather data for the current target, reverse lines, find the dependency hierarchy to the root, reverse lines.
-        output=$(cargo tree --workspace --format ':{lib}:{f}' \
+        # shellcheck disable=SC2046
+        output=$(cargo tree \
+            $(printf -- "-p %s " "${entrypoints[@]}") \
+            --format ':{lib}:{f}' \
             --prefix depth \
             --edges normal,build \
             --locked \
@@ -76,9 +87,11 @@ for target in "${targets[@]}"; do
     if $rewrite; then
         deps > "$resources/$target-default"
         deps --no-default-features > "$resources/$target-no-default-features"
+        deps --features jemalloc > "$resources/$target-jemalloc"
     else
         try diff "$resources/$target-default" <(deps)
         try diff "$resources/$target-no-default-features" <(deps --no-default-features)
+        try diff "$resources/$target-jemalloc" <(deps --features jemalloc)
     fi
 done
 

ci/test/lint-deps/aarch64-apple-darwin-jemalloc

@@ -0,0 +1,18 @@
+# generated by ci/test/lint-deps.sh -- see ci/test/lint-deps/README.md for details
+tikv_jemalloc_ctl
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+tikv_jemallocator
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+tikv_jemalloc_sys
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+4:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+3:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms (*)

ci/test/lint-deps/aarch64-unknown-linux-gnu-default

@@ -0,0 +1,18 @@
+# generated by ci/test/lint-deps.sh -- see ci/test/lint-deps/README.md for details
+tikv_jemalloc_ctl
+0::default,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+tikv_jemallocator
+0::default,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+tikv_jemalloc_sys
+0::default,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+4:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+3:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms (*)

ci/test/lint-deps/aarch64-unknown-linux-gnu-jemalloc

@@ -0,0 +1,18 @@
+# generated by ci/test/lint-deps.sh -- see ci/test/lint-deps/README.md for details
+tikv_jemalloc_ctl
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+tikv_jemallocator
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+tikv_jemalloc_sys
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+4:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+3:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms (*)

ci/test/lint-deps/aarch64-unknown-linux-gnu-no-default-features

@@ -0,0 +1 @@
+# generated by ci/test/lint-deps.sh -- see ci/test/lint-deps/README.md for details

ci/test/lint-deps/x86_64-apple-darwin-jemalloc

@@ -0,0 +1,18 @@
+# generated by ci/test/lint-deps.sh -- see ci/test/lint-deps/README.md for details
+tikv_jemalloc_ctl
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+tikv_jemallocator
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+tikv_jemalloc_sys
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+4:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+3:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms (*)

ci/test/lint-deps/x86_64-unknown-linux-gnu-default

@@ -1,8 +1,18 @@
 # generated by ci/test/lint-deps.sh -- see ci/test/lint-deps/README.md for details
 tikv_jemalloc_ctl
-0:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
-1:mz_prof:default,jemalloc,tikv-jemalloc-ctl,workspace-hack
-2:tikv_jemalloc_ctl:default,use_std
+0::default,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
 tikv_jemallocator
-0:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
-1:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+0::default,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+tikv_jemalloc_sys
+0::default,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+4:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+3:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms (*)

ci/test/lint-deps/x86_64-unknown-linux-gnu-jemalloc

@@ -0,0 +1,18 @@
+# generated by ci/test/lint-deps.sh -- see ci/test/lint-deps/README.md for details
+tikv_jemalloc_ctl
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+tikv_jemallocator
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+tikv_jemalloc_sys
+0::default,jemalloc,mz-alloc-default,tokio-console
+1:mz_alloc:default,jemalloc,tikv-jemallocator,workspace-hack
+2:mz_prof:jemalloc,tikv-jemalloc-ctl
+3:tikv_jemalloc_ctl:default,use_std
+4:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+2:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
+3:tikv_jemalloc_sys:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms (*)

src/alloc-default/Cargo.toml

@@ -0,0 +1,37 @@
+[package]
+name = "mz-alloc-default"
+description = "Activates the best default global memory allocator for the platform."
+version = "0.0.0"
+edition.workspace = true
+rust-version.workspace = true
+publish = false
+
+[lints]
+workspace = true
+
+[dependencies]
+mz-alloc = { path = "../alloc", default-features = false }
+workspace-hack = { version = "0.0.0", path = "../workspace-hack", optional = true }
+
+# We use jemalloc by default on non-macOS platforms, as benchmarks indicated it
+# outperforms the system allocator, while also providing heap profiles.
+#
+# However, on macOS, we default to the system allocator, as jemalloc is not well
+# supported on macOS [0][1][2]. The issues present as runaway latency on load
+# test workloads that are comfortably handled by the macOS system allocator.
+# Consider re-evaluating if jemalloc's macOS support improves.
+#
+# [0]: https://github.com/jemalloc/jemalloc/issues/26
+# [1]: https://github.com/jemalloc/jemalloc/issues/843
+# [2]: https://github.com/jemalloc/jemalloc/issues/1467
+#
+# Furthermore, as of August 2022, some engineers are using profiling tools, e.g.
+# `heaptrack`, that only work with the system allocator on macOS.
+[target.'cfg(not(target_os = "macos"))'.dependencies]
+mz-alloc = { path = "../alloc", features = ["jemalloc"] }
+
+[features]
+default = ["workspace-hack"]
+
+[package.metadata.cargo-udeps.ignore]
+normal = ["workspace-hack", "mz-alloc"]

src/alloc-default/src/lib.rs

@@ -0,0 +1,10 @@
+// Copyright Materialize, Inc. and contributors. All rights reserved.
+//
+// Use of this software is governed by the Business Source License
+// included in the LICENSE file.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0.
+
+//! Activates the best default global memory allocator for the platform.

src/alloc/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mz-alloc"
-description = "Activates the best memory allocator for the target platform."
+description = "Chooses a global memory allocator based on Cargo features."
 version = "0.0.0"
 edition.workspace = true
 rust-version.workspace = true
@@ -11,20 +11,6 @@ workspace = true
 
 [dependencies]
 mz-ore = { path = "../ore", default-features = false }
-workspace-hack = { version = "0.0.0", path = "../workspace-hack", optional = true }
-
-# Disable jemalloc on macOS, as it is not well supported [0][1][2]. The issues
-# present as runaway latency on load test workloads that are comfortably handled
-# by the macOS system allocator. Consider re-evaluating if jemalloc's macOS
-# support improves.
-#
-# [0]: https://github.com/jemalloc/jemalloc/issues/26
-# [1]: https://github.com/jemalloc/jemalloc/issues/843
-# [2]: https://github.com/jemalloc/jemalloc/issues/1467
-#
-# Furthermore, as of August 2022, some engineers are using profiling tools, e.g.
-# `heaptrack`, that only work with the system allocator.
-[target.'cfg(not(target_os = "macos"))'.dependencies]
 mz-prof = { path = "../prof", default-features = false }
 mz-prof-http = { path = "../prof-http", default-features = false }
 # According to jemalloc developers, `background_threads` should always be
@@ -34,16 +20,16 @@ mz-prof-http = { path = "../prof-http", default-features = false }
 #
 # See: https://github.com/jemalloc/jemalloc/issues/956#issuecomment-316224733
 tikv-jemallocator = { version = "0.5.0", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms", "background_threads"], optional = true }
+workspace-hack = { version = "0.0.0", path = "../workspace-hack", optional = true }
 
 [features]
 default = ["workspace-hack"]
-# Whether to enable the use of jemalloc on platforms that support it.
-jemalloc = ["tikv-jemallocator", "mz-prof/jemalloc", "mz-prof-http/jemalloc"]
+# Whether to use jemalloc instead of the system allocator.
+jemalloc = ["tikv-jemallocator", "mz-prof-http/jemalloc"]
 
 [package.metadata.cargo-udeps.ignore]
-# The only reason we depend on mz-prof-http
-# from this package is so that we can force its `jemalloc`
-# option to be enabled when this package's is, which
-# makes all the Materialize binaries correctly
-# serve heap profiling tools at the `/prof` endpoints.
+# The only reason we depend on mz-prof-http from this package is so that we can
+# force its `jemalloc` option to be enabled when this package's is, which makes
+# all the Materialize binaries correctly serve heap profiling tools at the
+# `/prof` endpoints.
 normal = ["workspace-hack", "mz-prof-http"]

src/alloc/src/lib.rs

@@ -7,17 +7,19 @@
 // the Business Source License, use of this software will be governed
 // by the Apache License, Version 2.0.
 
+//! Chooses a global memory allocator based on Cargo features.
+
 use mz_ore::metrics::MetricsRegistry;
 
-#[cfg(all(not(target_os = "macos"), feature = "jemalloc", not(miri)))]
+#[cfg(all(feature = "jemalloc", not(miri)))]
 #[global_allocator]
 static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
 
 /// Registers metrics for the global allocator into the provided registry.
 ///
 /// What metrics are registered varies by platform. Not all platforms use
 /// allocators that support metrics.
-#[cfg(any(target_os = "macos", not(feature = "jemalloc"), miri))]
+#[cfg(any(not(feature = "jemalloc"), miri))]
 #[allow(clippy::unused_async)]
 pub async fn register_metrics_into(_: &MetricsRegistry) {
     // No-op on platforms that don't use jemalloc.
@@ -27,7 +29,7 @@ pub async fn register_metrics_into(_: &MetricsRegistry) {
 ///
 /// What metrics are registered varies by platform. Not all platforms use
 /// allocators that support metrics.
-#[cfg(all(not(target_os = "macos"), feature = "jemalloc", not(miri)))]
+#[cfg(all(feature = "jemalloc", not(miri)))]
 pub async fn register_metrics_into(registry: &MetricsRegistry) {
     mz_prof::jemalloc::JemallocMetrics::register_into(registry).await;
 }

src/clusterd/Cargo.toml

@@ -16,6 +16,7 @@ clap = { version = "3.2.24", features = ["derive", "env"] }
 fail = { version = "0.5.1", features = ["failpoints"] }
 futures = "0.3.25"
 mz-alloc = { path = "../alloc" }
+mz-alloc-default = { path = "../alloc-default", optional = true }
 mz-build-info = { path = "../build-info" }
 mz-cloud-resources = { path = "../cloud-resources" }
 mz-compute = { path = "../compute" }
@@ -42,7 +43,7 @@ tracing = "0.1.37"
 workspace-hack = { version = "0.0.0", path = "../workspace-hack" }
 
 [features]
-default = ["tokio-console", "jemalloc"]
+default = ["tokio-console", "mz-alloc-default"]
 jemalloc = ["mz-alloc/jemalloc"]
 tokio-console = ["mz-ore/tokio-console"]
 

src/environmentd/Cargo.toml

@@ -41,6 +41,7 @@ once_cell = "1.16.0"
 libc = "0.2.138"
 mime = "0.3.16"
 mz-alloc = { path = "../alloc" }
+mz-alloc-default = { path = "../alloc-default", optional = true }
 mz-aws-secrets-controller = { path = "../aws-secrets-controller" }
 mz-build-info = { path = "../build-info" }
 mz-adapter = { path = "../adapter" }
@@ -159,7 +160,7 @@ cc = "1.0.78"
 mz-npm = { path = "../npm" }
 
 [features]
-default = ["tokio-console", "jemalloc"]
+default = ["tokio-console", "mz-alloc-default"]
 # When enabled, static assets for the web UI are loaded from disk on every HTTP
 # request rather than compiled into the binary. This vastly speeds up the
 # iteration cycle when developing the web UI.