Skip to content
New issue

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

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

Already on GitHub? # to your account

Investigate inconsistent performance of hashTreeRoot() #78

Open
twoeths opened this issue Feb 15, 2021 · 5 comments · May be fixed by #378
Open

Investigate inconsistent performance of hashTreeRoot() #78

twoeths opened this issue Feb 15, 2021 · 5 comments · May be fixed by #378
Assignees

Comments

@twoeths
Copy link
Contributor

twoeths commented Feb 15, 2021

part of ChainSafe/lodestar#2046

this is a simple test to calculate hashTreeRoot

it.only("set validator valances", function () {
    this.timeout(0);
    const originalState = state.getOriginalState();
    // cache hashTreeRoot
    config.types.BeaconState.hashTreeRoot(originalState);
    const balances = Array.from({length: originalState.validators.length}, () => BigInt(31217089836));
    let minTime = Number.MAX_SAFE_INTEGER;
    let maxTime = 0;
    let average = {duration: 0, count: 0};
    const MAX_TRY = 10000;
    for (let i = 0; i < MAX_TRY; i++) {
      const state = config.types.BeaconState.clone(originalState);
      state.balances = balances as List<Gwei>;
      const start = Date.now();
      config.types.BeaconState.hashTreeRoot(state);
      const duration = Date.now() - start;
      const totalDuration = average.duration * average.count + duration;
      const totalCount = average.count + 1;
      average.count = totalCount;
      average.duration = totalDuration / totalCount;
      if (duration < minTime) minTime = duration;
      if (duration > maxTime) maxTime = duration;
    }
    console.log("hashTreeRoot minTime:", minTime, "maxTime:", maxTime, "average:", average.duration, "MAX_TRY:", MAX_TRY);
  });

this prints out hashTreeRoot minTime: 65 maxTime: 507 average: 80.46129999999977 MAX_TRY: 10000. I notice the max time is very similar to the one we have during a long epoch transition.

we need to investigate why the performance is inconsistent and if we can improve it.

@twoeths twoeths changed the title Investigate inconsistent performance of hashTreeRoot Investigate inconsistent performance of hashTreeRoot() Feb 15, 2021
@wemeetagain
Copy link
Member

Performance is inconsistent because we only re-hash new nodes in the tree since the last hashTreeRoot.
If there aren't many new nodes, it takes less time.
If there are many new new nodes since a hashTreeRoot, then it takes longer.

In the case of your test, the balances, which is a large subtree, get replaced. So those subtrees need complete rehashing, and hashTreeRoot takes longer.

@twoeths
Copy link
Contributor Author

twoeths commented Feb 16, 2021

Performance is inconsistent because we only re-hash new nodes in the tree since the last hashTreeRoot.
If there aren't many new nodes, it takes less time.
If there are many new new nodes since a hashTreeRoot, then it takes longer.

In the case of your test, the balances, which is a large subtree, get replaced. So those subtrees need complete rehashing, and hashTreeRoot takes longer.

in each for loop, I clone a new BeaconState to make it all equivalent so I think the cache is all the same (initially I did a hashTreeRoot call before the for loop to make the cache). Also, if I change MAX_TRY to 20, the maxTime is only 92 instead of 507 ( in case of MAX_TRY=10000): hashTreeRoot minTime: 67 maxTime: 92 average: 74.25 MAX_TRY: 20

so we need to investigate why sometimes it takes up to more than 500ms for MAX_TRY 10000

@twoeths
Copy link
Contributor Author

twoeths commented Feb 25, 2021

this is due to our hash implementation in as-sha256 ChainSafe/as-sha256#45, I don't see anything that can be improved in ssz or persistent-merkle-tree regarding this.

@dapplion
Copy link
Contributor

dapplion commented Mar 21, 2021

I've done some extra analysis of the performance of hashTreeRoot. Commenting to link the issues ChainSafe/lodestar#2206

The approach is to compare us with Lighthouse, and apparently our hashing function is a fast a theirs but for some reason our performance is still slower.

Also, to explain why we spent so much time hashing the state in the epoch transitions, maybe they pre-compute more roots beforehand somehow?

@twoeths
Copy link
Contributor Author

twoeths commented Jul 9, 2024

we switched to ssz v2 a long time ago #223
also the batch hash work going to save a lot of memory allocation which is the main contribution to the unstability

@twoeths twoeths linked a pull request Jul 9, 2024 that will close this issue
3 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants