From 16dbcda2816a8d9ad1a3b453caa4cf4089c87ec8 Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Tue, 2 Jun 2026 18:48:12 +0800 Subject: [PATCH 1/2] fix: Inaccurate GetSize impl for a few types --- src/blocks/header.rs | 30 ++++++---- src/blocks/tipset.rs | 14 ++++- src/blocks/vrf_proof.rs | 4 +- src/cid_collections/small_cid_vec.rs | 4 +- src/rpc/methods/eth.rs | 22 ++------ src/rpc/methods/eth/types.rs | 21 +++---- src/shim/address.rs | 7 +-- src/shim/econ.rs | 4 +- src/shim/executor.rs | 10 ++-- src/shim/sector.rs | 4 +- src/shim/state_tree.rs | 4 +- src/state_manager/mod.rs | 20 ++++--- src/utils/get_size/mod.rs | 82 +++++++++++++++++++++++----- 13 files changed, 141 insertions(+), 85 deletions(-) diff --git a/src/blocks/header.rs b/src/blocks/header.rs index d5bd647bb8e7..99de34385d09 100644 --- a/src/blocks/header.rs +++ b/src/blocks/header.rs @@ -201,7 +201,10 @@ impl RawBlockHeader { // The derive macro does not compile for some reason impl GetSize for RawBlockHeader { - fn get_heap_size(&self) -> usize { + fn get_heap_size_with_tracker( + &self, + mut tracker: T, + ) -> (usize, T) { let Self { miner_address, ticket, @@ -220,16 +223,21 @@ impl GetSize for RawBlockHeader { fork_signal: _, parent_base_fee, } = self; - miner_address.get_heap_size() - + ticket.get_heap_size() - + election_proof.get_heap_size() - + beacon_entries.get_heap_size() - + winning_post_proof.get_heap_size() - + parents.get_heap_size() - + big_int_heap_size_helper(weight) - + bls_aggregate.get_heap_size() - + signature.get_heap_size() - + parent_base_fee.get_heap_size() + ( + miner_address.get_heap_size_with_tracker(&mut tracker).0 + + ticket.get_heap_size_with_tracker(&mut tracker).0 + + election_proof.get_heap_size_with_tracker(&mut tracker).0 + + beacon_entries.get_heap_size_with_tracker(&mut tracker).0 + + winning_post_proof + .get_heap_size_with_tracker(&mut tracker) + .0 + + parents.get_heap_size_with_tracker(&mut tracker).0 + + big_int_heap_size_helper(weight) + + bls_aggregate.get_heap_size_with_tracker(&mut tracker).0 + + signature.get_heap_size_with_tracker(&mut tracker).0 + + parent_base_fee.get_heap_size_with_tracker(&mut tracker).0, + tracker, + ) } } diff --git a/src/blocks/tipset.rs b/src/blocks/tipset.rs index 76fadd9c848a..8b8a2457b9fb 100644 --- a/src/blocks/tipset.rs +++ b/src/blocks/tipset.rs @@ -167,10 +167,9 @@ impl Default for TipsetKey { /// /// Represents non-null tipsets, see the documentation on [`crate::state_manager::apply_block_messages`] /// for more. -#[derive(Clone, Debug, GetSize)] +#[derive(Clone, Debug)] pub struct Tipset { /// Sorted - #[get_size(size_fn = nunny_vec_heap_size_helper)] headers: Arc>, // key is lazily initialized via `fn key()`. key: Arc>, @@ -185,6 +184,17 @@ impl ShallowClone for Tipset { } } +impl get_size2::GetSize for Tipset { + fn get_heap_size_with_tracker( + &self, + mut tracker: T, + ) -> (usize, T) { + let heap_size = nunny_vec_heap_size_helper(&self.headers, &mut tracker).0 + + self.key.get_heap_size_with_tracker(&mut tracker).0; + (heap_size, tracker) + } +} + impl From for Tipset { fn from(value: RawBlockHeader) -> Self { Self::from(CachingBlockHeader::from(value)) diff --git a/src/blocks/vrf_proof.rs b/src/blocks/vrf_proof.rs index fbb2549d0836..d92f8aacb85b 100644 --- a/src/blocks/vrf_proof.rs +++ b/src/blocks/vrf_proof.rs @@ -27,7 +27,7 @@ impl VRFProof { } impl GetSize for VRFProof { - fn get_heap_size(&self) -> usize { - self.0.get_heap_size() + fn get_heap_size_with_tracker(&self, tracker: T) -> (usize, T) { + self.0.get_heap_size_with_tracker(tracker) } } diff --git a/src/cid_collections/small_cid_vec.rs b/src/cid_collections/small_cid_vec.rs index 8375e8d87925..282261920cf9 100644 --- a/src/cid_collections/small_cid_vec.rs +++ b/src/cid_collections/small_cid_vec.rs @@ -23,8 +23,8 @@ use crate::blocks::TipsetKey; pub struct SmallCidNonEmptyVec(NonEmpty); impl GetSize for SmallCidNonEmptyVec { - fn get_heap_size(&self) -> usize { - nunny_vec_heap_size_helper(&self.0) + fn get_heap_size_with_tracker(&self, tracker: T) -> (usize, T) { + nunny_vec_heap_size_helper(&self.0, tracker) } } diff --git a/src/rpc/methods/eth.rs b/src/rpc/methods/eth.rs index 9de7441c4ca7..0e319b7e36da 100644 --- a/src/rpc/methods/eth.rs +++ b/src/rpc/methods/eth.rs @@ -137,8 +137,8 @@ pub struct EthBigInt( lotus_json_with_self!(EthBigInt); impl GetSize for EthBigInt { - fn get_heap_size(&self) -> usize { - big_int_heap_size_helper(&self.0) + fn get_heap_size_with_tracker(&self, tracker: T) -> (usize, T) { + (big_int_heap_size_helper(&self.0), tracker) } } @@ -156,34 +156,24 @@ impl From<&TokenAmount> for EthBigInt { type GasPriceResult = EthBigInt; -#[derive(PartialEq, Debug, Deserialize, Serialize, Default, Clone, JsonSchema)] +#[derive(PartialEq, Debug, Deserialize, Serialize, Default, Clone, JsonSchema, GetSize)] pub struct Nonce( #[schemars(with = "String")] #[serde(with = "crate::lotus_json::hexify_bytes")] + #[get_size(ignore)] pub ethereum_types::H64, ); lotus_json_with_self!(Nonce); -impl GetSize for Nonce { - fn get_heap_size(&self) -> usize { - 0 - } -} - -#[derive(PartialEq, Debug, Deserialize, Serialize, Default, Clone, JsonSchema)] +#[derive(PartialEq, Debug, Deserialize, Serialize, Default, Clone, JsonSchema, GetSize)] pub struct Bloom( #[schemars(with = "String")] #[serde(with = "crate::lotus_json::hexify_bytes")] + #[get_size(ignore)] pub ethereum_types::Bloom, ); lotus_json_with_self!(Bloom); -impl GetSize for Bloom { - fn get_heap_size(&self) -> usize { - 0 - } -} - impl Bloom { pub fn accrue(&mut self, input: &[u8]) { self.0.accrue(ethereum_types::BloomInput::Raw(input)); diff --git a/src/rpc/methods/eth/types.rs b/src/rpc/methods/eth/types.rs index 8a96380a81d8..df380c1d9c9a 100644 --- a/src/rpc/methods/eth/types.rs +++ b/src/rpc/methods/eth/types.rs @@ -116,20 +116,16 @@ impl GetStorageAtParams { derive_more::From, derive_more::Into, derive_more::FromStr, + GetSize, )] pub struct EthAddress( #[schemars(with = "String")] #[serde(with = "crate::lotus_json::hexify_bytes")] + #[get_size(ignore)] pub ethereum_types::Address, ); lotus_json_with_self!(EthAddress); -impl GetSize for EthAddress { - fn get_heap_size(&self) -> usize { - 0 - } -} - impl EthAddress { pub fn to_filecoin_address(self) -> anyhow::Result { if self.is_masked_id() { @@ -436,17 +432,16 @@ impl TryFrom for Message { derive_more::From, derive_more::Into, derive_more::FromStr, + GetSize, )] #[display("{_0:#x}")] -pub struct EthHash(#[schemars(with = "String")] pub ethereum_types::H256); +pub struct EthHash( + #[schemars(with = "String")] + #[get_size(ignore)] + pub ethereum_types::H256, +); lotus_json_with_self!(EthHash); -impl GetSize for EthHash { - fn get_heap_size(&self) -> usize { - 0 - } -} - #[derive(Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, Hash, Clone)] pub struct FilterID(EthHash); diff --git a/src/shim/address.rs b/src/shim/address.rs index 82cc7033007e..18799c70f2d4 100644 --- a/src/shim/address.rs +++ b/src/shim/address.rs @@ -260,11 +260,8 @@ impl Display for Address { } } -impl GetSize for Address { - fn get_heap_size(&self) -> usize { - 0 // all variants of the internal payload are stack-allocated - } -} +// all variants of the internal payload are stack-allocated +impl GetSize for Address {} /// A Filecoin address is an identifier that refers to an actor in the Filecoin state. All actors /// (miner actors, the storage market actor, account actors) have an address. This address encodes diff --git a/src/shim/econ.rs b/src/shim/econ.rs index 1b5dd6d03999..64132860c1cd 100644 --- a/src/shim/econ.rs +++ b/src/shim/econ.rs @@ -45,8 +45,8 @@ pub static TOTAL_FILECOIN: LazyLock = pub struct TokenAmount(TokenAmount_latest); impl GetSize for TokenAmount { - fn get_heap_size(&self) -> usize { - big_int_heap_size_helper(self.0.atto()) + fn get_heap_size_with_tracker(&self, tracker: T) -> (usize, T) { + (big_int_heap_size_helper(self.0.atto()), tracker) } } diff --git a/src/shim/executor.rs b/src/shim/executor.rs index af7ad51a672a..af8466fd2610 100644 --- a/src/shim/executor.rs +++ b/src/shim/executor.rs @@ -88,8 +88,8 @@ pub enum Receipt { } impl GetSize for Receipt { - fn get_heap_size(&self) -> usize { - delegate_receipt!(self.return_data.bytes().get_heap_size()) + fn get_heap_size_with_tracker(&self, tracker: T) -> (usize, T) { + delegate_receipt!(self.return_data.bytes().get_heap_size_with_tracker(tracker)) } } @@ -227,9 +227,9 @@ pub enum StampedEvent { } impl GetSize for StampedEvent { - fn get_heap_size(&self) -> usize { - delegate_stamped_event!(self => |e| vec_heap_size_with_fn_helper(&e.event.entries, |e| { - e.key.get_heap_size() + e.value.get_heap_size() + fn get_heap_size_with_tracker(&self, tracker: T) -> (usize, T) { + delegate_stamped_event!(self => |e| vec_heap_size_with_fn_helper(&e.event.entries, tracker, |e, mut tr| { + (e.key.get_heap_size_with_tracker(&mut tr).0 + e.value.get_heap_size_with_tracker(&mut tr).0, tr) })) } } diff --git a/src/shim/sector.rs b/src/shim/sector.rs index 1b2ffa770a84..76292b827faf 100644 --- a/src/shim/sector.rs +++ b/src/shim/sector.rs @@ -375,8 +375,8 @@ impl From for PoStProof { } impl GetSize for PoStProof { - fn get_heap_size(&self) -> usize { - self.0.proof_bytes.get_heap_size() + fn get_heap_size_with_tracker(&self, tracker: T) -> (usize, T) { + self.0.proof_bytes.get_heap_size_with_tracker(tracker) } } diff --git a/src/shim/state_tree.rs b/src/shim/state_tree.rs index d840817a28d8..b56da9e72129 100644 --- a/src/shim/state_tree.rs +++ b/src/shim/state_tree.rs @@ -427,8 +427,8 @@ impl ActorState { } impl GetSize for ActorState { - fn get_heap_size(&self) -> usize { - big_int_heap_size_helper(self.balance.atto()) + fn get_heap_size_with_tracker(&self, tracker: T) -> (usize, T) { + (big_int_heap_size_helper(self.balance.atto()), tracker) } } diff --git a/src/state_manager/mod.rs b/src/state_manager/mod.rs index b91e4a925ba8..f9fc1f7d23f2 100644 --- a/src/state_manager/mod.rs +++ b/src/state_manager/mod.rs @@ -50,7 +50,7 @@ use crate::shim::{ }; use crate::state_manager::cache::ForestCache; use crate::utils::cache::SizeTrackingCache; -use crate::utils::get_size::{GetSize, vec_heap_size_helper}; +use crate::utils::get_size::GetSize; use anyhow::Context as _; use chain_rand::ChainRand; use nonzero_ext::nonzero; @@ -77,14 +77,16 @@ pub struct ExecutedMessage { } impl GetSize for ExecutedMessage { - fn get_heap_size(&self) -> usize { - self.message.get_heap_size() - + self.receipt.get_heap_size() - + self - .events - .as_ref() - .map(vec_heap_size_helper) - .unwrap_or_default() + fn get_heap_size_with_tracker( + &self, + mut tracker: T, + ) -> (usize, T) { + ( + self.message.get_heap_size_with_tracker(&mut tracker).0 + + self.receipt.get_heap_size_with_tracker(&mut tracker).0 + + self.events.get_heap_size_with_tracker(&mut tracker).0, + tracker, + ) } } diff --git a/src/utils/get_size/mod.rs b/src/utils/get_size/mod.rs index 79259b83b429..5a922648defd 100644 --- a/src/utils/get_size/mod.rs +++ b/src/utils/get_size/mod.rs @@ -18,39 +18,52 @@ impl quick_cache::Equivalent for Cid { } macro_rules! impl_vec_alike_heap_size_with_fn_helper { - ($name:ident, $t:ty, $get_stack_size: expr, $get_heap_size: expr) => {{ + ($name:ident, $tracker:ident, $t:ty, $get_stack_size: expr, $get_heap_size_with_tracker: expr) => {{ let mut heap_size = 0; + let mut tr = $tracker; // use `____v` to avoid naming conflict for ____v in $name.iter() { - heap_size += $get_stack_size() + $get_heap_size(____v); + let (_s, _tr) = $get_heap_size_with_tracker(____v, tr); + heap_size += $get_stack_size() + _s; + tr = _tr; } let additional = usize::from($name.capacity()) - usize::from($name.len()); heap_size += additional * $get_stack_size(); - heap_size + (heap_size, tr) }}; } macro_rules! impl_vec_alike_heap_size_helper { - ($name:ident, $t:ty) => { + ($name:ident, $tracker:ident, $t:ty) => { impl_vec_alike_heap_size_with_fn_helper!( $name, + $tracker, $t, <$t>::get_stack_size, - GetSize::get_heap_size + GetSize::get_heap_size_with_tracker ) }; } -pub fn vec_heap_size_helper(v: &Vec) -> usize { - impl_vec_alike_heap_size_helper!(v, T) +pub fn vec_heap_size_with_fn_helper( + v: &Vec, + tracker: Tr, + get_heap_size_with_tracker: impl Fn(&T, Tr) -> (usize, Tr), +) -> (usize, Tr) { + impl_vec_alike_heap_size_with_fn_helper!( + v, + tracker, + T, + std::mem::size_of::, + get_heap_size_with_tracker + ) } -pub fn vec_heap_size_with_fn_helper(v: &Vec, get_heap_size: impl Fn(&T) -> usize) -> usize { - impl_vec_alike_heap_size_with_fn_helper!(v, T, std::mem::size_of::, get_heap_size) -} - -pub fn nunny_vec_heap_size_helper(v: &nunny::Vec) -> usize { - impl_vec_alike_heap_size_helper!(v, T) +pub fn nunny_vec_heap_size_helper( + v: &nunny::Vec, + tracker: Tr, +) -> (usize, Tr) { + impl_vec_alike_heap_size_helper!(v, tracker, T) } // This is a rough estimation. Use `b.allocation_size()` @@ -85,9 +98,50 @@ mod tests { let keys: nunny::Vec = nunny::vec![Cid::default().into(); 3]; // It's likely > 3 (4 on my laptop) println!("keys.capacity() = {}", keys.capacity()); + let tracker = get_size2::StandardTracker::new(); assert_eq!( - nunny_vec_heap_size_helper(&keys), + nunny_vec_heap_size_helper(&keys, tracker).0, CidWrapper::get_stack_size() * usize::from(keys.capacity()) ); } + + #[test] + fn test_derive_macro() { + #[derive(GetSize)] + struct A { + #[get_size(ignore)] + _cid: Cid, + } + + #[derive(GetSize)] + struct B { + #[get_size(size = 0)] + _cid: Cid, + } + + #[derive(GetSize)] + struct C { + #[get_size(size = 8)] + _cid: Cid, + } + + let _cid = Cid::default(); + let a = vec![A { _cid }]; + assert_eq!( + a.get_heap_size(), + std::mem::size_of_val(&_cid) * a.capacity() + ); + + let b = vec![B { _cid }]; + assert_eq!( + b.get_heap_size(), + std::mem::size_of_val(&_cid) * b.capacity() + ); + + let c = vec![C { _cid }]; + assert_eq!( + c.get_heap_size(), + (std::mem::size_of_val(&_cid) + 8) * c.capacity() + ); + } } From f75cf83451f9b69bab96c56393ab6d95dab5eb80 Mon Sep 17 00:00:00 2001 From: hanabi1224 Date: Mon, 8 Jun 2026 17:55:32 +0800 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 492377fdb9b0..eeadc28c6366 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ ### Fixed +- [#7129](https://github.com/ChainSafe/forest/pull/7129): Fixed a few inaccurate cache size metrics. + ## Forest v0.33.6 "Ebb" Non-mandatory release for all node operators. It fixes a critical memory leak in `v0.33.5`. (Earlier releases are not affected)