feat: cacheless hamt iteration by hanabi1224 · Pull Request #2216 · filecoin-project/ref-fvm

hanabi1224 · 2025-09-04T12:08:53Z

(Simliar to #2189)
This PR adds HamtImpl::for_each_cacheless and StateTree::for_each_cacheless to reduce memory usage for iterating over a large hamt, to address #2215 (comment)

cargo bench shows no significant difference between for_each and for_each_cacheless

HAMT for_each function  time:   [108.24 µs 108.70 µs 109.19 µs]
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild

HAMT for_each_cacheless function
                        time:   [106.85 µs 107.63 µs 108.58 µs]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

codecov-commenter · 2025-09-04T12:10:58Z

Codecov Report

❌ Patch coverage is 80.64516% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.59%. Comparing base (b5883f4) to head (aeaa05b).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
fvm/src/state_tree.rs	0.00%	10 Missing ⚠️
ipld/hamt/src/node.rs	95.45%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2216      +/-   ##
==========================================
+ Coverage   77.58%   77.59%   +0.01%     
==========================================
  Files         147      147              
  Lines       15789    15851      +62     
==========================================
+ Hits        12250    12300      +50     
- Misses       3539     3551      +12

Files with missing lines	Coverage Δ
ipld/hamt/src/hamt.rs	`97.26% <100.00%> (+0.10%)`	⬆️
ipld/hamt/src/hash_algorithm.rs	`73.33% <ø> (ø)`
ipld/hamt/src/lib.rs	`100.00% <ø> (ø)`
ipld/hamt/src/node.rs	`91.74% <95.45%> (+0.44%)`	⬆️
fvm/src/state_tree.rs	`72.61% <0.00%> (-3.15%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

hanabi1224 · 2025-09-04T14:24:44Z

+    fn hash<X>(key: &X) -> HashedKey
    where
-        X: Hash,
+        X: Hash + ?Sized,


To fix a clippy warning

LesnyRumcajs · 2025-09-10T11:00:24Z

        Ok(())
    }
+
+    pub fn for_each_cacheless<F>(&self, mut f: F) -> anyhow::Result<()>


LesnyRumcajs · 2025-09-10T11:01:44Z

Changelog entry?

LesnyRumcajs · 2025-09-10T11:06:21Z

All in all, LGTM, but I'll let @rvagg have a final say here, my HAMT-fu is not that strong.

Copilot

Pull Request Overview

This PR adds cacheless iteration functionality to HAMT (Hash Array Mapped Trie) to reduce memory usage when iterating over large HAMTs. The primary addition is for_each_cacheless methods that avoid caching nodes during iteration, which can be more memory-efficient for one-time traversals.

Implements for_each_cacheless methods for both HamtImpl and StateTree
Adds comprehensive tests for the new cacheless iteration functionality
Includes benchmarks comparing performance between cached and cacheless iteration

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
ipld/hamt/tests/hamt_tests.rs	Adds comprehensive test suite for `for_each_cacheless` functionality and refactors existing test infrastructure
ipld/hamt/src/pointer.rs	Implements `Clone` trait for `Pointer` enum to support cacheless iteration
ipld/hamt/src/node.rs	Implements `Clone` trait for `Node` and adds core `for_each_cacheless` iteration logic
ipld/hamt/src/lib.rs	Adds `Clone` trait to `KeyValuePair` struct to support cloning operations
ipld/hamt/src/hash_algorithm.rs	Minor refactor to parameter order for `Identity::hash` method
ipld/hamt/src/hamt.rs	Adds public `for_each_cacheless` method to the main HAMT API
ipld/hamt/benches/hamt_benchmark.rs	Adds benchmark for `for_each_cacheless` and refactors existing benchmarks
ipld/hamt/Cargo.toml	Adds `itertools` dependency for test utilities
fvm/src/state_tree.rs	Adds `for_each_cacheless` method to `StateTree` API

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-16T05:24:42Z

+                }
+                IterItem::Borrowed(Pointer::Dirty(node)) => stack.push(node.pointers.iter().into()),
+                IterItem::Owned(Pointer::Dirty(node)) => {
+                    stack.push(node.pointers.clone().into_iter().into())


The clone() call here creates an unnecessary copy of the entire pointers vector. Consider restructuring to avoid this clone operation, as it defeats the memory efficiency purpose of the cacheless iteration.

Suggested change

stack.push(node.pointers.clone().into_iter().into())

stack.push(node.pointers.into_iter().into())

Copilot · 2025-09-16T05:24:43Z

+    /// map.set(4, 2).unwrap();
+    ///
+    /// let mut total = 0;
+    /// map.for_each_cacheless(|_, v: &u64| {


The type annotation in the example should be &usize to match the generic type used in the example setup, not &u64.

Suggested change

/// map.for_each_cacheless(|_, v: &u64| {

/// map.for_each_cacheless(|_, v: &usize| {

…less

hanabi1224 · 2025-10-23T13:56:28Z

Hey @LesnyRumcajs @rvagg I have addressed all outstanding comments and all CI checks are green, please take another look.

LesnyRumcajs

LGTM, but I'll let @rvagg make a final call. As mentioned earlier, my HAMT-fu is weak. :)

ZenGround0

This LGTM. It's nice you are getting the same ordering but that's probably more than you need anyway since HAMTs are inherently unordered.

ZenGround0 · 2026-02-25T20:28:07Z

    /// map.set(4, 2).unwrap();
    ///
    /// let mut total = 0;
-    /// map.for_each(|_, v: &u64| {


why remove this? Unless I'm missing something it clarifies the example.

ZenGround0 · 2026-02-25T20:43:55Z

+    where
+        F: FnMut(&K, &V) -> anyhow::Result<()>,
+        S: Blockstore,
+        K: Clone,


Do we actually need to implement clone? I tried compiling with the trait removed from requirements here and Node and things seem to work.

You're right, Clone is no longer needed. Removed.

…less

feat: cacheless hamt iteration

0979a8d

github-project-automation Bot added this to FilOz Sep 4, 2025

github-project-automation Bot moved this to 📌 Triage in FilOz Sep 4, 2025

update bench

3cd52f7

hanabi1224 marked this pull request as ready for review September 4, 2025 12:31

hanabi1224 added 3 commits September 4, 2025 20:58

refactor

f60806e

parity test

9debdbc

fix iteration order

313aec8

hanabi1224 commented Sep 4, 2025

View reviewed changes

hanabi1224 requested review from LesnyRumcajs and rvagg September 5, 2025 13:57

hanabi1224 self-assigned this Sep 5, 2025

BigLep moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Sep 8, 2025

LesnyRumcajs reviewed Sep 10, 2025

View reviewed changes

BigLep requested a review from Copilot September 16, 2025 05:24

Copilot AI reviewed Sep 16, 2025

View reviewed changes

hanabi1224 added 3 commits October 23, 2025 17:33

Merge remote-tracking branch 'origin/master' into hamt/for_each_cache…

1b9d346

…less

docs and changelog

a5a552f

resolve AI comments

b0385e5

LesnyRumcajs reviewed Oct 27, 2025

View reviewed changes

ZenGround0 approved these changes Feb 25, 2026

View reviewed changes

github-project-automation Bot moved this from 🔎 Awaiting Review to ✔️ Approved by reviewer in FilOz Feb 25, 2026

hanabi1224 added 2 commits February 26, 2026 20:09

Merge remote-tracking branch 'origin/master' into hamt/for_each_cache…

a510f35

…less

remove unnecessary Clone

aeaa05b

LesnyRumcajs merged commit 8e2f86b into filecoin-project:master Feb 26, 2026
17 checks passed

github-project-automation Bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Feb 26, 2026

hanabi1224 mentioned this pull request Mar 6, 2026

consider for_each_cacheless for HAMT and KAMT #2193

Open

hanabi1224 deleted the hamt/for_each_cacheless branch March 6, 2026 11:09

	stack.push(node.pointers.clone().into_iter().into())
	stack.push(node.pointers.into_iter().into())

	/// map.for_each_cacheless(\|_, v: &u64\| {
	/// map.for_each_cacheless(\|_, v: &usize\| {

Uh oh!

Conversation

hanabi1224 commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LesnyRumcajs commented Sep 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanabi1224 commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LesnyRumcajs left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZenGround0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

hanabi1224 commented Sep 4, 2025 •

edited

Loading

codecov-commenter commented Sep 4, 2025 •

edited

Loading

hanabi1224 commented Oct 23, 2025 •

edited

Loading

LesnyRumcajs left a comment •

edited

Loading