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

Cache filtered block tree #4515

Merged
merged 12 commits into from Jan 13, 2020
Merged

Cache filtered block tree #4515

merged 12 commits into from Jan 13, 2020

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Jan 12, 2020

Part of #4509

This PR caches filtered block tree. Instead of calculating the filtered tree on the fly, the cache maintains the tree and update the tree when there's a new post state
The cache is gated by --cache-filtered-block-tree. Huge improvements on flame 👇

Before:
Screen Shot 2020-01-12 at 1 32 21 PM

After:
Screen Shot 2020-01-12 at 1 58 25 PM

@terencechain terencechain added the Ready For Review A pull request ready for code review label Jan 12, 2020
@codecov
Copy link

codecov bot commented Jan 12, 2020

Codecov Report

Merging #4515 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4515   +/-   ##
=======================================
  Coverage   54.62%   54.62%           
=======================================
  Files         231      231           
  Lines       15214    15214           
=======================================
  Hits         8310     8310           
  Misses       5674     5674           
  Partials     1230     1230

Copy link
Contributor

@rauljordan rauljordan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No mutexes?

@terencechain
Copy link
Member Author

No mutexes?

Didn't think it was needed because we recalculate/update the map as a whole.. but better be safe than sorry. Defensive programming 101, will add a mutex

filteredBlocks := make(map[[32]byte]*ethpb.BeaconBlock)
var err error
if featureconfig.Get().EnableBlockTreeCache {
s.filteredBlockTreeLock.Lock()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RLock? Seems like you could process many readers in parallel with attestations coming in pubsub quickly

shared/featureconfig/flags.go Outdated Show resolved Hide resolved
@prylabs-bulldozer prylabs-bulldozer bot merged commit a8edfa4 into master Jan 13, 2020
@delete-merged-branch delete-merged-branch bot deleted the cache-filtered-tree branch January 13, 2020 04:12
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 24, 2020
* Cache filtered block tree
* Merge refs/heads/master into cache-filtered-tree
* Merge refs/heads/master into cache-filtered-tree
* Add locks
* Merge branch 'cache-filtered-tree' of git+ssh://github.com/prysmaticlabs/prysm into cache-filtered-tree
* Confligt
* Merge refs/heads/master into cache-filtered-tree
* Merge refs/heads/master into cache-filtered-tree
* Update shared/featureconfig/flags.go

Co-Authored-By: Preston Van Loon <preston@prysmaticlabs.com>
* Rlock
* Merge branch 'master' of git+ssh://github.com/prysmaticlabs/prysm into cache-filtered-tree
* Merge branch 'cache-filtered-tree' of git+ssh://github.com/prysmaticlabs/prysm into cache-filtered-tree
cryptomental pushed a commit to cryptomental/prysm that referenced this pull request Feb 28, 2020
* Cache filtered block tree
* Merge refs/heads/master into cache-filtered-tree
* Merge refs/heads/master into cache-filtered-tree
* Add locks
* Merge branch 'cache-filtered-tree' of git+ssh://github.com/prysmaticlabs/prysm into cache-filtered-tree
* Confligt
* Merge refs/heads/master into cache-filtered-tree
* Merge refs/heads/master into cache-filtered-tree
* Update shared/featureconfig/flags.go

Co-Authored-By: Preston Van Loon <preston@prysmaticlabs.com>
* Rlock
* Merge branch 'master' of git+ssh://github.com/prysmaticlabs/prysm into cache-filtered-tree
* Merge branch 'cache-filtered-tree' of git+ssh://github.com/prysmaticlabs/prysm into cache-filtered-tree
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants