From 10dac51c6fc9466a920f10ceb195bec01e6b8d36 Mon Sep 17 00:00:00 2001
From: Michael Sproul <michael@sigmaprime.io>
Date: Wed, 15 Dec 2021 20:39:50 +0000
Subject: [PATCH] Enable `mallinfo2` behind feature flag (#2864)

## Proposed Changes

Add `mallinfo2` behind a feature flag so that we can get accurate memory metrics during debugging. It can be enabled when building Lighthouse like so (so long as the platform supports it):

```
cargo install --path lighthouse --features "malloc_utils/mallinfo2"
```
---
 common/malloc_utils/Cargo.toml   |  3 ++
 common/malloc_utils/src/glibc.rs | 48 +++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/common/malloc_utils/Cargo.toml b/common/malloc_utils/Cargo.toml
index 685c52421..813584992 100644
--- a/common/malloc_utils/Cargo.toml
+++ b/common/malloc_utils/Cargo.toml
@@ -11,3 +11,6 @@ lighthouse_metrics = { path = "../lighthouse_metrics" }
 lazy_static = "1.4.0"
 libc = "0.2.79"
 parking_lot = "0.11.0"
+
+[features]
+mallinfo2 = []
diff --git a/common/malloc_utils/src/glibc.rs b/common/malloc_utils/src/glibc.rs
index f65c933dd..402cdc27a 100644
--- a/common/malloc_utils/src/glibc.rs
+++ b/common/malloc_utils/src/glibc.rs
@@ -82,27 +82,8 @@ lazy_static! {
 
 /// 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.
-    //
-    // There exists a `mallinfo2` function, however it was released in February 2021 and this seems
-    // too recent to rely on.
-    //
-    // Docs:
-    //
-    // https://man7.org/linux/man-pages/man3/mallinfo.3.html
     let mallinfo = mallinfo();
 
-    /// Cast a C integer as returned by `mallinfo` to an unsigned i64.
-    ///
-    /// A cast from `i32` to `i64` preserves the sign bit, resulting in incorrect negative values.
-    /// Going via `u32` treats the sign bit as part of the number.
-    ///
-    /// Results are still wrong for memory usage over 4GiB due to limitations of mallinfo.
-    fn unsigned_i64(x: i32) -> i64 {
-        x as u32 as i64
-    }
-
     set_gauge(&MALLINFO_ARENA, unsigned_i64(mallinfo.arena));
     set_gauge(&MALLINFO_ORDBLKS, unsigned_i64(mallinfo.ordblks));
     set_gauge(&MALLINFO_SMBLKS, unsigned_i64(mallinfo.smblks));
@@ -114,6 +95,23 @@ pub fn scrape_mallinfo_metrics() {
     set_gauge(&MALLINFO_KEEPCOST, unsigned_i64(mallinfo.keepcost));
 }
 
+/// Cast a C integer as returned by `mallinfo` to an unsigned i64.
+///
+/// A cast from `i32` to `i64` preserves the sign bit, resulting in incorrect negative values.
+/// Going via `u32` treats the sign bit as part of the number.
+///
+/// Results are still wrong for memory usage over 4GiB due to limitations of mallinfo.
+#[cfg(not(feature = "mallinfo2"))]
+fn unsigned_i64(x: i32) -> i64 {
+    x as u32 as i64
+}
+
+/// Cast a C `size_t` as returned by `mallinfo2` to an unsigned i64.
+#[cfg(feature = "mallinfo2")]
+fn unsigned_i64(x: usize) -> i64 {
+    x as i64
+}
+
 /// Perform all configuration routines.
 pub fn configure_glibc_malloc() -> Result<(), String> {
     if !env_var_present(ENV_VAR_MMAP_THRESHOLD) {
@@ -146,12 +144,24 @@ fn mallopt(param: c_int, val: c_int) -> c_int {
     unsafe { libc::mallopt(param, val) }
 }
 
+/// By default we use `mallinfo`, but it overflows, so `mallinfo2` should be enabled if available.
+///
+/// https://man7.org/linux/man-pages/man3/mallinfo.3.html
+#[cfg(not(feature = "mallinfo2"))]
 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() }
 }
 
+/// Use `mallinfo2` if enabled.
+#[cfg(feature = "mallinfo2")]
+fn mallinfo() -> libc::mallinfo2 {
+    // Prevent this function from being called in parallel with any other non-thread-safe function.
+    let _lock = GLOBAL_LOCK.lock();
+    unsafe { libc::mallinfo2() }
+}
+
 fn into_result(result: c_int) -> Result<(), c_int> {
     if result == 1 {
         Ok(())