Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use BTreeMap for Milhouse pending updates (v7.0.0) #7043

Open
wants to merge 1 commit into
base: release-v7.0.0
Choose a base branch
from

Conversation

michaelsproul
Copy link
Member

Proposed Changes

Optimisation to remove VecMap usage, as with millions of validators it is becoming a significant contributor to peak memory usage.

CPU Benchmarks

CPU benchmarks show that BTreeMap is faster for recent slots. Originally when we introduced milhouse the BTreeMap was slower than VecMap. I suspect the memory allocation time is now dominating the memory access time, which was VecMap's main advantage.

Benchmark command:

RUST_LOG=debug lcli transition-blocks --pre-state-path bench/state_10876895.ssz --block-path bench/block_10876896.ssz --runs 100 --exclude-cache-builds

on unstable (as of 06329ec):

[2025-01-20T07:26:22Z DEBUG lcli::transition_blocks] Slot processing: 635.089345ms
[2025-01-20T07:26:22Z DEBUG lcli::transition_blocks] Build all caches (again): 620ns
[2025-01-20T07:26:22Z DEBUG lcli::transition_blocks] Batch verify block signatures: 31.863842ms
[2025-01-20T07:26:22Z DEBUG lcli::transition_blocks] Process block: 15.014572ms
[2025-01-20T07:26:22Z DEBUG lcli::transition_blocks] Post-block tree hash: 174.293822ms
[2025-01-20T07:26:22Z INFO lcli::transition_blocks] Run 99: 856.36194ms

on this branch using BTreeMap for validators and balances:

[2025-01-20T06:54:31Z DEBUG lcli::transition_blocks] Slot processing: 605.424233ms
[2025-01-20T06:54:31Z DEBUG lcli::transition_blocks] Build all caches (again): 580ns
[2025-01-20T06:54:31Z DEBUG lcli::transition_blocks] Batch verify block signatures: 30.883823ms
[2025-01-20T06:54:31Z DEBUG lcli::transition_blocks] Process block: 16.656031ms
[2025-01-20T06:54:31Z DEBUG lcli::transition_blocks] Post-block tree hash: 132.007032ms
[2025-01-20T06:54:31Z INFO lcli::transition_blocks] Run 99: 785.045689ms

Memory Metrics

Pending. Initial tests on Holesky while syncing during non-finality show that the VecMap uses substantial memory (20GB+).

Additional Info

Ideally merge after:

@michaelsproul michaelsproul changed the base branch from stable to release-v7.0.0 February 26, 2025 05:48
@michaelsproul michaelsproul changed the title Milhouse btreemap v7 Use BTreeMap for Milhouse pending updates (v7.0.0) Feb 26, 2025
@michaelsproul
Copy link
Member Author

This did not work. Removing from beta.2 for now.

@dknopik
Copy link
Member

dknopik commented Feb 26, 2025

This did not work. Removing from beta.2 for now.

I also experimented with this idea, unaware that this PR exists. Not sure what happened in your case, but in my case, it slowed things down quite a bit - but switching Balances back to the VecMap fixed it for me. My theory is that updates to Validators are more rare and affecting fewer validators, making the B-tree overhead less noticeable (and benefiting from not having a huge Vec for few updates), while the Balances are updated for all validators in every epoch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants