From 5298f50ad76afb263b87ea5b4e959fd5c7fd7be3 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sat, 3 Apr 2021 11:24:33 +1100 Subject: [PATCH 01/40] First impl of malloc_trim --- Cargo.lock | 6 ++ Cargo.toml | 1 + beacon_node/http_api/Cargo.toml | 1 + beacon_node/http_api/src/lib.rs | 29 +++++++++ common/malloc_ctl/Cargo.toml | 9 +++ common/malloc_ctl/build.rs | 3 + common/malloc_ctl/src/lib.rs | 87 +++++++++++++++++++++++++ common/malloc_ctl/src/trimmer_thread.rs | 20 ++++++ lighthouse/Cargo.toml | 1 + lighthouse/src/main.rs | 18 +++++ 10 files changed, 175 insertions(+) create mode 100644 common/malloc_ctl/Cargo.toml create mode 100644 common/malloc_ctl/build.rs create mode 100644 common/malloc_ctl/src/lib.rs create mode 100644 common/malloc_ctl/src/trimmer_thread.rs diff --git a/Cargo.lock b/Cargo.lock index 10dfc69b042..09d0d7ebc6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2860,6 +2860,7 @@ dependencies = [ "lazy_static", "lighthouse_metrics", "lighthouse_version", + "malloc_ctl", "network", "parking_lot", "serde", @@ -3727,6 +3728,7 @@ dependencies = [ "lighthouse_metrics", "lighthouse_version", "logging", + "malloc_ctl", "remote_signer", "slashing_protection", "slog", @@ -3853,6 +3855,10 @@ dependencies = [ "libc", ] +[[package]] +name = "malloc_ctl" +version = "0.1.0" + [[package]] name = "maplit" version = "1.0.2" diff --git a/Cargo.toml b/Cargo.toml index 8fd60b5ab64..42138700695 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ members = [ "common/lockfile", "common/logging", "common/lru_cache", + "common/malloc_ctl", "common/remote_signer_consumer", "common/slot_clock", "common/task_executor", diff --git a/beacon_node/http_api/Cargo.toml b/beacon_node/http_api/Cargo.toml index 4d5d88d405d..82f943a494b 100644 --- a/beacon_node/http_api/Cargo.toml +++ b/beacon_node/http_api/Cargo.toml @@ -26,6 +26,7 @@ lighthouse_metrics = { path = "../../common/lighthouse_metrics" } lazy_static = "1.4.0" warp_utils = { path = "../../common/warp_utils" } slot_clock = { path = "../../common/slot_clock" } +malloc_ctl = { path = "../../common/malloc_ctl" } eth2_ssz = { path = "../../consensus/ssz" } bs58 = "0.4.0" futures = "0.3.8" diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 653820367c1..219cdeb09fe 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -22,6 +22,7 @@ use block_id::BlockId; use eth2::types::{self as api_types, ValidatorId}; use eth2_libp2p::{types::SyncState, EnrExt, NetworkGlobals, PeerId, PubsubMessage}; use lighthouse_version::version_with_platform; +use malloc_ctl::{eprintln_malloc_stats, malloc_trim, DEFAULT_TRIM}; use network::NetworkMessage; use serde::{Deserialize, Serialize}; use slog::{crit, debug, error, info, warn, Logger}; @@ -2127,6 +2128,32 @@ pub fn serve( }) }); + // GET lighthouse/malloc_stats + let get_lighthouse_malloc_stats = warp::path("lighthouse") + .and(warp::path("malloc_stats")) + .and(warp::path::end()) + .and_then(|| { + blocking_json_task(move || { + eprintln_malloc_stats(); + Ok::<_, warp::reject::Rejection>(()) + }) + }); + + // GET lighthouse/malloc_trim + let get_lighthouse_malloc_trim = warp::path("lighthouse") + .and(warp::path("malloc_trim")) + .and(warp::path::end()) + .and_then(|| { + blocking_json_task(move || { + malloc_trim(DEFAULT_TRIM).map_err(|e| { + warp_utils::reject::custom_server_error(format!( + "malloc_trim failed with code {}", + e + )) + }) + }) + }); + let get_events = eth1_v1 .and(warp::path("events")) .and(warp::path::end()) @@ -2233,6 +2260,8 @@ pub fn serve( .or(get_lighthouse_eth1_deposit_cache.boxed()) .or(get_lighthouse_beacon_states_ssz.boxed()) .or(get_lighthouse_staking.boxed()) + .or(get_lighthouse_malloc_stats.boxed()) + .or(get_lighthouse_malloc_trim.boxed()) .or(get_events.boxed()), ) .or(warp::post().and( diff --git a/common/malloc_ctl/Cargo.toml b/common/malloc_ctl/Cargo.toml new file mode 100644 index 00000000000..bb9a63f65b9 --- /dev/null +++ b/common/malloc_ctl/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "malloc_ctl" +version = "0.1.0" +authors = ["Paul Hauner "] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/common/malloc_ctl/build.rs b/common/malloc_ctl/build.rs new file mode 100644 index 00000000000..71fe4432e8e --- /dev/null +++ b/common/malloc_ctl/build.rs @@ -0,0 +1,3 @@ +fn main() { + // println!("cargo:rustc-link-lib=libc"); +} diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs new file mode 100644 index 00000000000..cafac2c3ab4 --- /dev/null +++ b/common/malloc_ctl/src/lib.rs @@ -0,0 +1,87 @@ +pub mod trimmer_thread; + +pub use std::os::raw::{c_int, c_ulong}; + +/// A default value to be provided to `malloc_trim`. +/// +/// Value sourced from: +/// +/// - https://man7.org/linux/man-pages/man3/mallopt.3.html +pub const DEFAULT_TRIM: c_ulong = 1_024 * 128; + +/// Used to configure malloc internals. +/// +/// Source: +/// +/// https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/malloc/malloc.h#L123 +const M_ARENA_MAX: c_int = -8; + +mod ffi { + extern "C" { + pub fn malloc_trim(__pad: std::os::raw::c_ulong) -> ::std::os::raw::c_int; + } + + extern "C" { + pub fn malloc_stats(); + } + + extern "C" { + pub fn mallopt( + __param: ::std::os::raw::c_int, + __val: ::std::os::raw::c_int, + ) -> ::std::os::raw::c_int; + } +} + +fn into_result(result: c_int) -> Result<(), c_int> { + if result == 1 { + Ok(()) + } else { + Err(result) + } +} + +pub fn malloc_arena_max(num_arenas: c_int) -> Result<(), c_int> { + unsafe { into_result(ffi::mallopt(M_ARENA_MAX, num_arenas)) } +} + +/// Calls `malloc_trim(0)`, freeing up available memory at the expense of CPU time and arena +/// locking. +/// +/// ## Resources +/// +/// - https://man7.org/linux/man-pages/man3/malloc_trim.3.html +pub fn malloc_trim(pad: c_ulong) -> Result<(), c_int> { + unsafe { into_result(ffi::malloc_trim(pad)) } +} + +/// Calls `malloc_stats`, printing the output to stderr. +/// +/// ## Resources +/// +/// - https://man7.org/linux/man-pages/man3/malloc_stats.3.html +pub fn eprintln_malloc_stats() { + unsafe { ffi::malloc_stats() } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn malloc_arena_max_does_not_panic() { + malloc_arena_max(1).unwrap() + } + + #[test] + fn malloc_default_trim_does_not_panic() { + malloc_trim(DEFAULT_TRIM).unwrap() + } + + /// Unfortunately this test will print into the test results, even on success. I don't know any + /// way to avoid this. + #[test] + fn eprintln_malloc_stats_does_not_panic() { + eprintln_malloc_stats() + } +} diff --git a/common/malloc_ctl/src/trimmer_thread.rs b/common/malloc_ctl/src/trimmer_thread.rs new file mode 100644 index 00000000000..e0d8302e195 --- /dev/null +++ b/common/malloc_ctl/src/trimmer_thread.rs @@ -0,0 +1,20 @@ +use crate::{c_ulong, malloc_trim}; +use std::thread; +use std::time::Duration; + +pub const DEFAULT_TRIM_INTERVAL: Duration = Duration::from_secs(60); + +/// Spawns a thread which will call `crate::malloc_trim(trim)`, sleeping `interval` between each +/// call. +/// +/// The function will not call `malloc_trim` on start, the first call will happen after `interval` +/// has elapsed. +pub fn spawn_trimmer_thread(interval: Duration, trim: c_ulong) -> thread::JoinHandle<()> { + thread::spawn(move || loop { + thread::sleep(interval); + + if let Err(e) = malloc_trim(trim) { + eprintln!("malloc_trim failed with {}", e); + } + }) +} diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index cffadf5d826..4b524840711 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -43,6 +43,7 @@ account_utils = { path = "../common/account_utils" } remote_signer = { "path" = "../remote_signer" } lighthouse_metrics = { path = "../common/lighthouse_metrics" } lazy_static = "1.4.0" +malloc_ctl = { path = "../common/malloc_ctl" } [dev-dependencies] tempfile = "3.1.0" diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index acbc80a8468..99c53ac7dd3 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -6,6 +6,11 @@ use env_logger::{Builder, Env}; use environment::EnvironmentBuilder; use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK}; use lighthouse_version::VERSION; +use malloc_ctl::{ + malloc_arena_max, + trimmer_thread::{spawn_trimmer_thread, DEFAULT_TRIM_INTERVAL}, + DEFAULT_TRIM, +}; use slog::{crit, info, warn}; use std::path::PathBuf; use std::process::exit; @@ -27,6 +32,19 @@ fn bls_library_name() -> &'static str { } fn main() { + // Configure malloc as the first thing we do, before it has the change to use the default + // values for anything. + // + // TODO: check for env variable so we don't overwrite it. + if let Err(e) = malloc_arena_max(1) { + eprintln!("Failed (code {}) to set malloc max arena count", e); + exit(1) + } + + // Spawn a thread which will periodically force malloc to "trim" itself, returning fragmented + // memory to the OS. + spawn_trimmer_thread(DEFAULT_TRIM_INTERVAL, DEFAULT_TRIM); + // Parse the CLI parameters. let matches = App::new("Lighthouse") .version(VERSION.replace("Lighthouse/", "").as_str()) From ba95d0d5c5ef3b127870a6a853d72335ad5e4eb5 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sat, 3 Apr 2021 14:40:31 +1100 Subject: [PATCH 02/40] Remove build.rs, improve docs --- common/malloc_ctl/build.rs | 3 --- common/malloc_ctl/src/lib.rs | 12 ++++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) delete mode 100644 common/malloc_ctl/build.rs diff --git a/common/malloc_ctl/build.rs b/common/malloc_ctl/build.rs deleted file mode 100644 index 71fe4432e8e..00000000000 --- a/common/malloc_ctl/build.rs +++ /dev/null @@ -1,3 +0,0 @@ -fn main() { - // println!("cargo:rustc-link-lib=libc"); -} diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs index cafac2c3ab4..d94adec02c3 100644 --- a/common/malloc_ctl/src/lib.rs +++ b/common/malloc_ctl/src/lib.rs @@ -17,14 +17,17 @@ pub const DEFAULT_TRIM: c_ulong = 1_024 * 128; const M_ARENA_MAX: c_int = -8; mod ffi { + /// See: https://man7.org/linux/man-pages/man3/malloc_trim.3.html extern "C" { pub fn malloc_trim(__pad: std::os::raw::c_ulong) -> ::std::os::raw::c_int; } + /// See: https://man7.org/linux/man-pages/man3/malloc_stats.3.html extern "C" { pub fn malloc_stats(); } + /// See: https://man7.org/linux/man-pages/man3/mallopt.3.html extern "C" { pub fn mallopt( __param: ::std::os::raw::c_int, @@ -41,6 +44,15 @@ fn into_result(result: c_int) -> Result<(), c_int> { } } +/// Uses `mallopt` to set the `M_ARENA_MAX` value, specifying the number of memory arenas to be +/// created by malloc. +/// +/// Generally speaking, a smaller arena count reduces memory fragmentation at the cost of memory contention +/// between threads. +/// +/// ## Resources +/// +/// - https://man7.org/linux/man-pages/man3/mallopt.3.html pub fn malloc_arena_max(num_arenas: c_int) -> Result<(), c_int> { unsafe { into_result(ffi::mallopt(M_ARENA_MAX, num_arenas)) } } From 128a5bd84089531342bcbab933eb09b79b9f747a Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sat, 3 Apr 2021 14:56:53 +1100 Subject: [PATCH 03/40] Set mmap threshold to 1mb --- common/malloc_ctl/src/lib.rs | 22 ++++++++++++++++++++-- lighthouse/src/main.rs | 9 +++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs index d94adec02c3..1039149d47c 100644 --- a/common/malloc_ctl/src/lib.rs +++ b/common/malloc_ctl/src/lib.rs @@ -9,11 +9,19 @@ pub use std::os::raw::{c_int, c_ulong}; /// - https://man7.org/linux/man-pages/man3/mallopt.3.html pub const DEFAULT_TRIM: c_ulong = 1_024 * 128; -/// Used to configure malloc internals. +/// A default value to be provided to `malloc_mmap_threshold`. +/// +/// One megabyte. +/// +/// Value chosen so that it will store the values of the validators tree hash cache. +pub const DEFAULT_MMAP_THRESHOLD: c_int = 1_024 * 1_024; + +/// Constants used to configure malloc internals. /// /// Source: /// -/// https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/malloc/malloc.h#L123 +/// https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/malloc/malloc.h#L115-L123 +const M_MMAP_THRESHOLD: c_int = -4; const M_ARENA_MAX: c_int = -8; mod ffi { @@ -57,6 +65,16 @@ pub fn malloc_arena_max(num_arenas: c_int) -> Result<(), c_int> { unsafe { into_result(ffi::mallopt(M_ARENA_MAX, num_arenas)) } } +/// Uses `mallopt` to set the `M_MMAP_THRESHOLD` value, specifying the threshold where objects of this +/// size or larger are allocated via an `mmap`. +/// +/// ## Resources +/// +/// - https://man7.org/linux/man-pages/man3/mallopt.3.html +pub fn malloc_mmap_threshold(num_arenas: c_int) -> Result<(), c_int> { + unsafe { into_result(ffi::mallopt(M_MMAP_THRESHOLD, num_arenas)) } +} + /// Calls `malloc_trim(0)`, freeing up available memory at the expense of CPU time and arena /// locking. /// diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 99c53ac7dd3..0680ca8280f 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -7,9 +7,9 @@ use environment::EnvironmentBuilder; use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK}; use lighthouse_version::VERSION; use malloc_ctl::{ - malloc_arena_max, + malloc_arena_max, malloc_mmap_threshold, trimmer_thread::{spawn_trimmer_thread, DEFAULT_TRIM_INTERVAL}, - DEFAULT_TRIM, + DEFAULT_MMAP_THRESHOLD, DEFAULT_TRIM, }; use slog::{crit, info, warn}; use std::path::PathBuf; @@ -41,6 +41,11 @@ fn main() { exit(1) } + if let Err(e) = malloc_mmap_threshold(DEFAULT_MMAP_THRESHOLD) { + eprintln!("Failed (code {}) to set malloc mmap threshold", e); + exit(1) + } + // Spawn a thread which will periodically force malloc to "trim" itself, returning fragmented // memory to the OS. spawn_trimmer_thread(DEFAULT_TRIM_INTERVAL, DEFAULT_TRIM); From 6182f49593b050ed1e056cf05e0a76611296f490 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sat, 3 Apr 2021 15:32:47 +1100 Subject: [PATCH 04/40] Increase mmap threshold to 2mb --- common/malloc_ctl/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs index 1039149d47c..d515a9673c6 100644 --- a/common/malloc_ctl/src/lib.rs +++ b/common/malloc_ctl/src/lib.rs @@ -11,10 +11,8 @@ pub const DEFAULT_TRIM: c_ulong = 1_024 * 128; /// A default value to be provided to `malloc_mmap_threshold`. /// -/// One megabyte. -/// /// Value chosen so that it will store the values of the validators tree hash cache. -pub const DEFAULT_MMAP_THRESHOLD: c_int = 1_024 * 1_024; +pub const DEFAULT_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// Constants used to configure malloc internals. /// From d9b8d6bd7f991086f3a88c461e9f8e35329a7db8 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 6 Apr 2021 17:32:47 +1000 Subject: [PATCH 05/40] Refactor malloc_ctl API --- beacon_node/http_api/src/lib.rs | 20 +-- common/malloc_ctl/src/glibc.rs | 185 ++++++++++++++++++++++++ common/malloc_ctl/src/lib.rs | 119 ++------------- common/malloc_ctl/src/trimmer_thread.rs | 20 --- lighthouse/src/main.rs | 50 ++++--- 5 files changed, 227 insertions(+), 167 deletions(-) create mode 100644 common/malloc_ctl/src/glibc.rs delete mode 100644 common/malloc_ctl/src/trimmer_thread.rs diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 219cdeb09fe..0cb1b0aee14 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -22,7 +22,7 @@ use block_id::BlockId; use eth2::types::{self as api_types, ValidatorId}; use eth2_libp2p::{types::SyncState, EnrExt, NetworkGlobals, PeerId, PubsubMessage}; use lighthouse_version::version_with_platform; -use malloc_ctl::{eprintln_malloc_stats, malloc_trim, DEFAULT_TRIM}; +use malloc_ctl::eprintln_allocator_stats; use network::NetworkMessage; use serde::{Deserialize, Serialize}; use slog::{crit, debug, error, info, warn, Logger}; @@ -2134,26 +2134,11 @@ pub fn serve( .and(warp::path::end()) .and_then(|| { blocking_json_task(move || { - eprintln_malloc_stats(); + eprintln_allocator_stats(); Ok::<_, warp::reject::Rejection>(()) }) }); - // GET lighthouse/malloc_trim - let get_lighthouse_malloc_trim = warp::path("lighthouse") - .and(warp::path("malloc_trim")) - .and(warp::path::end()) - .and_then(|| { - blocking_json_task(move || { - malloc_trim(DEFAULT_TRIM).map_err(|e| { - warp_utils::reject::custom_server_error(format!( - "malloc_trim failed with code {}", - e - )) - }) - }) - }); - let get_events = eth1_v1 .and(warp::path("events")) .and(warp::path::end()) @@ -2261,7 +2246,6 @@ pub fn serve( .or(get_lighthouse_beacon_states_ssz.boxed()) .or(get_lighthouse_staking.boxed()) .or(get_lighthouse_malloc_stats.boxed()) - .or(get_lighthouse_malloc_trim.boxed()) .or(get_events.boxed()), ) .or(warp::post().and( diff --git a/common/malloc_ctl/src/glibc.rs b/common/malloc_ctl/src/glibc.rs new file mode 100644 index 00000000000..c7cba852796 --- /dev/null +++ b/common/malloc_ctl/src/glibc.rs @@ -0,0 +1,185 @@ +use std::env; +/// Contains functions for tuning and controlling "The GNU Allocator", included in the `glibc` +/// library. +/// +/// https://www.gnu.org/software/libc/manual/html_node/The-GNU-Allocator.html +/// +/// These functions are generally only suitable for Linux systems. +use std::os::raw::{c_int, c_ulong}; +use std::thread; +use std::time::Duration; + +/// A default value to be provided to `malloc_trim`. +/// +/// Value sourced from: +/// +/// - https://man7.org/linux/man-pages/man3/mallopt.3.html +pub const DEFAULT_TRIM: c_ulong = 1_024 * 128; + +/// A default value to be provided to `malloc_mmap_threshold`. +/// +/// Value chosen so that it will store the values of the validators tree hash cache. +const DEFAULT_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; + +/// Constants used to configure malloc internals. +/// +/// Source: +/// +/// https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/malloc/malloc.h#L115-L123 +const M_MMAP_THRESHOLD: c_int = -4; +const M_ARENA_MAX: c_int = -8; + +/// The default interval between calls to `spawn_trimmer_thread`. +const DEFAULT_TRIM_INTERVAL: Duration = Duration::from_secs(60); + +/// Environment variables used to configure malloc. +/// +/// Source: +/// +/// https://man7.org/linux/man-pages/man3/mallopt.3.html +const ENV_VAR_ARENA_MAX: &str = "MALLOC_ARENA_MAX"; +const ENV_VAR_MMAP_THRESHOLD: &str = "MALLOC_MMAP_THRESHOLD_"; + +pub fn configure_glibc_malloc() -> Result<(), String> { + if !env_var_present(ENV_VAR_ARENA_MAX) { + if let Err(e) = malloc_arena_max(1) { + return Err(format!("failed (code {}) to set malloc max arena count", e)); + } + } + + if !env_var_present(ENV_VAR_MMAP_THRESHOLD) { + if let Err(e) = malloc_mmap_threshold(DEFAULT_MMAP_THRESHOLD) { + return Err(format!("failed (code {}) to set malloc mmap threshold", e)); + } + } + + spawn_trimmer_thread(DEFAULT_TRIM_INTERVAL, DEFAULT_TRIM); + + Ok(()) +} + +/// Returns `true` if an environment variable is present. +fn env_var_present(name: &str) -> bool { + env::var(name) != Err(env::VarError::NotPresent) +} + +/// Spawns a thread which will call `crate::malloc_trim(trim)`, sleeping `interval` between each +/// call. +/// +/// The function will not call `malloc_trim` on start, the first call will happen after `interval` +/// has elapsed. +fn spawn_trimmer_thread(interval: Duration, trim: c_ulong) -> thread::JoinHandle<()> { + thread::spawn(move || loop { + thread::sleep(interval); + + if let Err(e) = malloc_trim(trim) { + eprintln!("malloc_trim failed with {}", e); + } + }) +} + +/// Uses `mallopt` to set the `M_ARENA_MAX` value, specifying the number of memory arenas to be +/// created by malloc. +/// +/// Generally speaking, a smaller arena count reduces memory fragmentation at the cost of memory contention +/// between threads. +/// +/// ## Resources +/// +/// - https://man7.org/linux/man-pages/man3/mallopt.3.html +fn malloc_arena_max(num_arenas: c_int) -> Result<(), c_int> { + unsafe { into_result(ffi::mallopt(M_ARENA_MAX, num_arenas)) } +} + +/// Uses `mallopt` to set the `M_MMAP_THRESHOLD` value, specifying the threshold where objects of this +/// size or larger are allocated via an `mmap`. +/// +/// ## Resources +/// +/// - https://man7.org/linux/man-pages/man3/mallopt.3.html +fn malloc_mmap_threshold(num_arenas: c_int) -> Result<(), c_int> { + unsafe { into_result(ffi::mallopt(M_MMAP_THRESHOLD, num_arenas)) } +} + +/// The outcome of calling `malloc_trim`. +pub enum TrimOutcome { + /// Memory was actually released back to the system. + MemoryFreed, + /// It was not possible to release any memory. + NoMemoryFreed, +} + +/// Calls `malloc_trim(0)`, freeing up available memory at the expense of CPU time and arena +/// locking. +/// +/// ## Resources +/// +/// - https://man7.org/linux/man-pages/man3/malloc_trim.3.html +pub fn malloc_trim(pad: c_ulong) -> Result { + unsafe { + match ffi::malloc_trim(pad) { + 0 => Ok(TrimOutcome::NoMemoryFreed), + 1 => Ok(TrimOutcome::MemoryFreed), + other => Err(other), + } + } +} + +/// Calls `malloc_stats`, printing the output to stderr. +/// +/// ## Resources +/// +/// - https://man7.org/linux/man-pages/man3/malloc_stats.3.html +pub fn eprintln_malloc_stats() { + unsafe { ffi::malloc_stats() } +} + +mod ffi { + /// See: https://man7.org/linux/man-pages/man3/malloc_trim.3.html + extern "C" { + pub fn malloc_trim(__pad: std::os::raw::c_ulong) -> ::std::os::raw::c_int; + } + + /// See: https://man7.org/linux/man-pages/man3/malloc_stats.3.html + extern "C" { + pub fn malloc_stats(); + } + + /// See: https://man7.org/linux/man-pages/man3/mallopt.3.html + extern "C" { + pub fn mallopt( + __param: ::std::os::raw::c_int, + __val: ::std::os::raw::c_int, + ) -> ::std::os::raw::c_int; + } +} + +fn into_result(result: c_int) -> Result<(), c_int> { + if result == 1 { + Ok(()) + } else { + Err(result) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn malloc_arena_max_does_not_panic() { + malloc_arena_max(1).unwrap(); + } + + #[test] + fn malloc_default_trim_does_not_panic() { + malloc_trim(DEFAULT_TRIM).unwrap(); + } + + /// Unfortunately this test will print into the test results, even on success. I don't know any + /// way to avoid this. + #[test] + fn eprintln_malloc_stats_does_not_panic() { + eprintln_malloc_stats(); + } +} diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs index d515a9673c6..65b107e9493 100644 --- a/common/malloc_ctl/src/lib.rs +++ b/common/malloc_ctl/src/lib.rs @@ -1,115 +1,22 @@ -pub mod trimmer_thread; +mod glibc; -pub use std::os::raw::{c_int, c_ulong}; +#[cfg(target_os = "linux")] +pub use linux::*; -/// A default value to be provided to `malloc_trim`. -/// -/// Value sourced from: -/// -/// - https://man7.org/linux/man-pages/man3/mallopt.3.html -pub const DEFAULT_TRIM: c_ulong = 1_024 * 128; +#[cfg(not(target_os = "linux"))] +pub use not_linux::*; -/// A default value to be provided to `malloc_mmap_threshold`. -/// -/// Value chosen so that it will store the values of the validators tree hash cache. -pub const DEFAULT_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; - -/// Constants used to configure malloc internals. -/// -/// Source: -/// -/// https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/malloc/malloc.h#L115-L123 -const M_MMAP_THRESHOLD: c_int = -4; -const M_ARENA_MAX: c_int = -8; - -mod ffi { - /// See: https://man7.org/linux/man-pages/man3/malloc_trim.3.html - extern "C" { - pub fn malloc_trim(__pad: std::os::raw::c_ulong) -> ::std::os::raw::c_int; - } - - /// See: https://man7.org/linux/man-pages/man3/malloc_stats.3.html - extern "C" { - pub fn malloc_stats(); - } - - /// See: https://man7.org/linux/man-pages/man3/mallopt.3.html - extern "C" { - pub fn mallopt( - __param: ::std::os::raw::c_int, - __val: ::std::os::raw::c_int, - ) -> ::std::os::raw::c_int; - } +pub mod linux { + pub use crate::glibc::configure_glibc_malloc as configure_memory_allocator; + pub use crate::glibc::eprintln_malloc_stats as eprintln_allocator_stats; } -fn into_result(result: c_int) -> Result<(), c_int> { - if result == 1 { +pub mod not_linux { + #[allow(dead_code)] + fn configure_memory_allocator() -> Result<(), String> { Ok(()) - } else { - Err(result) - } -} - -/// Uses `mallopt` to set the `M_ARENA_MAX` value, specifying the number of memory arenas to be -/// created by malloc. -/// -/// Generally speaking, a smaller arena count reduces memory fragmentation at the cost of memory contention -/// between threads. -/// -/// ## Resources -/// -/// - https://man7.org/linux/man-pages/man3/mallopt.3.html -pub fn malloc_arena_max(num_arenas: c_int) -> Result<(), c_int> { - unsafe { into_result(ffi::mallopt(M_ARENA_MAX, num_arenas)) } -} - -/// Uses `mallopt` to set the `M_MMAP_THRESHOLD` value, specifying the threshold where objects of this -/// size or larger are allocated via an `mmap`. -/// -/// ## Resources -/// -/// - https://man7.org/linux/man-pages/man3/mallopt.3.html -pub fn malloc_mmap_threshold(num_arenas: c_int) -> Result<(), c_int> { - unsafe { into_result(ffi::mallopt(M_MMAP_THRESHOLD, num_arenas)) } -} - -/// Calls `malloc_trim(0)`, freeing up available memory at the expense of CPU time and arena -/// locking. -/// -/// ## Resources -/// -/// - https://man7.org/linux/man-pages/man3/malloc_trim.3.html -pub fn malloc_trim(pad: c_ulong) -> Result<(), c_int> { - unsafe { into_result(ffi::malloc_trim(pad)) } -} - -/// Calls `malloc_stats`, printing the output to stderr. -/// -/// ## Resources -/// -/// - https://man7.org/linux/man-pages/man3/malloc_stats.3.html -pub fn eprintln_malloc_stats() { - unsafe { ffi::malloc_stats() } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn malloc_arena_max_does_not_panic() { - malloc_arena_max(1).unwrap() } - #[test] - fn malloc_default_trim_does_not_panic() { - malloc_trim(DEFAULT_TRIM).unwrap() - } - - /// Unfortunately this test will print into the test results, even on success. I don't know any - /// way to avoid this. - #[test] - fn eprintln_malloc_stats_does_not_panic() { - eprintln_malloc_stats() - } + #[allow(dead_code)] + fn eprintln_allocator_stats() {} } diff --git a/common/malloc_ctl/src/trimmer_thread.rs b/common/malloc_ctl/src/trimmer_thread.rs deleted file mode 100644 index e0d8302e195..00000000000 --- a/common/malloc_ctl/src/trimmer_thread.rs +++ /dev/null @@ -1,20 +0,0 @@ -use crate::{c_ulong, malloc_trim}; -use std::thread; -use std::time::Duration; - -pub const DEFAULT_TRIM_INTERVAL: Duration = Duration::from_secs(60); - -/// Spawns a thread which will call `crate::malloc_trim(trim)`, sleeping `interval` between each -/// call. -/// -/// The function will not call `malloc_trim` on start, the first call will happen after `interval` -/// has elapsed. -pub fn spawn_trimmer_thread(interval: Duration, trim: c_ulong) -> thread::JoinHandle<()> { - thread::spawn(move || loop { - thread::sleep(interval); - - if let Err(e) = malloc_trim(trim) { - eprintln!("malloc_trim failed with {}", e); - } - }) -} diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 0680ca8280f..06c5a5ab3ce 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -6,11 +6,7 @@ use env_logger::{Builder, Env}; use environment::EnvironmentBuilder; use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK}; use lighthouse_version::VERSION; -use malloc_ctl::{ - malloc_arena_max, malloc_mmap_threshold, - trimmer_thread::{spawn_trimmer_thread, DEFAULT_TRIM_INTERVAL}, - DEFAULT_MMAP_THRESHOLD, DEFAULT_TRIM, -}; +use malloc_ctl::configure_memory_allocator; use slog::{crit, info, warn}; use std::path::PathBuf; use std::process::exit; @@ -19,6 +15,9 @@ use validator_client::ProductionValidatorClient; pub const ETH2_CONFIG_FILENAME: &str = "eth2-spec.toml"; +// CLI flags: +pub const DISABLE_MALLOC_TUNING: &str = "disable-malloc-tuning"; + fn bls_library_name() -> &'static str { if cfg!(feature = "portable") { "blst-portable" @@ -32,24 +31,6 @@ fn bls_library_name() -> &'static str { } fn main() { - // Configure malloc as the first thing we do, before it has the change to use the default - // values for anything. - // - // TODO: check for env variable so we don't overwrite it. - if let Err(e) = malloc_arena_max(1) { - eprintln!("Failed (code {}) to set malloc max arena count", e); - exit(1) - } - - if let Err(e) = malloc_mmap_threshold(DEFAULT_MMAP_THRESHOLD) { - eprintln!("Failed (code {}) to set malloc mmap threshold", e); - exit(1) - } - - // Spawn a thread which will periodically force malloc to "trim" itself, returning fragmented - // memory to the OS. - spawn_trimmer_thread(DEFAULT_TRIM_INTERVAL, DEFAULT_TRIM); - // Parse the CLI parameters. let matches = App::new("Lighthouse") .version(VERSION.replace("Lighthouse/", "").as_str()) @@ -148,6 +129,16 @@ fn main() { .global(true) ) + .arg( + Arg::with_name(DISABLE_MALLOC_TUNING) + .long(DISABLE_MALLOC_TUNING) + .help( + "If present, do not configure the system allocator. Providing this flag will \ + generally increase memory usage, it should only be provided when debugging \ + specific memory allocation issues." + ) + .global(true), + ) .subcommand(beacon_node::cli_app()) .subcommand(boot_node::cli_app()) .subcommand(validator_client::cli_app()) @@ -155,6 +146,19 @@ fn main() { .subcommand(remote_signer::cli_app()) .get_matches(); + // Configure the allocator early in the process, before it has the chance to use the default values for + // anything important. + if !matches.is_present(DISABLE_MALLOC_TUNING) { + if let Err(e) = configure_memory_allocator() { + eprintln!( + "Unable to configure the memory allocator: {} \n\ + Try providing the --{} flag", + e, DISABLE_MALLOC_TUNING + ); + exit(1) + } + } + // Debugging output for libp2p and external crates. if matches.is_present("env_log") { Builder::from_env(Env::default()).init(); From 20d90782393f360935bf87be362cc90520274628 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 6 Apr 2021 17:40:52 +1000 Subject: [PATCH 06/40] Ignore clippy lint --- common/malloc_ctl/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs index 65b107e9493..335983a244b 100644 --- a/common/malloc_ctl/src/lib.rs +++ b/common/malloc_ctl/src/lib.rs @@ -12,7 +12,7 @@ pub mod linux { } pub mod not_linux { - #[allow(dead_code)] + #[allow(dead_code, clippy::unnecessary_wraps)] fn configure_memory_allocator() -> Result<(), String> { Ok(()) } From 008826b9379f1b704b28c43dffe2c7e5a6c78158 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 7 Apr 2021 09:46:57 +1000 Subject: [PATCH 07/40] Rename default -> optimal --- common/malloc_ctl/src/glibc.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/common/malloc_ctl/src/glibc.rs b/common/malloc_ctl/src/glibc.rs index c7cba852796..c96bdd24be4 100644 --- a/common/malloc_ctl/src/glibc.rs +++ b/common/malloc_ctl/src/glibc.rs @@ -9,17 +9,20 @@ use std::os::raw::{c_int, c_ulong}; use std::thread; use std::time::Duration; -/// A default value to be provided to `malloc_trim`. +/// The value to be provided to `malloc_trim`. /// /// Value sourced from: /// /// - https://man7.org/linux/man-pages/man3/mallopt.3.html -pub const DEFAULT_TRIM: c_ulong = 1_024 * 128; +const OPTIMAL_TRIM: c_ulong = 1_024 * 128; -/// A default value to be provided to `malloc_mmap_threshold`. +/// The value to be provided to `malloc_mmap_threshold`. /// /// Value chosen so that it will store the values of the validators tree hash cache. -const DEFAULT_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; +const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; + +/// The maximum number of arenas allowed to be created by malloc. +const OPTIMAL_ARENA_MAX: c_int = 1; /// Constants used to configure malloc internals. /// @@ -30,7 +33,7 @@ const M_MMAP_THRESHOLD: c_int = -4; const M_ARENA_MAX: c_int = -8; /// The default interval between calls to `spawn_trimmer_thread`. -const DEFAULT_TRIM_INTERVAL: Duration = Duration::from_secs(60); +const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60); /// Environment variables used to configure malloc. /// @@ -42,18 +45,18 @@ const ENV_VAR_MMAP_THRESHOLD: &str = "MALLOC_MMAP_THRESHOLD_"; pub fn configure_glibc_malloc() -> Result<(), String> { if !env_var_present(ENV_VAR_ARENA_MAX) { - if let Err(e) = malloc_arena_max(1) { + if let Err(e) = malloc_arena_max(OPTIMAL_ARENA_MAX) { return Err(format!("failed (code {}) to set malloc max arena count", e)); } } if !env_var_present(ENV_VAR_MMAP_THRESHOLD) { - if let Err(e) = malloc_mmap_threshold(DEFAULT_MMAP_THRESHOLD) { + if let Err(e) = malloc_mmap_threshold(OPTIMAL_MMAP_THRESHOLD) { return Err(format!("failed (code {}) to set malloc mmap threshold", e)); } } - spawn_trimmer_thread(DEFAULT_TRIM_INTERVAL, DEFAULT_TRIM); + spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM); Ok(()) } @@ -102,7 +105,7 @@ fn malloc_mmap_threshold(num_arenas: c_int) -> Result<(), c_int> { } /// The outcome of calling `malloc_trim`. -pub enum TrimOutcome { +enum TrimOutcome { /// Memory was actually released back to the system. MemoryFreed, /// It was not possible to release any memory. @@ -115,7 +118,7 @@ pub enum TrimOutcome { /// ## Resources /// /// - https://man7.org/linux/man-pages/man3/malloc_trim.3.html -pub fn malloc_trim(pad: c_ulong) -> Result { +fn malloc_trim(pad: c_ulong) -> Result { unsafe { match ffi::malloc_trim(pad) { 0 => Ok(TrimOutcome::NoMemoryFreed), @@ -173,7 +176,7 @@ mod tests { #[test] fn malloc_default_trim_does_not_panic() { - malloc_trim(DEFAULT_TRIM).unwrap(); + malloc_trim(OPTIMAL_TRIM).unwrap(); } /// Unfortunately this test will print into the test results, even on success. I don't know any From c158c13393b42a10ad3c36c909d5aba8dc911e8c Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 7 Apr 2021 09:47:08 +1000 Subject: [PATCH 08/40] Make non-mac functions pub --- common/malloc_ctl/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs index 335983a244b..08a58d5881a 100644 --- a/common/malloc_ctl/src/lib.rs +++ b/common/malloc_ctl/src/lib.rs @@ -13,10 +13,10 @@ pub mod linux { pub mod not_linux { #[allow(dead_code, clippy::unnecessary_wraps)] - fn configure_memory_allocator() -> Result<(), String> { + pub fn configure_memory_allocator() -> Result<(), String> { Ok(()) } #[allow(dead_code)] - fn eprintln_allocator_stats() {} + pub fn eprintln_allocator_stats() {} } From e9ee4c815a57150136350c9ac3e3be6fed483459 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 7 Apr 2021 11:15:12 +1000 Subject: [PATCH 09/40] Add musl conditionals --- common/malloc_ctl/src/lib.rs | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs index 08a58d5881a..93906bcaec1 100644 --- a/common/malloc_ctl/src/lib.rs +++ b/common/malloc_ctl/src/lib.rs @@ -1,17 +1,38 @@ +//! Provides utilities for configuring the system allocator. +//! +//! ## Conditional Compilation +//! +//! Presently, only configuration for "The GNU Allocator" from `glibc` is supported. All other +//! allocators are ignored. +//! +//! It is assumed that if the following two statements are correct, then we should expect to +//! configure `glibc`: +//! +//! - `target_os = linux` +//! - `target_env != musl` +//! +//! In all other cases this library will not attempt to do anything (i.e., all functions are no-ops). +//! +//! ## Notes +//! +//! It's not clear how to precisely determine what the underlying allocator is. The efforts at +//! detecting `glibc` are best-effort. If this crate throws errors about undefined external +//! functions, then try to compile with the `not_glibc_interface` module. + mod glibc; -#[cfg(target_os = "linux")] -pub use linux::*; +#[cfg(all(target_os = "linux", not(target_env = "musl")))] +pub use glibc_interface::*; -#[cfg(not(target_os = "linux"))] -pub use not_linux::*; +#[cfg(any(not(target_os = "linux"), target_env = "musl"))] +pub use not_glibc_interface::*; -pub mod linux { +pub mod glibc_interface { pub use crate::glibc::configure_glibc_malloc as configure_memory_allocator; pub use crate::glibc::eprintln_malloc_stats as eprintln_allocator_stats; } -pub mod not_linux { +pub mod not_glibc_interface { #[allow(dead_code, clippy::unnecessary_wraps)] pub fn configure_memory_allocator() -> Result<(), String> { Ok(()) From 5ce393c42af1134113f54c3cffe8f93325071953 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 8 Apr 2021 10:12:09 +1000 Subject: [PATCH 10/40] Add malloc metrics --- Cargo.lock | 6 ++ beacon_node/http_metrics/Cargo.toml | 1 + beacon_node/http_metrics/src/metrics.rs | 3 + common/malloc_ctl/Cargo.toml | 3 + common/malloc_ctl/src/glibc.rs | 77 +++++++++++++++++++++++-- common/malloc_ctl/src/lib.rs | 8 ++- 6 files changed, 90 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 09d0d7ebc6f..94510196987 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2888,6 +2888,7 @@ dependencies = [ "lazy_static", "lighthouse_metrics", "lighthouse_version", + "malloc_ctl", "prometheus", "reqwest", "serde", @@ -3858,6 +3859,11 @@ dependencies = [ [[package]] name = "malloc_ctl" version = "0.1.0" +dependencies = [ + "lazy_static", + "libc", + "lighthouse_metrics", +] [[package]] name = "maplit" diff --git a/beacon_node/http_metrics/Cargo.toml b/beacon_node/http_metrics/Cargo.toml index 78394874648..8dcf0425ad5 100644 --- a/beacon_node/http_metrics/Cargo.toml +++ b/beacon_node/http_metrics/Cargo.toml @@ -20,6 +20,7 @@ lazy_static = "1.4.0" eth2 = { path = "../../common/eth2" } lighthouse_version = { path = "../../common/lighthouse_version" } warp_utils = { path = "../../common/warp_utils" } +malloc_ctl = { path = "../../common/malloc_ctl" } [dev-dependencies] tokio = { version = "1.1.0", features = ["sync"] } diff --git a/beacon_node/http_metrics/src/metrics.rs b/beacon_node/http_metrics/src/metrics.rs index 3d1b125ead9..c14f6cef1a6 100644 --- a/beacon_node/http_metrics/src/metrics.rs +++ b/beacon_node/http_metrics/src/metrics.rs @@ -1,6 +1,7 @@ use crate::Context; use beacon_chain::BeaconChainTypes; use lighthouse_metrics::{Encoder, TextEncoder}; +use malloc_ctl::scrape_allocator_metrics; pub use lighthouse_metrics::*; @@ -41,6 +42,8 @@ pub fn gather_prometheus_metrics( warp_utils::metrics::scrape_health_metrics(); + scrape_allocator_metrics(); + encoder .encode(&lighthouse_metrics::gather(), &mut buffer) .unwrap(); diff --git a/common/malloc_ctl/Cargo.toml b/common/malloc_ctl/Cargo.toml index bb9a63f65b9..39a716f1186 100644 --- a/common/malloc_ctl/Cargo.toml +++ b/common/malloc_ctl/Cargo.toml @@ -7,3 +7,6 @@ edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +lighthouse_metrics = { path = "../lighthouse_metrics" } +lazy_static = "1.4.0" +libc = "0.2.79" diff --git a/common/malloc_ctl/src/glibc.rs b/common/malloc_ctl/src/glibc.rs index c96bdd24be4..9a6430a46c8 100644 --- a/common/malloc_ctl/src/glibc.rs +++ b/common/malloc_ctl/src/glibc.rs @@ -1,11 +1,15 @@ +//! Contains functions for tuning and controlling "The GNU Allocator", included in the `glibc` +//! library. +//! +//! https://www.gnu.org/software/libc/manual/html_node/The-GNU-Allocator.html +//! +//! These functions are generally only suitable for Linux systems. +use lazy_static::lazy_static; +use libc; +use lighthouse_metrics::*; use std::env; -/// Contains functions for tuning and controlling "The GNU Allocator", included in the `glibc` -/// library. -/// -/// https://www.gnu.org/software/libc/manual/html_node/The-GNU-Allocator.html -/// -/// These functions are generally only suitable for Linux systems. use std::os::raw::{c_int, c_ulong}; +use std::result::Result; use std::thread; use std::time::Duration; @@ -43,6 +47,67 @@ const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60); const ENV_VAR_ARENA_MAX: &str = "MALLOC_ARENA_MAX"; const ENV_VAR_MMAP_THRESHOLD: &str = "MALLOC_MMAP_THRESHOLD_"; +// Metrics for the malloc. For more information, see: +// +// https://man7.org/linux/man-pages/man3/mallinfo.3.html +lazy_static! { + pub static ref MALLINFO_ARENA: lighthouse_metrics::Result = try_create_int_gauge( + "mallinfo_arena", + "The total amount of memory allocated by means other than mmap(2). \ + This figure includes both in-use blocks and blocks on the free list.", + ); + pub static ref MALLINFO_ORDBLKS: lighthouse_metrics::Result = try_create_int_gauge( + "mallinfo_ordblks", + "The number of ordinary (i.e., non-fastbin) free blocks.", + ); + pub static ref MALLINFO_SMBLKS: lighthouse_metrics::Result = + try_create_int_gauge("mallinfo_smblks", "The number of fastbin free blocks.",); + pub static ref MALLINFO_HBLKS: lighthouse_metrics::Result = try_create_int_gauge( + "mallinfo_hblks", + "The number of blocks currently allocated using mmap.", + ); + pub static ref MALLINFO_HBLKHD: lighthouse_metrics::Result = try_create_int_gauge( + "mallinfo_hblkhd", + "The number of bytes in blocks currently allocated using mmap.", + ); + pub static ref MALLINFO_FSMBLKS: lighthouse_metrics::Result = try_create_int_gauge( + "mallinfo_fsmblks", + "The total number of bytes in fastbin free blocks.", + ); + pub static ref MALLINFO_UORDBLKS: lighthouse_metrics::Result = try_create_int_gauge( + "mallinfo_uordblks", + "The total number of bytes used by in-use allocations.", + ); + pub static ref MALLINFO_FORDBLKS: lighthouse_metrics::Result = try_create_int_gauge( + "mallinfo_fordblks", + "The total number of bytes in free blocks.", + ); + pub static ref MALLINFO_KEEPCOST: lighthouse_metrics::Result = try_create_int_gauge( + "mallinfo_keepcost", + "The total amount of releasable free space at the top of the heap..", + ); +} + +/// Calls `mallinfo` and updates Prometheus metrics with the results. +pub fn scrape_mallinfo_metrics() { + // The docs for this function say it is thread-unsafe since it may return inconsistent results. + // Since these are just metrics it's not a concern to us if they're sometimes inconsistent. + // + // Docs: + // + // https://man7.org/linux/man-pages/man3/mallinfo.3.html + let mallinfo = unsafe { libc::mallinfo() }; + + set_gauge(&MALLINFO_ARENA, mallinfo.arena as i64); + set_gauge(&MALLINFO_ORDBLKS, mallinfo.ordblks as i64); + set_gauge(&MALLINFO_SMBLKS, mallinfo.smblks as i64); + set_gauge(&MALLINFO_HBLKHD, mallinfo.hblkhd as i64); + set_gauge(&MALLINFO_FSMBLKS, mallinfo.fsmblks as i64); + set_gauge(&MALLINFO_UORDBLKS, mallinfo.uordblks as i64); + set_gauge(&MALLINFO_FORDBLKS, mallinfo.fordblks as i64); + set_gauge(&MALLINFO_KEEPCOST, mallinfo.keepcost as i64); +} + pub fn configure_glibc_malloc() -> Result<(), String> { if !env_var_present(ENV_VAR_ARENA_MAX) { if let Err(e) = malloc_arena_max(OPTIMAL_ARENA_MAX) { diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_ctl/src/lib.rs index 93906bcaec1..a4a2c359022 100644 --- a/common/malloc_ctl/src/lib.rs +++ b/common/malloc_ctl/src/lib.rs @@ -27,12 +27,13 @@ pub use glibc_interface::*; #[cfg(any(not(target_os = "linux"), target_env = "musl"))] pub use not_glibc_interface::*; -pub mod glibc_interface { +mod glibc_interface { pub use crate::glibc::configure_glibc_malloc as configure_memory_allocator; pub use crate::glibc::eprintln_malloc_stats as eprintln_allocator_stats; + pub use crate::glibc::scrape_mallinfo_metrics as scrape_allocator_metrics; } -pub mod not_glibc_interface { +mod not_glibc_interface { #[allow(dead_code, clippy::unnecessary_wraps)] pub fn configure_memory_allocator() -> Result<(), String> { Ok(()) @@ -40,4 +41,7 @@ pub mod not_glibc_interface { #[allow(dead_code)] pub fn eprintln_allocator_stats() {} + + #[allow(dead_code)] + pub fn scrape_allocator_metrics() {} } From 72888eb0dcd4c83112f30eb92c853becbffcd74f Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 8 Apr 2021 10:37:56 +1000 Subject: [PATCH 11/40] Add missed metric --- common/malloc_ctl/src/glibc.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/common/malloc_ctl/src/glibc.rs b/common/malloc_ctl/src/glibc.rs index 9a6430a46c8..c989fc6ddf1 100644 --- a/common/malloc_ctl/src/glibc.rs +++ b/common/malloc_ctl/src/glibc.rs @@ -101,6 +101,7 @@ pub fn scrape_mallinfo_metrics() { set_gauge(&MALLINFO_ARENA, mallinfo.arena as i64); set_gauge(&MALLINFO_ORDBLKS, mallinfo.ordblks as i64); set_gauge(&MALLINFO_SMBLKS, mallinfo.smblks as i64); + set_gauge(&MALLINFO_HBLKS, mallinfo.hblks as i64); set_gauge(&MALLINFO_HBLKHD, mallinfo.hblkhd as i64); set_gauge(&MALLINFO_FSMBLKS, mallinfo.fsmblks as i64); set_gauge(&MALLINFO_UORDBLKS, mallinfo.uordblks as i64); From ac9aa3cc2788bc0a1e72efa06d57e302dbc89a36 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 8 Apr 2021 10:43:01 +1000 Subject: [PATCH 12/40] Increase malloc_trim interval to 5m --- common/malloc_ctl/src/glibc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/malloc_ctl/src/glibc.rs b/common/malloc_ctl/src/glibc.rs index c989fc6ddf1..a6a7dc05db7 100644 --- a/common/malloc_ctl/src/glibc.rs +++ b/common/malloc_ctl/src/glibc.rs @@ -37,7 +37,7 @@ const M_MMAP_THRESHOLD: c_int = -4; const M_ARENA_MAX: c_int = -8; /// The default interval between calls to `spawn_trimmer_thread`. -const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60); +const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); /// Environment variables used to configure malloc. /// From a636a49301c73fd4f1c9f3a85f64a6abd650a2a6 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 8 Apr 2021 10:47:05 +1000 Subject: [PATCH 13/40] Hold thread join handle --- Cargo.lock | 1 + common/malloc_ctl/Cargo.toml | 1 + common/malloc_ctl/src/glibc.rs | 7 ++++++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 94510196987..e7d775b4105 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3863,6 +3863,7 @@ dependencies = [ "lazy_static", "libc", "lighthouse_metrics", + "parking_lot", ] [[package]] diff --git a/common/malloc_ctl/Cargo.toml b/common/malloc_ctl/Cargo.toml index 39a716f1186..350d62a1f30 100644 --- a/common/malloc_ctl/Cargo.toml +++ b/common/malloc_ctl/Cargo.toml @@ -10,3 +10,4 @@ edition = "2018" lighthouse_metrics = { path = "../lighthouse_metrics" } lazy_static = "1.4.0" libc = "0.2.79" +parking_lot = "0.11.0" diff --git a/common/malloc_ctl/src/glibc.rs b/common/malloc_ctl/src/glibc.rs index a6a7dc05db7..4b2427f26d6 100644 --- a/common/malloc_ctl/src/glibc.rs +++ b/common/malloc_ctl/src/glibc.rs @@ -7,6 +7,7 @@ use lazy_static::lazy_static; use libc; use lighthouse_metrics::*; +use parking_lot::Mutex; use std::env; use std::os::raw::{c_int, c_ulong}; use std::result::Result; @@ -47,6 +48,10 @@ const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); const ENV_VAR_ARENA_MAX: &str = "MALLOC_ARENA_MAX"; const ENV_VAR_MMAP_THRESHOLD: &str = "MALLOC_MMAP_THRESHOLD_"; +lazy_static! { + pub static ref TRIMMER_THREAD_HANDLE: Mutex>> = <_>::default(); +} + // Metrics for the malloc. For more information, see: // // https://man7.org/linux/man-pages/man3/mallinfo.3.html @@ -122,7 +127,7 @@ pub fn configure_glibc_malloc() -> Result<(), String> { } } - spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM); + *TRIMMER_THREAD_HANDLE.lock() = Some(spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM)); Ok(()) } From 035ab392b7ca52b146f83573116aeead6cdd91dd Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 13 Apr 2021 10:37:16 +1000 Subject: [PATCH 14/40] Rename malloc_ctl -> malloc_utils --- Cargo.lock | 8 ++++---- Cargo.toml | 2 +- beacon_node/http_api/Cargo.toml | 2 +- beacon_node/http_api/src/lib.rs | 2 +- beacon_node/http_metrics/Cargo.toml | 2 +- beacon_node/http_metrics/src/metrics.rs | 2 +- common/{malloc_ctl => malloc_utils}/Cargo.toml | 2 +- common/{malloc_ctl => malloc_utils}/src/glibc.rs | 0 common/{malloc_ctl => malloc_utils}/src/lib.rs | 0 lighthouse/Cargo.toml | 2 +- lighthouse/src/main.rs | 2 +- 11 files changed, 12 insertions(+), 12 deletions(-) rename common/{malloc_ctl => malloc_utils}/Cargo.toml (93%) rename common/{malloc_ctl => malloc_utils}/src/glibc.rs (100%) rename common/{malloc_ctl => malloc_utils}/src/lib.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index e7d775b4105..6eff6f8777e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2860,7 +2860,7 @@ dependencies = [ "lazy_static", "lighthouse_metrics", "lighthouse_version", - "malloc_ctl", + "malloc_utils", "network", "parking_lot", "serde", @@ -2888,7 +2888,7 @@ dependencies = [ "lazy_static", "lighthouse_metrics", "lighthouse_version", - "malloc_ctl", + "malloc_utils", "prometheus", "reqwest", "serde", @@ -3729,7 +3729,7 @@ dependencies = [ "lighthouse_metrics", "lighthouse_version", "logging", - "malloc_ctl", + "malloc_utils", "remote_signer", "slashing_protection", "slog", @@ -3857,7 +3857,7 @@ dependencies = [ ] [[package]] -name = "malloc_ctl" +name = "malloc_utils" version = "0.1.0" dependencies = [ "lazy_static", diff --git a/Cargo.toml b/Cargo.toml index 42138700695..4d253285e65 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ members = [ "common/lockfile", "common/logging", "common/lru_cache", - "common/malloc_ctl", + "common/malloc_utils", "common/remote_signer_consumer", "common/slot_clock", "common/task_executor", diff --git a/beacon_node/http_api/Cargo.toml b/beacon_node/http_api/Cargo.toml index 82f943a494b..cfd631ea79d 100644 --- a/beacon_node/http_api/Cargo.toml +++ b/beacon_node/http_api/Cargo.toml @@ -26,7 +26,7 @@ lighthouse_metrics = { path = "../../common/lighthouse_metrics" } lazy_static = "1.4.0" warp_utils = { path = "../../common/warp_utils" } slot_clock = { path = "../../common/slot_clock" } -malloc_ctl = { path = "../../common/malloc_ctl" } +malloc_utils = { path = "../../common/malloc_utils" } eth2_ssz = { path = "../../consensus/ssz" } bs58 = "0.4.0" futures = "0.3.8" diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 0cb1b0aee14..4187dff64ea 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -22,7 +22,7 @@ use block_id::BlockId; use eth2::types::{self as api_types, ValidatorId}; use eth2_libp2p::{types::SyncState, EnrExt, NetworkGlobals, PeerId, PubsubMessage}; use lighthouse_version::version_with_platform; -use malloc_ctl::eprintln_allocator_stats; +use malloc_utils::eprintln_allocator_stats; use network::NetworkMessage; use serde::{Deserialize, Serialize}; use slog::{crit, debug, error, info, warn, Logger}; diff --git a/beacon_node/http_metrics/Cargo.toml b/beacon_node/http_metrics/Cargo.toml index 8dcf0425ad5..e1746781bf1 100644 --- a/beacon_node/http_metrics/Cargo.toml +++ b/beacon_node/http_metrics/Cargo.toml @@ -20,7 +20,7 @@ lazy_static = "1.4.0" eth2 = { path = "../../common/eth2" } lighthouse_version = { path = "../../common/lighthouse_version" } warp_utils = { path = "../../common/warp_utils" } -malloc_ctl = { path = "../../common/malloc_ctl" } +malloc_utils = { path = "../../common/malloc_utils" } [dev-dependencies] tokio = { version = "1.1.0", features = ["sync"] } diff --git a/beacon_node/http_metrics/src/metrics.rs b/beacon_node/http_metrics/src/metrics.rs index c14f6cef1a6..a72a03690f8 100644 --- a/beacon_node/http_metrics/src/metrics.rs +++ b/beacon_node/http_metrics/src/metrics.rs @@ -1,7 +1,7 @@ use crate::Context; use beacon_chain::BeaconChainTypes; use lighthouse_metrics::{Encoder, TextEncoder}; -use malloc_ctl::scrape_allocator_metrics; +use malloc_utils::scrape_allocator_metrics; pub use lighthouse_metrics::*; diff --git a/common/malloc_ctl/Cargo.toml b/common/malloc_utils/Cargo.toml similarity index 93% rename from common/malloc_ctl/Cargo.toml rename to common/malloc_utils/Cargo.toml index 350d62a1f30..685c524212f 100644 --- a/common/malloc_ctl/Cargo.toml +++ b/common/malloc_utils/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "malloc_ctl" +name = "malloc_utils" version = "0.1.0" authors = ["Paul Hauner "] edition = "2018" diff --git a/common/malloc_ctl/src/glibc.rs b/common/malloc_utils/src/glibc.rs similarity index 100% rename from common/malloc_ctl/src/glibc.rs rename to common/malloc_utils/src/glibc.rs diff --git a/common/malloc_ctl/src/lib.rs b/common/malloc_utils/src/lib.rs similarity index 100% rename from common/malloc_ctl/src/lib.rs rename to common/malloc_utils/src/lib.rs diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index 4b524840711..2b3efddd725 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -43,7 +43,7 @@ account_utils = { path = "../common/account_utils" } remote_signer = { "path" = "../remote_signer" } lighthouse_metrics = { path = "../common/lighthouse_metrics" } lazy_static = "1.4.0" -malloc_ctl = { path = "../common/malloc_ctl" } +malloc_utils = { path = "../common/malloc_utils" } [dev-dependencies] tempfile = "3.1.0" diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 06c5a5ab3ce..6bb69c3dfff 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -6,7 +6,7 @@ use env_logger::{Builder, Env}; use environment::EnvironmentBuilder; use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK}; use lighthouse_version::VERSION; -use malloc_ctl::configure_memory_allocator; +use malloc_utils::configure_memory_allocator; use slog::{crit, info, warn}; use std::path::PathBuf; use std::process::exit; From 3a8f7dfe474289fd47997f236e3d8a6aa08f7b09 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 13 Apr 2021 10:38:59 +1000 Subject: [PATCH 15/40] Remove malloc stats endpoint --- Cargo.lock | 1 - beacon_node/http_api/Cargo.toml | 1 - beacon_node/http_api/src/lib.rs | 13 ------------- 3 files changed, 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6eff6f8777e..bb0978007a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2860,7 +2860,6 @@ dependencies = [ "lazy_static", "lighthouse_metrics", "lighthouse_version", - "malloc_utils", "network", "parking_lot", "serde", diff --git a/beacon_node/http_api/Cargo.toml b/beacon_node/http_api/Cargo.toml index cfd631ea79d..4d5d88d405d 100644 --- a/beacon_node/http_api/Cargo.toml +++ b/beacon_node/http_api/Cargo.toml @@ -26,7 +26,6 @@ lighthouse_metrics = { path = "../../common/lighthouse_metrics" } lazy_static = "1.4.0" warp_utils = { path = "../../common/warp_utils" } slot_clock = { path = "../../common/slot_clock" } -malloc_utils = { path = "../../common/malloc_utils" } eth2_ssz = { path = "../../consensus/ssz" } bs58 = "0.4.0" futures = "0.3.8" diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 4187dff64ea..653820367c1 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -22,7 +22,6 @@ use block_id::BlockId; use eth2::types::{self as api_types, ValidatorId}; use eth2_libp2p::{types::SyncState, EnrExt, NetworkGlobals, PeerId, PubsubMessage}; use lighthouse_version::version_with_platform; -use malloc_utils::eprintln_allocator_stats; use network::NetworkMessage; use serde::{Deserialize, Serialize}; use slog::{crit, debug, error, info, warn, Logger}; @@ -2128,17 +2127,6 @@ pub fn serve( }) }); - // GET lighthouse/malloc_stats - let get_lighthouse_malloc_stats = warp::path("lighthouse") - .and(warp::path("malloc_stats")) - .and(warp::path::end()) - .and_then(|| { - blocking_json_task(move || { - eprintln_allocator_stats(); - Ok::<_, warp::reject::Rejection>(()) - }) - }); - let get_events = eth1_v1 .and(warp::path("events")) .and(warp::path::end()) @@ -2245,7 +2233,6 @@ pub fn serve( .or(get_lighthouse_eth1_deposit_cache.boxed()) .or(get_lighthouse_beacon_states_ssz.boxed()) .or(get_lighthouse_staking.boxed()) - .or(get_lighthouse_malloc_stats.boxed()) .or(get_events.boxed()), ) .or(warp::post().and( From 7e0348cbdcc984bbb4f4fb9bc4f480d9a0b4c8b9 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 13 Apr 2021 10:40:28 +1000 Subject: [PATCH 16/40] Merge in `attn-smallvec` (#2182) commit 7f62b7c8e9e3e84dd9e3aab59b4759a8215d54c9 Author: Paul Hauner Date: Mon Feb 1 11:23:58 2021 +1100 Use smallvec for bitfield --- Cargo.lock | 1 + consensus/ssz_types/Cargo.toml | 1 + consensus/ssz_types/src/bitfield.rs | 21 +++++++++++++-------- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bb0978007a8..436f120e6fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2101,6 +2101,7 @@ dependencies = [ "serde_derive", "serde_json", "serde_utils", + "smallvec", "tree_hash", "tree_hash_derive", "typenum", diff --git a/consensus/ssz_types/Cargo.toml b/consensus/ssz_types/Cargo.toml index 39d085b66ed..8ed6ee8e027 100644 --- a/consensus/ssz_types/Cargo.toml +++ b/consensus/ssz_types/Cargo.toml @@ -15,6 +15,7 @@ serde_utils = { path = "../serde_utils" } eth2_ssz = "0.1.2" typenum = "1.12.0" arbitrary = { version = "0.4.6", features = ["derive"], optional = true } +smallvec = "1.6.1" [dev-dependencies] serde_json = "1.0.58" diff --git a/consensus/ssz_types/src/bitfield.rs b/consensus/ssz_types/src/bitfield.rs index 89f1272a3d3..6489880a7a8 100644 --- a/consensus/ssz_types/src/bitfield.rs +++ b/consensus/ssz_types/src/bitfield.rs @@ -4,10 +4,13 @@ use core::marker::PhantomData; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; use serde_utils::hex::{encode as hex_encode, PrefixedHexVisitor}; +use smallvec::{smallvec, SmallVec}; use ssz::{Decode, Encode}; use tree_hash::Hash256; use typenum::Unsigned; +const SMALLVEC_LEN: usize = 32; + /// A marker trait applied to `Variable` and `Fixed` that defines the behaviour of a `Bitfield`. pub trait BitfieldBehaviour: Clone {} @@ -89,7 +92,7 @@ pub type BitVector = Bitfield>; /// bit-index. E.g., `vec![0b0000_0001, 0b0000_0010]` has bits `0, 9` set. #[derive(Clone, Debug, PartialEq)] pub struct Bitfield { - bytes: Vec, + bytes: SmallVec<[u8; 32]>, len: usize, _phantom: PhantomData, } @@ -104,7 +107,7 @@ impl Bitfield> { pub fn with_capacity(num_bits: usize) -> Result { if num_bits <= N::to_usize() { Ok(Self { - bytes: vec![0; bytes_for_bit_len(num_bits)], + bytes: smallvec![0; bytes_for_bit_len(num_bits)], len: num_bits, _phantom: PhantomData, }) @@ -142,8 +145,8 @@ impl Bitfield> { bytes.resize(bytes_for_bit_len(len + 1), 0); - let mut bitfield: Bitfield> = Bitfield::from_raw_bytes(bytes, len + 1) - .unwrap_or_else(|_| { + let mut bitfield: Bitfield> = + Bitfield::from_raw_bytes(bytes.into_vec(), len + 1).unwrap_or_else(|_| { unreachable!( "Bitfield with {} bytes must have enough capacity for {} bits.", bytes_for_bit_len(len + 1), @@ -154,7 +157,7 @@ impl Bitfield> { .set(len, true) .expect("len must be in bounds for bitfield."); - bitfield.bytes + bitfield.bytes.into_vec() } /// Instantiates a new instance from `bytes`. Consumes the same format that `self.into_bytes()` @@ -233,7 +236,7 @@ impl Bitfield> { /// All bits are initialized to `false`. pub fn new() -> Self { Self { - bytes: vec![0; bytes_for_bit_len(Self::capacity())], + bytes: smallvec![0; bytes_for_bit_len(Self::capacity())], len: Self::capacity(), _phantom: PhantomData, } @@ -328,7 +331,7 @@ impl Bitfield { /// Returns the underlying bytes representation of the bitfield. pub fn into_raw_bytes(self) -> Vec { - self.bytes + self.bytes.into_vec() } /// Returns a view into the underlying bytes representation of the bitfield. @@ -345,8 +348,10 @@ impl Bitfield { /// - `bit_len` is not a multiple of 8 and `bytes` contains set bits that are higher than, or /// equal to `bit_len`. fn from_raw_bytes(bytes: Vec, bit_len: usize) -> Result { + let bytes: SmallVec<[u8; SMALLVEC_LEN]> = bytes.into(); + if bit_len == 0 { - if bytes.len() == 1 && bytes == [0] { + if bytes.len() == 1 && bytes.as_slice() == [0] { // A bitfield with `bit_len` 0 can only be represented by a single zero byte. Ok(Self { bytes, From 27f1fd0831eb5bfd4c8ae97b25be84d3c7c0fec3 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 13 Apr 2021 10:49:04 +1000 Subject: [PATCH 17/40] Ensure glibc isn't compiled on non-linux --- common/malloc_utils/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/common/malloc_utils/src/lib.rs b/common/malloc_utils/src/lib.rs index a4a2c359022..424a2071d25 100644 --- a/common/malloc_utils/src/lib.rs +++ b/common/malloc_utils/src/lib.rs @@ -23,16 +23,16 @@ mod glibc; #[cfg(all(target_os = "linux", not(target_env = "musl")))] pub use glibc_interface::*; - -#[cfg(any(not(target_os = "linux"), target_env = "musl"))] -pub use not_glibc_interface::*; - +#[cfg(all(target_os = "linux", not(target_env = "musl")))] mod glibc_interface { pub use crate::glibc::configure_glibc_malloc as configure_memory_allocator; pub use crate::glibc::eprintln_malloc_stats as eprintln_allocator_stats; pub use crate::glibc::scrape_mallinfo_metrics as scrape_allocator_metrics; } +#[cfg(any(not(target_os = "linux"), target_env = "musl"))] +pub use not_glibc_interface::*; +#[cfg(any(not(target_os = "linux"), target_env = "musl"))] mod not_glibc_interface { #[allow(dead_code, clippy::unnecessary_wraps)] pub fn configure_memory_allocator() -> Result<(), String> { From 35d3e12c75146f78d3844ddd5d5c1f09688ff1dc Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 13 Apr 2021 10:51:04 +1000 Subject: [PATCH 18/40] Tidy conditional compilation --- common/malloc_utils/src/lib.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/common/malloc_utils/src/lib.rs b/common/malloc_utils/src/lib.rs index 424a2071d25..446c0e012d8 100644 --- a/common/malloc_utils/src/lib.rs +++ b/common/malloc_utils/src/lib.rs @@ -19,21 +19,20 @@ //! detecting `glibc` are best-effort. If this crate throws errors about undefined external //! functions, then try to compile with the `not_glibc_interface` module. +#[cfg(all(target_os = "linux", not(target_env = "musl")))] mod glibc; +pub use interface::*; + #[cfg(all(target_os = "linux", not(target_env = "musl")))] -pub use glibc_interface::*; -#[cfg(all(target_os = "linux", not(target_env = "musl")))] -mod glibc_interface { +mod interface { pub use crate::glibc::configure_glibc_malloc as configure_memory_allocator; pub use crate::glibc::eprintln_malloc_stats as eprintln_allocator_stats; pub use crate::glibc::scrape_mallinfo_metrics as scrape_allocator_metrics; } #[cfg(any(not(target_os = "linux"), target_env = "musl"))] -pub use not_glibc_interface::*; -#[cfg(any(not(target_os = "linux"), target_env = "musl"))] -mod not_glibc_interface { +mod interface { #[allow(dead_code, clippy::unnecessary_wraps)] pub fn configure_memory_allocator() -> Result<(), String> { Ok(()) From 58a89a13412afa93881d35b284025c25c16799dc Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 13 Apr 2021 11:26:55 +1000 Subject: [PATCH 19/40] Fix clippy lint --- common/malloc_utils/src/glibc.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 4b2427f26d6..5b754fb182a 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -5,7 +5,6 @@ //! //! These functions are generally only suitable for Linux systems. use lazy_static::lazy_static; -use libc; use lighthouse_metrics::*; use parking_lot::Mutex; use std::env; From b7b6ac4af26243188b2d806a5255e46e315d07d8 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 13 Apr 2021 11:27:08 +1000 Subject: [PATCH 20/40] Disable trimmer --- common/malloc_utils/src/glibc.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 5b754fb182a..a9e70436f8e 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -13,6 +13,9 @@ use std::result::Result; use std::thread; use std::time::Duration; +/// If true, periodically call `malloc_trim`. +const START_TRIMMER: bool = false; + /// The value to be provided to `malloc_trim`. /// /// Value sourced from: @@ -20,6 +23,9 @@ use std::time::Duration; /// - https://man7.org/linux/man-pages/man3/mallopt.3.html const OPTIMAL_TRIM: c_ulong = 1_024 * 128; +/// The default interval between calls to `spawn_trimmer_thread`. +const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); + /// The value to be provided to `malloc_mmap_threshold`. /// /// Value chosen so that it will store the values of the validators tree hash cache. @@ -36,9 +42,6 @@ const OPTIMAL_ARENA_MAX: c_int = 1; const M_MMAP_THRESHOLD: c_int = -4; const M_ARENA_MAX: c_int = -8; -/// The default interval between calls to `spawn_trimmer_thread`. -const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); - /// Environment variables used to configure malloc. /// /// Source: @@ -126,7 +129,10 @@ pub fn configure_glibc_malloc() -> Result<(), String> { } } - *TRIMMER_THREAD_HANDLE.lock() = Some(spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM)); + if START_TRIMMER { + *TRIMMER_THREAD_HANDLE.lock() = + Some(spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM)); + } Ok(()) } From f53174a97b3e9f9e05a3e6b79f7152e8ebe9129b Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 14 Apr 2021 11:34:07 +1000 Subject: [PATCH 21/40] Revert "Disable trimmer" This reverts commit 4bda1a75b576d0241d12939acabb57a16cc4d978. --- common/malloc_utils/src/glibc.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index a9e70436f8e..5b754fb182a 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -13,9 +13,6 @@ use std::result::Result; use std::thread; use std::time::Duration; -/// If true, periodically call `malloc_trim`. -const START_TRIMMER: bool = false; - /// The value to be provided to `malloc_trim`. /// /// Value sourced from: @@ -23,9 +20,6 @@ const START_TRIMMER: bool = false; /// - https://man7.org/linux/man-pages/man3/mallopt.3.html const OPTIMAL_TRIM: c_ulong = 1_024 * 128; -/// The default interval between calls to `spawn_trimmer_thread`. -const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); - /// The value to be provided to `malloc_mmap_threshold`. /// /// Value chosen so that it will store the values of the validators tree hash cache. @@ -42,6 +36,9 @@ const OPTIMAL_ARENA_MAX: c_int = 1; const M_MMAP_THRESHOLD: c_int = -4; const M_ARENA_MAX: c_int = -8; +/// The default interval between calls to `spawn_trimmer_thread`. +const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); + /// Environment variables used to configure malloc. /// /// Source: @@ -129,10 +126,7 @@ pub fn configure_glibc_malloc() -> Result<(), String> { } } - if START_TRIMMER { - *TRIMMER_THREAD_HANDLE.lock() = - Some(spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM)); - } + *TRIMMER_THREAD_HANDLE.lock() = Some(spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM)); Ok(()) } From c57265e71e4afea3783e386ca96006635da65025 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 14 Apr 2021 11:51:17 +1000 Subject: [PATCH 22/40] Set arenas to 4 --- common/malloc_utils/src/glibc.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 5b754fb182a..44944106bf0 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -26,7 +26,7 @@ const OPTIMAL_TRIM: c_ulong = 1_024 * 128; const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// The maximum number of arenas allowed to be created by malloc. -const OPTIMAL_ARENA_MAX: c_int = 1; +const OPTIMAL_ARENA_MAX: c_int = 4; /// Constants used to configure malloc internals. /// @@ -241,7 +241,7 @@ mod tests { #[test] fn malloc_arena_max_does_not_panic() { - malloc_arena_max(1).unwrap(); + malloc_arena_max(OPTIMAL_ARENA_MAX).unwrap(); } #[test] From 443c7e86ab96f5ff74236c46498be7f88dd34cce Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 19 Apr 2021 09:16:56 +1000 Subject: [PATCH 23/40] Un-revert "Disable trimmer" This reverts commit f53174a97b3e9f9e05a3e6b79f7152e8ebe9129b. --- common/malloc_utils/src/glibc.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 44944106bf0..947fa734006 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -13,6 +13,9 @@ use std::result::Result; use std::thread; use std::time::Duration; +/// If true, periodically call `malloc_trim`. +const START_TRIMMER: bool = false; + /// The value to be provided to `malloc_trim`. /// /// Value sourced from: @@ -20,6 +23,9 @@ use std::time::Duration; /// - https://man7.org/linux/man-pages/man3/mallopt.3.html const OPTIMAL_TRIM: c_ulong = 1_024 * 128; +/// The default interval between calls to `spawn_trimmer_thread`. +const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); + /// The value to be provided to `malloc_mmap_threshold`. /// /// Value chosen so that it will store the values of the validators tree hash cache. @@ -36,9 +42,6 @@ const OPTIMAL_ARENA_MAX: c_int = 4; const M_MMAP_THRESHOLD: c_int = -4; const M_ARENA_MAX: c_int = -8; -/// The default interval between calls to `spawn_trimmer_thread`. -const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); - /// Environment variables used to configure malloc. /// /// Source: @@ -126,7 +129,10 @@ pub fn configure_glibc_malloc() -> Result<(), String> { } } - *TRIMMER_THREAD_HANDLE.lock() = Some(spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM)); + if START_TRIMMER { + *TRIMMER_THREAD_HANDLE.lock() = + Some(spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM)); + } Ok(()) } From 38d851f011fa3c832056566ca2bdf39f4c80aa6e Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 26 Apr 2021 15:44:49 +1000 Subject: [PATCH 24/40] Set arena max to 0 --- common/malloc_utils/src/glibc.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 947fa734006..8c6ed3960f2 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -32,7 +32,10 @@ const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// The maximum number of arenas allowed to be created by malloc. -const OPTIMAL_ARENA_MAX: c_int = 4; +/// +/// When set to `0`, the limit on the number of arenas is determined by the number of CPU cores +/// online. +const OPTIMAL_ARENA_MAX: c_int = 0; /// Constants used to configure malloc internals. /// From a6bd680fc666be582ed478b55775dbc663f1e92c Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 26 Apr 2021 15:58:56 +1000 Subject: [PATCH 25/40] Disable call to set arena max --- common/malloc_utils/src/glibc.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 8c6ed3960f2..22b35ad9433 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -33,9 +33,11 @@ const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// The maximum number of arenas allowed to be created by malloc. /// -/// When set to `0`, the limit on the number of arenas is determined by the number of CPU cores +/// When set to `Some(0)`, the limit on the number of arenas is determined by the number of CPU cores /// online. -const OPTIMAL_ARENA_MAX: c_int = 0; +/// +/// When set to `None`, the parameter is not set using this program (i.e., it is left as default). +const OPTIMAL_ARENA_MAX: Option = None; /// Constants used to configure malloc internals. /// @@ -120,9 +122,11 @@ pub fn scrape_mallinfo_metrics() { } pub fn configure_glibc_malloc() -> Result<(), String> { - if !env_var_present(ENV_VAR_ARENA_MAX) { - if let Err(e) = malloc_arena_max(OPTIMAL_ARENA_MAX) { - return Err(format!("failed (code {}) to set malloc max arena count", e)); + if let Some(optimal) = OPTIMAL_ARENA_MAX { + if !env_var_present(ENV_VAR_ARENA_MAX) { + if let Err(e) = malloc_arena_max(optimal) { + return Err(format!("failed (code {}) to set malloc max arena count", e)); + } } } @@ -250,7 +254,7 @@ mod tests { #[test] fn malloc_arena_max_does_not_panic() { - malloc_arena_max(OPTIMAL_ARENA_MAX).unwrap(); + malloc_arena_max(OPTIMAL_ARENA_MAX.unwrap_or(2)).unwrap(); } #[test] From acdecf674398a86d3306d28981477f37b7a2307a Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 4 May 2021 12:12:02 +1000 Subject: [PATCH 26/40] Set arena max to 1 --- common/malloc_utils/src/glibc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 22b35ad9433..7fefef8e011 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -37,7 +37,7 @@ const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// online. /// /// When set to `None`, the parameter is not set using this program (i.e., it is left as default). -const OPTIMAL_ARENA_MAX: Option = None; +const OPTIMAL_ARENA_MAX: Option = Some(1); /// Constants used to configure malloc internals. /// From 4762a89000c600626d82bc042162c352192315ea Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 26 May 2021 17:52:50 +1000 Subject: [PATCH 27/40] Set arena max to 4 --- common/malloc_utils/src/glibc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 7fefef8e011..e2e84dfb99e 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -37,7 +37,7 @@ const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// online. /// /// When set to `None`, the parameter is not set using this program (i.e., it is left as default). -const OPTIMAL_ARENA_MAX: Option = Some(1); +const OPTIMAL_ARENA_MAX: Option = Some(4); /// Constants used to configure malloc internals. /// From 31637967f7e80ecbbe03db0e456b42eb2572370a Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 26 May 2021 18:05:07 +1000 Subject: [PATCH 28/40] Set arena to CPU count --- Cargo.lock | 1 + common/malloc_utils/Cargo.toml | 1 + common/malloc_utils/src/glibc.rs | 24 ++++++++++++++++++++---- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 516dfb04ece..24257c76575 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3884,6 +3884,7 @@ dependencies = [ "lazy_static", "libc", "lighthouse_metrics", + "num_cpus", "parking_lot", ] diff --git a/common/malloc_utils/Cargo.toml b/common/malloc_utils/Cargo.toml index 685c524212f..89cc0d36256 100644 --- a/common/malloc_utils/Cargo.toml +++ b/common/malloc_utils/Cargo.toml @@ -11,3 +11,4 @@ lighthouse_metrics = { path = "../lighthouse_metrics" } lazy_static = "1.4.0" libc = "0.2.79" parking_lot = "0.11.0" +num_cpus = "1.13.0" diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index e2e84dfb99e..0ac47e6fe55 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -37,7 +37,7 @@ const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// online. /// /// When set to `None`, the parameter is not set using this program (i.e., it is left as default). -const OPTIMAL_ARENA_MAX: Option = Some(4); +const OPTIMAL_ARENA_MAX: ArenaMaxSetting = ArenaMaxSetting::NumCpus; /// Constants used to configure malloc internals. /// @@ -55,6 +55,16 @@ const M_ARENA_MAX: c_int = -8; const ENV_VAR_ARENA_MAX: &str = "MALLOC_ARENA_MAX"; const ENV_VAR_MMAP_THRESHOLD: &str = "MALLOC_MMAP_THRESHOLD_"; +#[allow(dead_code)] +enum ArenaMaxSetting { + /// Do not set any value for MALLOC_ARENA_MAX, leave it as default. + DoNotSet, + /// Set a fixed value. + Fixed(c_int), + /// Read the number of CPUs at runtime and use that value. + NumCpus, +} + lazy_static! { pub static ref TRIMMER_THREAD_HANDLE: Mutex>> = <_>::default(); } @@ -122,9 +132,15 @@ pub fn scrape_mallinfo_metrics() { } pub fn configure_glibc_malloc() -> Result<(), String> { - if let Some(optimal) = OPTIMAL_ARENA_MAX { + let arena_max = match OPTIMAL_ARENA_MAX { + ArenaMaxSetting::DoNotSet => None, + ArenaMaxSetting::Fixed(n) => Some(n), + ArenaMaxSetting::NumCpus => Some(num_cpus::get() as c_int), + }; + + if let Some(max) = arena_max { if !env_var_present(ENV_VAR_ARENA_MAX) { - if let Err(e) = malloc_arena_max(optimal) { + if let Err(e) = malloc_arena_max(max) { return Err(format!("failed (code {}) to set malloc max arena count", e)); } } @@ -254,7 +270,7 @@ mod tests { #[test] fn malloc_arena_max_does_not_panic() { - malloc_arena_max(OPTIMAL_ARENA_MAX.unwrap_or(2)).unwrap(); + malloc_arena_max(2).unwrap(); } #[test] From 9a892d233dd57a10537286703346fea3feecbbd6 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 27 May 2021 14:49:35 +1000 Subject: [PATCH 29/40] Remove trim, eprintln --- common/malloc_utils/src/glibc.rs | 98 +------------------------------- common/malloc_utils/src/lib.rs | 4 -- 2 files changed, 1 insertion(+), 101 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 0ac47e6fe55..5b46067e8b0 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -6,25 +6,9 @@ //! These functions are generally only suitable for Linux systems. use lazy_static::lazy_static; use lighthouse_metrics::*; -use parking_lot::Mutex; use std::env; -use std::os::raw::{c_int, c_ulong}; +use std::os::raw::c_int; use std::result::Result; -use std::thread; -use std::time::Duration; - -/// If true, periodically call `malloc_trim`. -const START_TRIMMER: bool = false; - -/// The value to be provided to `malloc_trim`. -/// -/// Value sourced from: -/// -/// - https://man7.org/linux/man-pages/man3/mallopt.3.html -const OPTIMAL_TRIM: c_ulong = 1_024 * 128; - -/// The default interval between calls to `spawn_trimmer_thread`. -const OPTIMAL_TRIM_INTERVAL: Duration = Duration::from_secs(60 * 5); /// The value to be provided to `malloc_mmap_threshold`. /// @@ -65,10 +49,6 @@ enum ArenaMaxSetting { NumCpus, } -lazy_static! { - pub static ref TRIMMER_THREAD_HANDLE: Mutex>> = <_>::default(); -} - // Metrics for the malloc. For more information, see: // // https://man7.org/linux/man-pages/man3/mallinfo.3.html @@ -152,11 +132,6 @@ pub fn configure_glibc_malloc() -> Result<(), String> { } } - if START_TRIMMER { - *TRIMMER_THREAD_HANDLE.lock() = - Some(spawn_trimmer_thread(OPTIMAL_TRIM_INTERVAL, OPTIMAL_TRIM)); - } - Ok(()) } @@ -165,21 +140,6 @@ fn env_var_present(name: &str) -> bool { env::var(name) != Err(env::VarError::NotPresent) } -/// Spawns a thread which will call `crate::malloc_trim(trim)`, sleeping `interval` between each -/// call. -/// -/// The function will not call `malloc_trim` on start, the first call will happen after `interval` -/// has elapsed. -fn spawn_trimmer_thread(interval: Duration, trim: c_ulong) -> thread::JoinHandle<()> { - thread::spawn(move || loop { - thread::sleep(interval); - - if let Err(e) = malloc_trim(trim) { - eprintln!("malloc_trim failed with {}", e); - } - }) -} - /// Uses `mallopt` to set the `M_ARENA_MAX` value, specifying the number of memory arenas to be /// created by malloc. /// @@ -203,51 +163,7 @@ fn malloc_mmap_threshold(num_arenas: c_int) -> Result<(), c_int> { unsafe { into_result(ffi::mallopt(M_MMAP_THRESHOLD, num_arenas)) } } -/// The outcome of calling `malloc_trim`. -enum TrimOutcome { - /// Memory was actually released back to the system. - MemoryFreed, - /// It was not possible to release any memory. - NoMemoryFreed, -} - -/// Calls `malloc_trim(0)`, freeing up available memory at the expense of CPU time and arena -/// locking. -/// -/// ## Resources -/// -/// - https://man7.org/linux/man-pages/man3/malloc_trim.3.html -fn malloc_trim(pad: c_ulong) -> Result { - unsafe { - match ffi::malloc_trim(pad) { - 0 => Ok(TrimOutcome::NoMemoryFreed), - 1 => Ok(TrimOutcome::MemoryFreed), - other => Err(other), - } - } -} - -/// Calls `malloc_stats`, printing the output to stderr. -/// -/// ## Resources -/// -/// - https://man7.org/linux/man-pages/man3/malloc_stats.3.html -pub fn eprintln_malloc_stats() { - unsafe { ffi::malloc_stats() } -} - mod ffi { - /// See: https://man7.org/linux/man-pages/man3/malloc_trim.3.html - extern "C" { - pub fn malloc_trim(__pad: std::os::raw::c_ulong) -> ::std::os::raw::c_int; - } - - /// See: https://man7.org/linux/man-pages/man3/malloc_stats.3.html - extern "C" { - pub fn malloc_stats(); - } - - /// See: https://man7.org/linux/man-pages/man3/mallopt.3.html extern "C" { pub fn mallopt( __param: ::std::os::raw::c_int, @@ -272,16 +188,4 @@ mod tests { fn malloc_arena_max_does_not_panic() { malloc_arena_max(2).unwrap(); } - - #[test] - fn malloc_default_trim_does_not_panic() { - malloc_trim(OPTIMAL_TRIM).unwrap(); - } - - /// Unfortunately this test will print into the test results, even on success. I don't know any - /// way to avoid this. - #[test] - fn eprintln_malloc_stats_does_not_panic() { - eprintln_malloc_stats(); - } } diff --git a/common/malloc_utils/src/lib.rs b/common/malloc_utils/src/lib.rs index 446c0e012d8..3a0619eddff 100644 --- a/common/malloc_utils/src/lib.rs +++ b/common/malloc_utils/src/lib.rs @@ -27,7 +27,6 @@ pub use interface::*; #[cfg(all(target_os = "linux", not(target_env = "musl")))] mod interface { pub use crate::glibc::configure_glibc_malloc as configure_memory_allocator; - pub use crate::glibc::eprintln_malloc_stats as eprintln_allocator_stats; pub use crate::glibc::scrape_mallinfo_metrics as scrape_allocator_metrics; } @@ -38,9 +37,6 @@ mod interface { Ok(()) } - #[allow(dead_code)] - pub fn eprintln_allocator_stats() {} - #[allow(dead_code)] pub fn scrape_allocator_metrics() {} } From 00a80f94df38b1a2ea589698121eda576884bb40 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 10:13:54 +1000 Subject: [PATCH 30/40] Tidy --- common/malloc_utils/src/glibc.rs | 22 ++++++++++------------ common/malloc_utils/src/lib.rs | 9 +++++++-- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 5b46067e8b0..701a2b5d1dc 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -17,10 +17,7 @@ const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// The maximum number of arenas allowed to be created by malloc. /// -/// When set to `Some(0)`, the limit on the number of arenas is determined by the number of CPU cores -/// online. -/// -/// When set to `None`, the parameter is not set using this program (i.e., it is left as default). +/// See `ArenaMaxSetting` docs for details. const OPTIMAL_ARENA_MAX: ArenaMaxSetting = ArenaMaxSetting::NumCpus; /// Constants used to configure malloc internals. @@ -111,15 +108,16 @@ pub fn scrape_mallinfo_metrics() { set_gauge(&MALLINFO_KEEPCOST, mallinfo.keepcost as i64); } +/// Perform all configuration routines. pub fn configure_glibc_malloc() -> Result<(), String> { - let arena_max = match OPTIMAL_ARENA_MAX { - ArenaMaxSetting::DoNotSet => None, - ArenaMaxSetting::Fixed(n) => Some(n), - ArenaMaxSetting::NumCpus => Some(num_cpus::get() as c_int), - }; - - if let Some(max) = arena_max { - if !env_var_present(ENV_VAR_ARENA_MAX) { + if !env_var_present(ENV_VAR_ARENA_MAX) { + let arena_max = match OPTIMAL_ARENA_MAX { + ArenaMaxSetting::DoNotSet => None, + ArenaMaxSetting::Fixed(n) => Some(n), + ArenaMaxSetting::NumCpus => Some(num_cpus::get() as c_int), + }; + + if let Some(max) = arena_max { if let Err(e) = malloc_arena_max(max) { return Err(format!("failed (code {}) to set malloc max arena count", e)); } diff --git a/common/malloc_utils/src/lib.rs b/common/malloc_utils/src/lib.rs index 3a0619eddff..b8aed948f8b 100644 --- a/common/malloc_utils/src/lib.rs +++ b/common/malloc_utils/src/lib.rs @@ -5,13 +5,18 @@ //! Presently, only configuration for "The GNU Allocator" from `glibc` is supported. All other //! allocators are ignored. //! -//! It is assumed that if the following two statements are correct, then we should expect to +//! It is assumed that if the following two statements are correct then we should expect to //! configure `glibc`: //! //! - `target_os = linux` //! - `target_env != musl` //! -//! In all other cases this library will not attempt to do anything (i.e., all functions are no-ops). +//! In all other cases this library will not attempt to do anything (i.e., all functions are +//! no-ops). +//! +//! If the above conditions are fulfilled but `glibc` still isn't present at runtime then a panic +//! may be triggered. It is understood that there's no way to be certain that a compatible `glibc` +//! is present: https://github.com/rust-lang/rust/issues/33244. //! //! ## Notes //! From 268dc4ee282bcc60e046b6e42d74e8fe91f23c9f Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 10:15:25 +1000 Subject: [PATCH 31/40] Add malloc threshold test --- common/malloc_utils/src/glibc.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 701a2b5d1dc..a6dee416ec3 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -186,4 +186,9 @@ mod tests { fn malloc_arena_max_does_not_panic() { malloc_arena_max(2).unwrap(); } + + #[test] + fn malloc_mmap_threshold_does_not_panic() { + malloc_mmap_threshold(OPTIMAL_MMAP_THRESHOLD).unwrap(); + } } From 4a90729f0acd7535bdc98181eed7908a041c6182 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 10:16:20 +1000 Subject: [PATCH 32/40] Revery changes to bitfield --- consensus/ssz_types/Cargo.toml | 1 - consensus/ssz_types/src/bitfield.rs | 21 ++++++++------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/consensus/ssz_types/Cargo.toml b/consensus/ssz_types/Cargo.toml index 8ed6ee8e027..39d085b66ed 100644 --- a/consensus/ssz_types/Cargo.toml +++ b/consensus/ssz_types/Cargo.toml @@ -15,7 +15,6 @@ serde_utils = { path = "../serde_utils" } eth2_ssz = "0.1.2" typenum = "1.12.0" arbitrary = { version = "0.4.6", features = ["derive"], optional = true } -smallvec = "1.6.1" [dev-dependencies] serde_json = "1.0.58" diff --git a/consensus/ssz_types/src/bitfield.rs b/consensus/ssz_types/src/bitfield.rs index 6489880a7a8..89f1272a3d3 100644 --- a/consensus/ssz_types/src/bitfield.rs +++ b/consensus/ssz_types/src/bitfield.rs @@ -4,13 +4,10 @@ use core::marker::PhantomData; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; use serde_utils::hex::{encode as hex_encode, PrefixedHexVisitor}; -use smallvec::{smallvec, SmallVec}; use ssz::{Decode, Encode}; use tree_hash::Hash256; use typenum::Unsigned; -const SMALLVEC_LEN: usize = 32; - /// A marker trait applied to `Variable` and `Fixed` that defines the behaviour of a `Bitfield`. pub trait BitfieldBehaviour: Clone {} @@ -92,7 +89,7 @@ pub type BitVector = Bitfield>; /// bit-index. E.g., `vec![0b0000_0001, 0b0000_0010]` has bits `0, 9` set. #[derive(Clone, Debug, PartialEq)] pub struct Bitfield { - bytes: SmallVec<[u8; 32]>, + bytes: Vec, len: usize, _phantom: PhantomData, } @@ -107,7 +104,7 @@ impl Bitfield> { pub fn with_capacity(num_bits: usize) -> Result { if num_bits <= N::to_usize() { Ok(Self { - bytes: smallvec![0; bytes_for_bit_len(num_bits)], + bytes: vec![0; bytes_for_bit_len(num_bits)], len: num_bits, _phantom: PhantomData, }) @@ -145,8 +142,8 @@ impl Bitfield> { bytes.resize(bytes_for_bit_len(len + 1), 0); - let mut bitfield: Bitfield> = - Bitfield::from_raw_bytes(bytes.into_vec(), len + 1).unwrap_or_else(|_| { + let mut bitfield: Bitfield> = Bitfield::from_raw_bytes(bytes, len + 1) + .unwrap_or_else(|_| { unreachable!( "Bitfield with {} bytes must have enough capacity for {} bits.", bytes_for_bit_len(len + 1), @@ -157,7 +154,7 @@ impl Bitfield> { .set(len, true) .expect("len must be in bounds for bitfield."); - bitfield.bytes.into_vec() + bitfield.bytes } /// Instantiates a new instance from `bytes`. Consumes the same format that `self.into_bytes()` @@ -236,7 +233,7 @@ impl Bitfield> { /// All bits are initialized to `false`. pub fn new() -> Self { Self { - bytes: smallvec![0; bytes_for_bit_len(Self::capacity())], + bytes: vec![0; bytes_for_bit_len(Self::capacity())], len: Self::capacity(), _phantom: PhantomData, } @@ -331,7 +328,7 @@ impl Bitfield { /// Returns the underlying bytes representation of the bitfield. pub fn into_raw_bytes(self) -> Vec { - self.bytes.into_vec() + self.bytes } /// Returns a view into the underlying bytes representation of the bitfield. @@ -348,10 +345,8 @@ impl Bitfield { /// - `bit_len` is not a multiple of 8 and `bytes` contains set bits that are higher than, or /// equal to `bit_len`. fn from_raw_bytes(bytes: Vec, bit_len: usize) -> Result { - let bytes: SmallVec<[u8; SMALLVEC_LEN]> = bytes.into(); - if bit_len == 0 { - if bytes.len() == 1 && bytes.as_slice() == [0] { + if bytes.len() == 1 && bytes == [0] { // A bitfield with `bit_len` 0 can only be represented by a single zero byte. Ok(Self { bytes, From 7d87746158cf65e1b1286d8ae4703fd5d314c746 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 10:33:36 +1000 Subject: [PATCH 33/40] Freshen Cargo.lock --- Cargo.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 24257c76575..708fe54bc8a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2112,7 +2112,6 @@ dependencies = [ "serde_derive", "serde_json", "serde_utils", - "smallvec", "tree_hash", "tree_hash_derive", "typenum", From a1ca131824f2b7f1b45abfc514987fdf2b33b1cd Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 10:42:28 +1000 Subject: [PATCH 34/40] Add CLI tests --- lighthouse/tests/beacon_node.rs | 8 ++++++++ lighthouse/tests/validator_client.rs | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index d21a4845cd7..d81aca42e59 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -800,6 +800,14 @@ fn slasher_broadcast_flag() { }); } #[test] +pub fn malloc_tuning_flag() { + CommandLineTest::new() + .flag("disable-malloc-tuning", None) + // Simply ensure that the node can start with this flag, it's very difficult to observe the + // effects of it. + .run(); +} +#[test] #[should_panic] fn ensure_panic_on_failed_launch() { CommandLineTest::new() diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 435c45dd133..a0982064b76 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -344,3 +344,11 @@ fn metrics_allow_origin_all_flag() { .run() .with_config(|config| assert_eq!(config.http_metrics.allow_origin, Some("*".to_string()))); } +#[test] +pub fn malloc_tuning_flag() { + CommandLineTest::new() + .flag("disable-malloc-tuning", None) + // Simply ensure that the node can start with this flag, it's very difficult to observe the + // effects of it. + .run(); +} From 88306c653a2351ab40136e3fb27bd90fe1196b5e Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 14:00:34 +1000 Subject: [PATCH 35/40] Add comment about malloc2 --- common/malloc_utils/src/glibc.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index a6dee416ec3..507bff9426d 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -92,6 +92,9 @@ pub fn scrape_mallinfo_metrics() { // The docs for this function say it is thread-unsafe since it may return inconsistent results. // Since these are just metrics it's not a concern to us if they're sometimes inconsistent. // + // There exists a `malloc2` function, however it was release in February 2021 and this seems too + // recent to rely on. + // // Docs: // // https://man7.org/linux/man-pages/man3/mallinfo.3.html From fca7488d1d4f184378cef245301840b5cbf40ea9 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 14:12:45 +1000 Subject: [PATCH 36/40] Use libc::mallopt, protect against concurrency --- common/malloc_utils/src/glibc.rs | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 507bff9426d..b4fe0f62bc6 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -6,6 +6,7 @@ //! These functions are generally only suitable for Linux systems. use lazy_static::lazy_static; use lighthouse_metrics::*; +use parking_lot::Mutex; use std::env; use std::os::raw::c_int; use std::result::Result; @@ -46,6 +47,10 @@ enum ArenaMaxSetting { NumCpus, } +lazy_static! { + pub static ref GLOBAL_LOCK: Mutex<()> = <_>::default(); +} + // Metrics for the malloc. For more information, see: // // https://man7.org/linux/man-pages/man3/mallinfo.3.html @@ -98,7 +103,7 @@ pub fn scrape_mallinfo_metrics() { // Docs: // // https://man7.org/linux/man-pages/man3/mallinfo.3.html - let mallinfo = unsafe { libc::mallinfo() }; + let mallinfo = mallinfo(); set_gauge(&MALLINFO_ARENA, mallinfo.arena as i64); set_gauge(&MALLINFO_ORDBLKS, mallinfo.ordblks as i64); @@ -151,7 +156,7 @@ fn env_var_present(name: &str) -> bool { /// /// - https://man7.org/linux/man-pages/man3/mallopt.3.html fn malloc_arena_max(num_arenas: c_int) -> Result<(), c_int> { - unsafe { into_result(ffi::mallopt(M_ARENA_MAX, num_arenas)) } + into_result(mallopt(M_ARENA_MAX, num_arenas)) } /// Uses `mallopt` to set the `M_MMAP_THRESHOLD` value, specifying the threshold where objects of this @@ -161,16 +166,19 @@ fn malloc_arena_max(num_arenas: c_int) -> Result<(), c_int> { /// /// - https://man7.org/linux/man-pages/man3/mallopt.3.html fn malloc_mmap_threshold(num_arenas: c_int) -> Result<(), c_int> { - unsafe { into_result(ffi::mallopt(M_MMAP_THRESHOLD, num_arenas)) } + into_result(mallopt(M_MMAP_THRESHOLD, num_arenas)) } -mod ffi { - extern "C" { - pub fn mallopt( - __param: ::std::os::raw::c_int, - __val: ::std::os::raw::c_int, - ) -> ::std::os::raw::c_int; - } +fn mallopt(param: c_int, val: c_int) -> c_int { + // Prevent this function from being called in parallel with any other non-thread-safe function. + let _lock = GLOBAL_LOCK.lock(); + unsafe { libc::mallopt(param, val) } +} + +fn mallinfo() -> libc::mallinfo { + // Prevent this function from being called in parallel with any other non-thread-safe function. + let _lock = GLOBAL_LOCK.lock(); + unsafe { libc::mallinfo() } } fn into_result(result: c_int) -> Result<(), c_int> { From ddcdda9ac05ddb2574e161b893f82925d5003841 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 14:32:15 +1000 Subject: [PATCH 37/40] Gate malloc metrics behind CLI flag --- beacon_node/http_metrics/src/lib.rs | 2 ++ beacon_node/http_metrics/src/metrics.rs | 6 +++++- beacon_node/src/config.rs | 7 ++++++- common/clap_utils/src/flags.rs | 3 +++ common/clap_utils/src/lib.rs | 2 ++ lighthouse/src/main.rs | 12 +++++------- lighthouse/tests/beacon_node.rs | 7 ++++--- 7 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 common/clap_utils/src/flags.rs diff --git a/beacon_node/http_metrics/src/lib.rs b/beacon_node/http_metrics/src/lib.rs index 4837d513b55..66c7a6a6f69 100644 --- a/beacon_node/http_metrics/src/lib.rs +++ b/beacon_node/http_metrics/src/lib.rs @@ -49,6 +49,7 @@ pub struct Config { pub listen_addr: Ipv4Addr, pub listen_port: u16, pub allow_origin: Option, + pub allocator_metrics_enabled: bool, } impl Default for Config { @@ -58,6 +59,7 @@ impl Default for Config { listen_addr: Ipv4Addr::new(127, 0, 0, 1), listen_port: 5054, allow_origin: None, + allocator_metrics_enabled: true, } } } diff --git a/beacon_node/http_metrics/src/metrics.rs b/beacon_node/http_metrics/src/metrics.rs index a72a03690f8..4a870c889f8 100644 --- a/beacon_node/http_metrics/src/metrics.rs +++ b/beacon_node/http_metrics/src/metrics.rs @@ -42,7 +42,11 @@ pub fn gather_prometheus_metrics( warp_utils::metrics::scrape_health_metrics(); - scrape_allocator_metrics(); + // It's important to ensure these metrics are explicitly enabled in the case that users aren't + // using glibc and this function causes panics. + if ctx.config.allocator_metrics_enabled { + scrape_allocator_metrics(); + } encoder .encode(&lighthouse_metrics::gather(), &mut buffer) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 0550e8e0850..05c6f03c366 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -1,5 +1,5 @@ use clap::ArgMatches; -use clap_utils::BAD_TESTNET_DIR_MESSAGE; +use clap_utils::{flags::DISABLE_MALLOC_TUNING_FLAG, BAD_TESTNET_DIR_MESSAGE}; use client::{ClientConfig, ClientGenesis}; use directory::{DEFAULT_BEACON_NODE_DIR, DEFAULT_NETWORK_DIR, DEFAULT_ROOT_DIR}; use eth2_libp2p::{multiaddr::Protocol, Enr, Multiaddr, NetworkConfig, PeerIdSerialized}; @@ -145,6 +145,11 @@ pub fn get_config( ); } + // Do not scrape for malloc metrics if we've disabled tuning malloc as it may cause panics. + if cli_args.is_present(DISABLE_MALLOC_TUNING_FLAG) { + client_config.http_metrics.allocator_metrics_enabled = false; + } + /* * Eth1 */ diff --git a/common/clap_utils/src/flags.rs b/common/clap_utils/src/flags.rs new file mode 100644 index 00000000000..8ae0cf03261 --- /dev/null +++ b/common/clap_utils/src/flags.rs @@ -0,0 +1,3 @@ +//! CLI flags used across the Lighthouse code base can be located here. + +pub const DISABLE_MALLOC_TUNING_FLAG: &str = "disable-malloc-tuning"; diff --git a/common/clap_utils/src/lib.rs b/common/clap_utils/src/lib.rs index 2d43a1b28f9..dc82cbe6697 100644 --- a/common/clap_utils/src/lib.rs +++ b/common/clap_utils/src/lib.rs @@ -6,6 +6,8 @@ use ssz::Decode; use std::path::PathBuf; use std::str::FromStr; +pub mod flags; + pub const BAD_TESTNET_DIR_MESSAGE: &str = "The hard-coded testnet directory was invalid. \ This happens when Lighthouse is migrating between spec versions \ or when there is no default public network to connect to. \ diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 104e28732a0..fd299e04e16 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -2,6 +2,7 @@ mod metrics; use beacon_node::{get_eth2_network_config, ProductionBeaconNode}; use clap::{App, Arg, ArgMatches}; +use clap_utils::flags::DISABLE_MALLOC_TUNING_FLAG; use env_logger::{Builder, Env}; use environment::EnvironmentBuilder; use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK}; @@ -17,9 +18,6 @@ use validator_client::ProductionValidatorClient; pub const ETH2_CONFIG_FILENAME: &str = "eth2-spec.toml"; -// CLI flags: -pub const DISABLE_MALLOC_TUNING: &str = "disable-malloc-tuning"; - fn bls_library_name() -> &'static str { if cfg!(feature = "portable") { "blst-portable" @@ -149,8 +147,8 @@ fn main() { .global(true) ) .arg( - Arg::with_name(DISABLE_MALLOC_TUNING) - .long(DISABLE_MALLOC_TUNING) + Arg::with_name(DISABLE_MALLOC_TUNING_FLAG) + .long(DISABLE_MALLOC_TUNING_FLAG) .help( "If present, do not configure the system allocator. Providing this flag will \ generally increase memory usage, it should only be provided when debugging \ @@ -167,12 +165,12 @@ fn main() { // Configure the allocator early in the process, before it has the chance to use the default values for // anything important. - if !matches.is_present(DISABLE_MALLOC_TUNING) { + if !matches.is_present(DISABLE_MALLOC_TUNING_FLAG) { if let Err(e) = configure_memory_allocator() { eprintln!( "Unable to configure the memory allocator: {} \n\ Try providing the --{} flag", - e, DISABLE_MALLOC_TUNING + e, DISABLE_MALLOC_TUNING_FLAG ); exit(1) } diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index d81aca42e59..249033377d1 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -803,9 +803,10 @@ fn slasher_broadcast_flag() { pub fn malloc_tuning_flag() { CommandLineTest::new() .flag("disable-malloc-tuning", None) - // Simply ensure that the node can start with this flag, it's very difficult to observe the - // effects of it. - .run(); + .run() + .with_config(|config| { + assert!(!config.http_metrics.allocator_metrics_enabled); + }); } #[test] #[should_panic] From cb07b740cd372efc2102e5f615b40a9ccab0bb89 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 14:36:33 +1000 Subject: [PATCH 38/40] Fix broken metrics tests --- beacon_node/http_metrics/tests/tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon_node/http_metrics/tests/tests.rs b/beacon_node/http_metrics/tests/tests.rs index db161c6d68b..633b81115f3 100644 --- a/beacon_node/http_metrics/tests/tests.rs +++ b/beacon_node/http_metrics/tests/tests.rs @@ -20,6 +20,7 @@ async fn returns_200_ok() { listen_addr: Ipv4Addr::new(127, 0, 0, 1), listen_port: 0, allow_origin: None, + allocator_metrics_enabled: true, }, chain: None, db_path: None, From 4a679e641fd20b2fbddf96fdf92df438c8430cfe Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 14:50:16 +1000 Subject: [PATCH 39/40] Drop mmap threshold to 1mb --- common/malloc_utils/src/glibc.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index b4fe0f62bc6..905a0567edd 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -13,8 +13,12 @@ use std::result::Result; /// The value to be provided to `malloc_mmap_threshold`. /// -/// Value chosen so that it will store the values of the validators tree hash cache. -const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; +/// Value chosen so that values of the validators tree hash cache will be allocated via `mmap`. +/// +/// The size of a single chunk is: +/// +/// NODES_PER_VALIDATOR * VALIDATORS_PER_ARENA * 32 = 15 * 4096 * 32 = 1.875 MiB +const OPTIMAL_MMAP_THRESHOLD: c_int = 1 * 1_024 * 1_024; /// The maximum number of arenas allowed to be created by malloc. /// From eea4186571506d790e56e7c0dc388486ebdbd150 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Fri, 28 May 2021 15:36:30 +1000 Subject: [PATCH 40/40] Set mmap threshold back to 2mb --- common/malloc_utils/src/glibc.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs index 905a0567edd..9cefc644151 100644 --- a/common/malloc_utils/src/glibc.rs +++ b/common/malloc_utils/src/glibc.rs @@ -13,12 +13,13 @@ use std::result::Result; /// The value to be provided to `malloc_mmap_threshold`. /// -/// Value chosen so that values of the validators tree hash cache will be allocated via `mmap`. +/// Value chosen so that values of the validators tree hash cache will *not* be allocated via +/// `mmap`. /// /// The size of a single chunk is: /// /// NODES_PER_VALIDATOR * VALIDATORS_PER_ARENA * 32 = 15 * 4096 * 32 = 1.875 MiB -const OPTIMAL_MMAP_THRESHOLD: c_int = 1 * 1_024 * 1_024; +const OPTIMAL_MMAP_THRESHOLD: c_int = 2 * 1_024 * 1_024; /// The maximum number of arenas allowed to be created by malloc. ///