From 8218248000d0cc3099e7f62b907f4c2b7d58f02d Mon Sep 17 00:00:00 2001 From: houseme Date: Sat, 1 Aug 2026 15:04:54 +0800 Subject: [PATCH] fix(hotpath): pin mimalloc allocator backend (#5550) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(hotpath): pin mimalloc allocator backend * test(hotpath): verify mimalloc allocator backend Co-Authored-By: heihutu * chore(hotpath): document unsafe allocator tests Co-Authored-By: heihutu * feat(kms): record real cache hit, miss and eviction metrics (#5531) * feat(kms): record real cache hit, miss and eviction metrics The metadata cache reported (entry_count, 0) because moka exposes no hit or miss counts, so the miss half of every cache report was a constant. Track lookups and removals in the cache itself: hit/miss counters on the lookup path, a moka eviction listener classifying removals by cause, and an entry gauge refreshed whenever the entry set changes. The counters are exported through the metrics facade under the rustfs_kms_ prefix with static label values only, matching the operation-policy metrics, and are also returned as a KmsCacheStats snapshot in place of the old tuple. Cache semantics are unchanged: capacity, TTL and invalidation points are the same, and remove now flushes pending maintenance so the gauge and the removal notification describe the cache the caller sees. Refs rustfs/backlog#1584 * fix(kms): report real cache counters through the admin status API KmsStatusResponse.cache_stats mapped the old (entry_count, 0) tuple onto hit_count and miss_count, so operators polling KMS status read the entry count as a hit count and a miss count that was always zero. Map the fields to the counters they claim to be, and add entry_count and eviction_count as additive, defaulted fields so the entry number that hit_count used to carry is still available. Refs rustfs/backlog#1584 * fix(kms): refresh the cache entry gauge on lookup misses The entry gauge was published only from the write paths, so an entry dropped by TTL expiry left `rustfs_kms_metadata_cache_entries` reporting a population that no longer existed until the next put, remove or clear. A cache that goes quiet — entries ageing out with no further writes — kept over-reporting indefinitely. Republish the gauge from the lookup path when the lookup misses. A miss is where expiry surfaces, and moka reaps expired entries in the maintenance it runs during that same lookup, so the count read afterwards reflects the reaping. Hits stay free of the extra work. * docs(kms): correct the entry gauge convergence claim on the miss path The comment on the miss-path gauge refresh said moka reaps expired entries in the maintenance it runs on that same lookup. It does not: `should_apply_reads` is gated on a full read log or an elapsed housekeeping interval, so the removal that decrements `entry_count` and reaches the eviction listener may land on a later lookup. The behaviour and the test are unchanged — the gauge still converges, and the test drives `run_pending_tasks` explicitly rather than riding on that interval. Only the stated guarantee was wrong, so say interval instead of same-lookup and record why forcing maintenance on the read path was not the trade taken. * chore(deps): refresh cargo dependencies Co-Authored-By: heihutu --------- Co-authored-by: heihutu Co-authored-by: Zhengchao An --- Cargo.lock | 58 ++++++++++++++++++++--------------------- Cargo.toml | 9 ++++--- deny.toml | 3 +++ rustfs/Cargo.toml | 2 +- rustfs/src/main.rs | 64 ++++++++++++++++++++++++++++++++++++++++------ 5 files changed, 93 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4614fdd0f..9d70a14e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -290,11 +290,11 @@ dependencies = [ [[package]] name = "ar_archive_writer" -version = "0.5.2" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4087686b4b0a3427190bae57a1d9a478dbb2d40c5dc1bd6e2b6d797913bdd348" +checksum = "73cd58deff2140a0a8eae87e417bd01db68a33e148aa93d1e8cd837e55e312b6" dependencies = [ - "object 0.37.3", + "object 0.39.1", ] [[package]] @@ -1589,7 +1589,7 @@ version = "0.10.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" dependencies = [ - "generic-array 0.14.7", + "generic-array 0.14.9", ] [[package]] @@ -1608,7 +1608,7 @@ version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8894febbff9f758034a5b8e12d87918f56dfc64a8e1fe757d65e29041538d93" dependencies = [ - "generic-array 0.14.7", + "generic-array 0.14.9", ] [[package]] @@ -1740,9 +1740,9 @@ dependencies = [ [[package]] name = "bytesize" -version = "2.4.2" +version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3d7c8918969267b2932ffd5655509bbbea0833823058c378876953217f5fc50e" +checksum = "351a3e803ee3c6eaeee6b00076b767514b37c32a73d326c3ec7abddb7d6c3493" [[package]] name = "bytestring" @@ -1950,7 +1950,7 @@ version = "0.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" dependencies = [ - "crypto-common 0.1.7", + "crypto-common 0.1.6", "inout 0.1.4", ] @@ -1968,9 +1968,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.6.4" +version = "4.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d91e0c145792ef73a6ad36d27c75ac09f1832222a3c209689d90f534685ee5b7" +checksum = "301b56658598e48f3648647ac6fc887be7e7108eddfa4e9b63fcf3ec58c0cadf" dependencies = [ "clap_builder", "clap_derive", @@ -1978,9 +1978,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.6.2" +version = "4.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f09628afdcc538b57f3c6341e9c8e9970f18e4a481690a64974d7023bd33548b" +checksum = "94a65403d1a1bd28f7dc68eb8506e8874808ee5eecb59298de588e2e1407a078" dependencies = [ "anstream", "anstyle", @@ -2408,7 +2408,7 @@ version = "0.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0dc92fb57ca44df6db8059111ab3af99a63d5d0f8375d9972e319a379c6bab76" dependencies = [ - "generic-array 0.14.7", + "generic-array 0.14.9", "rand_core 0.6.4", "subtle", "zeroize", @@ -2433,11 +2433,11 @@ dependencies = [ [[package]] name = "crypto-common" -version = "0.1.7" +version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" +checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" dependencies = [ - "generic-array 0.14.7", + "generic-array 0.14.9", "typenum", ] @@ -3634,7 +3634,7 @@ checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer 0.10.4", "const-oid 0.9.6", - "crypto-common 0.1.7", + "crypto-common 0.1.6", "subtle", ] @@ -3894,7 +3894,7 @@ dependencies = [ "crypto-bigint 0.5.5", "digest 0.10.7", "ff 0.13.1", - "generic-array 0.14.7", + "generic-array 0.14.9", "group 0.13.0", "hkdf 0.12.4", "pem-rfc7468 0.7.0", @@ -4339,9 +4339,9 @@ dependencies = [ [[package]] name = "generic-array" -version = "0.14.7" +version = "0.14.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" +checksum = "4bb6743198531e02858aeaea5398fcc883e71851fcbcb5a2f773e2fb6cb1edf2" dependencies = [ "typenum", "version_check", @@ -4354,7 +4354,7 @@ version = "1.4.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab4e5aa225bc56696909483320f0ff9b600f1a971b52e07a17d70f3d9b43254b" dependencies = [ - "generic-array 0.14.7", + "generic-array 0.14.9", "rustversion", "typenum", ] @@ -5381,7 +5381,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "879f10e63c20629ecabbb64a8010319738c66a5cd0c29b02d63d272b03751d01" dependencies = [ "block-padding 0.3.3", - "generic-array 0.14.7", + "generic-array 0.14.9", ] [[package]] @@ -5915,8 +5915,7 @@ checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981" [[package]] name = "libmimalloc-sys" version = "0.1.49" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a45a52f43e1c16f667ccfe4dd8c85b7f7c204fd5e3bf46c5b0db9a5c3c0b8e9" +source = "git+https://github.com/xonatius/mimalloc_rust.git?rev=1cdadea43e9c5a0f054b65be21200ce580e4eb13#1cdadea43e9c5a0f054b65be21200ce580e4eb13" dependencies = [ "cc", "cty", @@ -6325,8 +6324,7 @@ dependencies = [ [[package]] name = "mimalloc" version = "0.1.52" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d4139bb28d14ad1facf21d5eb8825051b326e172d216b39f6d31df53cc97862" +source = "git+https://github.com/xonatius/mimalloc_rust.git?rev=1cdadea43e9c5a0f054b65be21200ce580e4eb13#1cdadea43e9c5a0f054b65be21200ce580e4eb13" dependencies = [ "libmimalloc-sys", ] @@ -8824,9 +8822,9 @@ dependencies = [ [[package]] name = "russh" -version = "0.62.4" +version = "0.62.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8b67b5a0d8068c89dcbe9d95df986af7a851d1f3c604525274c37468e60464f" +checksum = "da7c230e0ed9cbeb92fbad6c8848985d6df2a1464c0dc247a021abd666e9005e" dependencies = [ "aes 0.9.2", "aws-lc-rs", @@ -10715,7 +10713,7 @@ checksum = "d3e97a565f76233a6003f9f5c54be1d9c5bdfa3eccfb189469f11ec4901c47dc" dependencies = [ "base16ct 0.2.0", "der 0.7.10", - "generic-array 0.14.7", + "generic-array 0.14.9", "pkcs8 0.10.2", "subtle", "zeroize", @@ -11711,7 +11709,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.4.3", + "getrandom 0.3.4", "once_cell", "rustix", "windows-sys 0.61.2", diff --git a/Cargo.toml b/Cargo.toml index 60f355b1b..9f66414d5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -173,7 +173,7 @@ tower-http = { version = "0.7.0" } # Serialization and Data Formats apache-avro = "0.21.0" bytes = { version = "1.12.1" } -bytesize = "2.4.2" +bytesize = "2.6.0" byteorder = "1.5.0" flatbuffers = "25.12.19" form_urlencoded = "1.2.2" @@ -236,7 +236,7 @@ aws-smithy-types = { version = "1.6.1" } base64 = "0.23.0" base64-simd = "0.8.0" brotli = "8.0.4" -clap = { version = "4.6.4" } +clap = { version = "4.6.5" } const-str = { version = "1.1.0" } convert_case = "0.11.0" criterion = { version = "0.8" } @@ -341,14 +341,15 @@ libunftp = { version = "0.23.0" } unftp-core = "0.1.0" suppaftp = { version = "10.0.1" } rcgen = { version = "0.14.8", default-features = false, features = ["aws_lc_rs", "crypto", "pem"] } -russh = { version = "0.62.4" } +russh = { version = "0.62.5" } russh-sftp = "2.3.0" # WebDAV dav-server = "0.11.0" # Performance Analysis and Memory Profiling -mimalloc = "0.1.52" +mimalloc = { version = "0.1.52", git = "https://github.com/xonatius/mimalloc_rust.git", rev = "1cdadea43e9c5a0f054b65be21200ce580e4eb13" } +libmimalloc-sys = { version = "0.1.49", git = "https://github.com/xonatius/mimalloc_rust.git", rev = "1cdadea43e9c5a0f054b65be21200ce580e4eb13", features = ["extended"] } hotpath = { version = "0.22.0", default-features = false } # Snapshot testing for output format regression detection insta = { version = "1.48" } diff --git a/deny.toml b/deny.toml index 91524f4cb..23c592ee5 100644 --- a/deny.toml +++ b/deny.toml @@ -43,6 +43,9 @@ allow-git = [ # Presigned expiry and constant-time authentication fixes pending upstream merge. # owner: cxymds review: 2026-10 "https://github.com/cxymds/s3s.git", + # MiMalloc fork pinned for hotpath allocation counting support. + # owner: houseme review: 2026-10 + "https://github.com/xonatius/mimalloc_rust.git", "https://github.com/apache/datafusion.git", ] diff --git a/rustfs/Cargo.toml b/rustfs/Cargo.toml index 24a18a938..40c15c9dc 100644 --- a/rustfs/Cargo.toml +++ b/rustfs/Cargo.toml @@ -330,7 +330,7 @@ mimalloc = { workspace = true } libsystemd.workspace = true [target.'cfg(not(target_os = "windows"))'.dependencies] -libmimalloc-sys = { version = "0.1.49", features = ["extended"] } +libmimalloc-sys.workspace = true [dev-dependencies] uuid = { workspace = true, features = ["v4", "fast-rng", "macro-diagnostics"] } diff --git a/rustfs/src/main.rs b/rustfs/src/main.rs index 75f9c81c4..d27e916bd 100644 --- a/rustfs/src/main.rs +++ b/rustfs/src/main.rs @@ -17,27 +17,37 @@ use std::alloc::{GlobalAlloc, Layout}; #[cfg(all(feature = "hotpath", feature = "hotpath-alloc"))] #[derive(Default)] -struct DefaultMiMalloc; +struct MiMallocAllocator; #[cfg(all(feature = "hotpath", feature = "hotpath-alloc"))] -// SAFETY: allocation and deallocation are forwarded unchanged to MiMalloc, so +// SAFETY: allocation operations are forwarded unchanged to MiMalloc, so // MiMalloc's GlobalAlloc guarantees apply to every returned pointer and layout. #[allow(unsafe_code)] -unsafe impl GlobalAlloc for DefaultMiMalloc { +unsafe impl GlobalAlloc for MiMallocAllocator { unsafe fn alloc(&self, layout: Layout) -> *mut u8 { // SAFETY: the caller upholds GlobalAlloc's contract for layout. unsafe { mimalloc::MiMalloc.alloc(layout) } } + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + // SAFETY: the caller upholds GlobalAlloc's contract for layout. + unsafe { mimalloc::MiMalloc.alloc_zeroed(layout) } + } + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { // SAFETY: ptr and layout came from this allocator and are forwarded unchanged. unsafe { mimalloc::MiMalloc.dealloc(ptr, layout) } } + + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + // SAFETY: ptr and layout came from this allocator and are forwarded unchanged. + unsafe { mimalloc::MiMalloc.realloc(ptr, layout, new_size) } + } } #[cfg(all(feature = "hotpath", feature = "hotpath-alloc"))] #[global_allocator] -static GLOBAL: hotpath::CountingAllocator = hotpath::CountingAllocator::new(); +static GLOBAL: hotpath::CountingAllocator = hotpath::CountingAllocator::new(); #[cfg(not(all(feature = "hotpath", feature = "hotpath-alloc")))] #[global_allocator] @@ -49,14 +59,52 @@ fn main() { rustfs::startup_entrypoint::run_process(); } -#[cfg(all(test, feature = "hotpath", feature = "hotpath-alloc"))] +#[cfg(all(test, feature = "hotpath", feature = "hotpath-alloc", not(target_os = "windows")))] mod tests { #[test] + // SAFETY: This test inspects a live allocation pointer with mimalloc's heap + // ownership API without dereferencing or extending the pointer lifetime. #[allow(unsafe_code)] - fn hotpath_allocator_uses_mimalloc() { - let allocation = Box::new([0_u8; 64]); + fn hotpath_allocation_workload_uses_mimalloc() { + let _guard = hotpath::MeasurementGuardSync::new("rustfs::tests::hotpath_allocation_workload_uses_mimalloc", false, false); + let mut allocation = Vec::with_capacity(64); + allocation.extend_from_slice(&[7_u8; 64]); - // SAFETY: the live Box pointer is valid to inspect for heap ownership. + assert_eq!(allocation.len(), 64); + // SAFETY: the live Vec pointer is valid to inspect for heap ownership. assert!(unsafe { libmimalloc_sys::mi_is_in_heap_region(allocation.as_ptr().cast()) }); } + + #[test] + // SAFETY: This test directly exercises the allocator wrapper and releases + // every successful allocation with the matching layout. + #[allow(unsafe_code)] + fn mimalloc_allocator_forwards_extended_global_alloc_operations() { + use std::alloc::{GlobalAlloc, Layout}; + + let layout = Layout::from_size_align(32, 8).expect("valid test allocation layout"); + let grown_layout = Layout::from_size_align(64, 8).expect("valid grown test allocation layout"); + let allocator = super::MiMallocAllocator; + + // SAFETY: The pointer is checked for null before use and later released + // through the same allocator with the corresponding layout. + let ptr = unsafe { allocator.alloc_zeroed(layout) }; + assert!(!ptr.is_null()); + assert!(unsafe { libmimalloc_sys::mi_is_in_heap_region(ptr.cast()) }); + assert!(unsafe { std::slice::from_raw_parts(ptr, 32).iter().all(|byte| *byte == 0) }); + + // SAFETY: `ptr` was allocated by `allocator` with `layout`; on failure + // the original allocation remains valid and is released below. + let grown_ptr = unsafe { allocator.realloc(ptr, layout, 64) }; + if grown_ptr.is_null() { + // SAFETY: `ptr` is still valid when realloc returns null. + unsafe { allocator.dealloc(ptr, layout) }; + panic!("mimalloc realloc failed in allocator smoke test"); + } + + assert!(unsafe { libmimalloc_sys::mi_is_in_heap_region(grown_ptr.cast()) }); + // SAFETY: `grown_ptr` was reallocated by `allocator` and is released + // with the matching grown layout. + unsafe { allocator.dealloc(grown_ptr, grown_layout) }; + } }