Skip to content

Conversation

@meetrick
Copy link
Contributor

Summary

Add shrink_to_fit() call after removing stale filters in EthFilter::clear_stale_filters().

Problem

When filters are removed via HashMap::retain(), the HashMap keeps its allocated capacity. This means if many filters are created and then expire, the memory stays allocated until the node restarts.

Example scenario:

  1. 10,000 filters are created → HashMap capacity grows to ~10,000
  2. All filters expire and get removed by clear_stale_filters()
  3. HashMap now has 0 elements but still holds capacity for ~10,000 entries
  4. This memory is never released

This is known as the "high water mark" problem.

Solution

Call shrink_to_fit() after retain() to release unused capacity.

This follows the same pattern used in EthStateCache::shrink_queues() (added in #14105).

Before vs After

Scenario Before After
10,000 filters created then expired Capacity: ~10,000 (memory retained) Capacity: 0 (memory released)
Memory released when? Only on node restart Every 5 minutes (stale_filter_ttl)

Testing

  • cargo check -p reth-rpc --lib
  • cargo test -p reth-rpc --lib -- filter ✅ (10 tests passed)

Add shrink_to_fit() call after retain() in clear_stale_filters() to release
excess HashMap capacity. This prevents memory from being retained indefinitely
after a large number of filters expire (high water mark problem).

Similar to the shrink_queues() pattern used in EthStateCache (PR paradigmxyz#14105).

Signed-off-by: Hwangjae Lee <[email protected]>
@meetrick
Copy link
Contributor Author

@mattsse In this PR, I used shrink_to_fit(), which shrinks the map to the exact size (capacity is 0 when empty).

My thinking was that clear_stale_filters runs every 5 minutes, so the cost of reallocating should be small. Still, I’m not sure if keeping a small minimum capacity would be better.

Which option do you prefer?

  • shrink_to_fit(): shrink to exact size
  • shrink_to(2): keep a small minimum capacity, like EthStateCache

I’m happy to change it if you have a preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant