Skip to content

Commit e490ebd

Browse files
committed
alloc: extract jemalloc config into new crate
So that we don't need to write the platform-specific logic for choosing the right allocator in both environmentd and clusterd. Also remove a few error paths in `JemallocMetrics::update` by using unsigned types everywhere.
1 parent 34341f4 commit e490ebd

File tree

15 files changed

+195
-120
lines changed

15 files changed

+195
-120
lines changed

Cargo.lock

+12-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
[workspace]
22
members = [
33
"src/adapter",
4+
"src/alloc",
45
"src/audit-log",
56
"src/avro",
67
"src/avro-derive",
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
tikv_jemalloc_ctl
2-
0::default,jemalloc,tikv-jemallocator
2+
0:mz_alloc:jemalloc,tikv-jemallocator
33
1:mz_prof:jemalloc,tikv-jemalloc-ctl
44
2:tikv_jemalloc_ctl:default,use_std
55
tikv_jemallocator
6-
0::default,jemalloc,tikv-jemallocator
6+
0:mz_alloc:jemalloc,tikv-jemallocator
77
1:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms
8-
0:mz_environmentd:default,jemalloc,tikv-jemallocator
9-
1:tikv_jemallocator:background_threads,background_threads_runtime_support,default,profiling,stats,unprefixed_malloc_on_supported_platforms (*)

src/alloc/Cargo.toml

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
[package]
2+
name = "mz-alloc"
3+
description = "Activates the best memory allocator for the target platform."
4+
version = "0.0.0"
5+
edition.workspace = true
6+
rust-version.workspace = true
7+
publish = false
8+
9+
[dependencies]
10+
mz-ore = { path = "../ore" }
11+
workspace-hack = { version = "0.0.0", path = "../workspace-hack" }
12+
13+
# Disable jemalloc on macOS, as it is not well supported [0][1][2]. The issues
14+
# present as runaway latency on load test workloads that are comfortably handled
15+
# by the macOS system allocator. Consider re-evaluating if jemalloc's macOS
16+
# support improves.
17+
#
18+
# [0]: https://github.com/jemalloc/jemalloc/issues/26
19+
# [1]: https://github.com/jemalloc/jemalloc/issues/843
20+
# [2]: https://github.com/jemalloc/jemalloc/issues/1467
21+
#
22+
# Furthermore, as of August 2022, some engineers are using profiling tools, e.g.
23+
# `heaptrack`, that only work with the system allocator.
24+
[target.'cfg(not(target_os = "macos"))'.dependencies]
25+
mz-prof = { path = "../prof" }
26+
# According to jemalloc developers, `background_threads` should always be
27+
# enabled, except in "esoteric" situations that don't apply to Materialize
28+
# (namely, if the application relies on new threads not being created for
29+
# whatever reason).
30+
#
31+
# See: https://github.com/jemalloc/jemalloc/issues/956#issuecomment-316224733
32+
tikv-jemallocator = { version = "0.5.0", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms", "background_threads"], optional = true }
33+
34+
[features]
35+
# Whether to enable the use of jemalloc on platforms that support it.
36+
jemalloc = ["tikv-jemallocator", "mz-prof/jemalloc"]

src/alloc/src/lib.rs

+100
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// Copyright Materialize, Inc. and contributors. All rights reserved.
2+
//
3+
// Use of this software is governed by the Business Source License
4+
// included in the LICENSE file.
5+
//
6+
// As of the Change Date specified in that file, in accordance with
7+
// the Business Source License, use of this software will be governed
8+
// by the Apache License, Version 2.0.
9+
10+
// BEGIN LINT CONFIG
11+
// DO NOT EDIT. Automatically generated by bin/gen-lints.
12+
// Have complaints about the noise? See the note in misc/python/materialize/cli/gen-lints.py first.
13+
#![allow(clippy::style)]
14+
#![allow(clippy::complexity)]
15+
#![allow(clippy::large_enum_variant)]
16+
#![allow(clippy::mutable_key_type)]
17+
#![allow(clippy::stable_sort_primitive)]
18+
#![allow(clippy::map_entry)]
19+
#![allow(clippy::box_default)]
20+
#![warn(clippy::bool_comparison)]
21+
#![warn(clippy::clone_on_ref_ptr)]
22+
#![warn(clippy::no_effect)]
23+
#![warn(clippy::unnecessary_unwrap)]
24+
#![warn(clippy::dbg_macro)]
25+
#![warn(clippy::todo)]
26+
#![warn(clippy::wildcard_dependencies)]
27+
#![warn(clippy::zero_prefixed_literal)]
28+
#![warn(clippy::borrowed_box)]
29+
#![warn(clippy::deref_addrof)]
30+
#![warn(clippy::double_must_use)]
31+
#![warn(clippy::double_parens)]
32+
#![warn(clippy::extra_unused_lifetimes)]
33+
#![warn(clippy::needless_borrow)]
34+
#![warn(clippy::needless_question_mark)]
35+
#![warn(clippy::needless_return)]
36+
#![warn(clippy::redundant_pattern)]
37+
#![warn(clippy::redundant_slicing)]
38+
#![warn(clippy::redundant_static_lifetimes)]
39+
#![warn(clippy::single_component_path_imports)]
40+
#![warn(clippy::unnecessary_cast)]
41+
#![warn(clippy::useless_asref)]
42+
#![warn(clippy::useless_conversion)]
43+
#![warn(clippy::builtin_type_shadow)]
44+
#![warn(clippy::duplicate_underscore_argument)]
45+
#![warn(clippy::double_neg)]
46+
#![warn(clippy::unnecessary_mut_passed)]
47+
#![warn(clippy::wildcard_in_or_patterns)]
48+
#![warn(clippy::collapsible_if)]
49+
#![warn(clippy::collapsible_else_if)]
50+
#![warn(clippy::crosspointer_transmute)]
51+
#![warn(clippy::excessive_precision)]
52+
#![warn(clippy::overflow_check_conditional)]
53+
#![warn(clippy::as_conversions)]
54+
#![warn(clippy::match_overlapping_arm)]
55+
#![warn(clippy::zero_divided_by_zero)]
56+
#![warn(clippy::must_use_unit)]
57+
#![warn(clippy::suspicious_assignment_formatting)]
58+
#![warn(clippy::suspicious_else_formatting)]
59+
#![warn(clippy::suspicious_unary_op_formatting)]
60+
#![warn(clippy::mut_mutex_lock)]
61+
#![warn(clippy::print_literal)]
62+
#![warn(clippy::same_item_push)]
63+
#![warn(clippy::useless_format)]
64+
#![warn(clippy::write_literal)]
65+
#![warn(clippy::redundant_closure)]
66+
#![warn(clippy::redundant_closure_call)]
67+
#![warn(clippy::unnecessary_lazy_evaluations)]
68+
#![warn(clippy::partialeq_ne_impl)]
69+
#![warn(clippy::redundant_field_names)]
70+
#![warn(clippy::transmutes_expressible_as_ptr_casts)]
71+
#![warn(clippy::unused_async)]
72+
#![warn(clippy::disallowed_methods)]
73+
#![warn(clippy::disallowed_macros)]
74+
#![warn(clippy::from_over_into)]
75+
// END LINT CONFIG
76+
77+
use mz_ore::metrics::MetricsRegistry;
78+
79+
#[cfg(all(not(target_os = "macos"), feature = "jemalloc"))]
80+
#[global_allocator]
81+
static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
82+
83+
/// Registers metrics for the global allocator into the provided registry.
84+
///
85+
/// What metrics are registered varies by platform. Not all platforms use
86+
/// allocators that support metrics.
87+
#[cfg(any(target_os = "macos", not(feature = "jemalloc")))]
88+
#[allow(clippy::unused_async)]
89+
pub async fn register_metrics_into(_: &MetricsRegistry) {
90+
// No-op on platforms that don't use jemalloc.
91+
}
92+
93+
/// Registers metrics for the global allocator into the provided registry.
94+
///
95+
/// What metrics are registered varies by platform. Not all platforms use
96+
/// allocators that support metrics.
97+
#[cfg(all(not(target_os = "macos"), feature = "jemalloc"))]
98+
pub async fn register_metrics_into(registry: &MetricsRegistry) {
99+
mz_prof::jemalloc::JemallocMetrics::register_into(registry).await;
100+
}

src/clusterd/Cargo.toml

+2-9
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ axum = "0.6.1"
1212
clap = { version = "3.2.20", features = ["derive", "env"] }
1313
fail = { version = "0.5.1", features = ["failpoints"] }
1414
futures = "0.3.25"
15+
mz-alloc = { path = "../alloc" }
1516
mz-build-info = { path = "../build-info" }
1617
mz-cloud-resources = { path = "../cloud-resources" }
1718
mz-compute = { path = "../compute" }
@@ -32,15 +33,7 @@ tokio = { version = "1.23.0", features = ["fs", "rt", "sync", "test-util"] }
3233
tracing = "0.1.37"
3334
workspace-hack = { version = "0.0.0", path = "../workspace-hack" }
3435

35-
[target.'cfg(not(target_os = "macos"))'.dependencies]
36-
# According to jemalloc developers, `background_threads` should always be
37-
# enabled, except in "esoteric" situations that don't apply to Materialize
38-
# (Namely: if the application relies on new threads not being created for whatever reason)
39-
#
40-
# See: https://github.com/jemalloc/jemalloc/issues/956#issuecomment-316224733
41-
tikv-jemallocator = { version = "0.5.0", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms", "background_threads"], optional = true }
42-
4336
[features]
4437
default = ["jemalloc"]
45-
jemalloc = ["tikv-jemallocator", "mz-prof/jemalloc"]
38+
jemalloc = ["mz-alloc/jemalloc"]
4639
tokio-console = ["mz-ore/tokio-console"]

src/clusterd/src/bin/clusterd.rs

+3-19
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,12 @@ use mz_ore::tracing::TracingHandle;
9898
use mz_persist_client::cache::PersistClientCache;
9999
use mz_persist_client::PersistConfig;
100100
use mz_pid_file::PidFile;
101-
use mz_prof::jemalloc_metrics;
102101
use mz_service::emit_boot_diagnostics;
103102
use mz_service::grpc::GrpcServer;
104103
use mz_service::secrets::SecretsReaderCliArgs;
105104
use mz_storage_client::client::proto_storage_server::ProtoStorageServer;
106105
use mz_storage_client::types::connections::ConnectionContext;
107106

108-
// Disable jemalloc on macOS, as it is not well supported [0][1][2].
109-
// The issues present as runaway latency on load test workloads that are
110-
// comfortably handled by the macOS system allocator. Consider re-evaluating if
111-
// jemalloc's macOS support improves.
112-
//
113-
// [0]: https://github.com/jemalloc/jemalloc/issues/26
114-
// [1]: https://github.com/jemalloc/jemalloc/issues/843
115-
// [2]: https://github.com/jemalloc/jemalloc/issues/1467
116-
//
117-
// Furthermore, as of Aug. 2022, some engineers are using profiling
118-
// tools, e.g. `heaptrack`, that only work with the system allocator.
119-
#[cfg(all(not(target_os = "macos"), feature = "jemalloc"))]
120-
#[global_allocator]
121-
static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
122-
123107
const BUILD_INFO: BuildInfo = build_info!();
124108

125109
pub static VERSION: Lazy<String> = Lazy::new(|| BUILD_INFO.human_version());
@@ -214,6 +198,9 @@ async fn run(args: Args) -> Result<(), anyhow::Error> {
214198

215199
emit_boot_diagnostics!(&BUILD_INFO);
216200

201+
let metrics_registry = MetricsRegistry::new();
202+
mz_alloc::register_metrics_into(&metrics_registry).await;
203+
217204
let mut _pid_file = None;
218205
if let Some(pid_file_location) = &args.pid_file_location {
219206
_pid_file = Some(PidFile::open(pid_file_location).unwrap());
@@ -225,9 +212,6 @@ async fn run(args: Args) -> Result<(), anyhow::Error> {
225212
.await
226213
.context("loading secrets reader")?;
227214

228-
let metrics_registry = MetricsRegistry::new();
229-
jemalloc_metrics::register_into(&metrics_registry);
230-
231215
mz_ore::task::spawn(|| "clusterd_internal_http_server", {
232216
let metrics_registry = metrics_registry.clone();
233217
tracing::info!(

src/environmentd/Cargo.toml

+2-9
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ jsonwebtoken = "8.2.0"
3030
once_cell = "1.16.0"
3131
libc = "0.2.138"
3232
mime = "0.3.16"
33+
mz-alloc = { path = "../alloc" }
3334
mz-build-info = { path = "../build-info" }
3435
mz-adapter = { path = "../adapter" }
3536
mz-cloud-resources = { path = "../cloud-resources" }
@@ -83,14 +84,6 @@ url = "2.3.1"
8384
uuid = "1.2.2"
8485
workspace-hack = { version = "0.0.0", path = "../workspace-hack" }
8586

86-
[target.'cfg(not(target_os = "macos"))'.dependencies]
87-
# According to jemalloc developers, `background_threads` should always be
88-
# enabled, except in "esoteric" situations that don't apply to Materialize
89-
# (Namely: if the application relies on new threads not being created for whatever reason)
90-
#
91-
# See: https://github.com/jemalloc/jemalloc/issues/956#issuecomment-316224733
92-
tikv-jemallocator = { version = "0.5.0", features = ["profiling", "stats", "unprefixed_malloc_on_supported_platforms", "background_threads"], optional = true }
93-
9487
[dev-dependencies]
9588
assert_cmd = "2.0.5"
9689
bytes = "1.3.0"
@@ -126,7 +119,7 @@ default = ["jemalloc"]
126119
# WARNING: For development use only! When enabled, may allow unrestricted read
127120
# access to the file system.
128121
dev-web = []
129-
jemalloc = ["mz-prof/jemalloc", "tikv-jemallocator"]
122+
jemalloc = ["mz-alloc/jemalloc"]
130123
tokio-console = ["mz-ore/tokio-console", "mz-orchestrator-tracing/tokio-console"]
131124

132125
[package.metadata.cargo-udeps.ignore]

src/environmentd/src/bin/environmentd/main.rs

+2-15
Original file line numberDiff line numberDiff line change
@@ -133,21 +133,6 @@ use mz_storage_client::types::connections::ConnectionContext;
133133

134134
mod sys;
135135

136-
// Disable jemalloc on macOS, as it is not well supported [0][1][2].
137-
// The issues present as runaway latency on load test workloads that are
138-
// comfortably handled by the macOS system allocator. Consider re-evaluating if
139-
// jemalloc's macOS support improves.
140-
//
141-
// [0]: https://github.com/jemalloc/jemalloc/issues/26
142-
// [1]: https://github.com/jemalloc/jemalloc/issues/843
143-
// [2]: https://github.com/jemalloc/jemalloc/issues/1467
144-
//
145-
// Furthermore, as of Aug. 2022, some engineers are using profiling
146-
// tools, e.g. `heaptrack`, that only work with the system allocator.
147-
#[cfg(all(not(target_os = "macos"), feature = "jemalloc"))]
148-
#[global_allocator]
149-
static ALLOC: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
150-
151136
static VERSION: Lazy<String> = Lazy::new(|| BUILD_INFO.human_version());
152137
static LONG_VERSION: Lazy<String> = Lazy::new(|| {
153138
iter::once(BUILD_INFO.human_version())
@@ -599,6 +584,8 @@ fn run(mut args: Args) -> Result<(), anyhow::Error> {
599584
let metrics_registry = MetricsRegistry::new();
600585
let metrics = Metrics::register_into(&metrics_registry);
601586

587+
runtime.block_on(mz_alloc::register_metrics_into(&metrics_registry));
588+
602589
// Configure tracing to log the service name when using the process
603590
// orchestrator, which intermingles log output from multiple services. Other
604591
// orchestrators separate log output from different services.

src/environmentd/src/lib.rs

-3
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ use mz_ore::now::NowFn;
107107
use mz_ore::task;
108108
use mz_ore::tracing::TracingHandle;
109109
use mz_persist_client::usage::StorageUsageClient;
110-
use mz_prof::jemalloc_metrics;
111110
use mz_secrets::SecretsController;
112111
use mz_sql::catalog::EnvironmentId;
113112
use mz_stash::Stash;
@@ -252,8 +251,6 @@ pub enum TlsMode {
252251

253252
/// Start an `environmentd` server.
254253
pub async fn serve(config: Config) -> Result<Server, anyhow::Error> {
255-
jemalloc_metrics::register_into(&config.metrics_registry);
256-
257254
let tls = mz_postgres_util::make_tls(&tokio_postgres::config::Config::from_str(
258255
&config.adapter_stash_url,
259256
)?)?;

src/prof/Cargo.toml

+1-3
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,11 @@ mz-ore = { path = "../ore" }
2525
pprof = { git = "https://github.com/MaterializeInc/pprof-rs.git" }
2626
serde = { version = "1.0.152", features = ["derive"] }
2727
tempfile = "3.2.0"
28+
tikv-jemalloc-ctl = { version = "0.5.0", features = ["use_std"], optional = true }
2829
tracing = "0.1.37"
2930
tokio = { version = "1.23.0", features = ["time"] }
3031
workspace-hack = { version = "0.0.0", path = "../workspace-hack" }
3132

32-
[target.'cfg(not(target_os = "macos"))'.dependencies]
33-
tikv-jemalloc-ctl = { version = "0.5.0", features = ["use_std"], optional = true }
34-
3533
[build-dependencies]
3634
anyhow = "1.0.66"
3735
mz-npm = { path = "../npm" }

0 commit comments

Comments
 (0)