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.

Sign up for GitHub

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

Jump to bottom

HWY-250: Use ConsensusValue's hash as a key in map. #881

Merged
merged 4 commits into from
Feb 8, 2021
Merged

HWY-250: Use ConsensusValue's hash as a key in map. #881

merged 4 commits into from
Feb 8, 2021

Conversation

goral09
Copy link
Collaborator

@goral09 goral09 commented Feb 7, 2021 •
edited

Instead of storing the whole ConsensusValue as a key in the map (where ConsensusValue is a set of all deploys that the block includes), we will store a hash of ConsensusValue. This will improve memory management.

https://casperlabs.atlassian.net/browse/HWY-250

@goral09
HWY-250: Use ConsensusValue's hash as a key in map.
3cd4881 
Helps improve the memory management.
@goral09 goral09 requested a review from afck February 7, 2021 21:10
@goral09 goral09 requested a review from fizyk20 February 7, 2021 23:38
afck
afck reviewed Feb 8, 2021
View reviewed changes
Copy link
Contributor

@afck afck left a comment

Choose a reason for hiding this comment

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

That solves only the smaller part of the problem, and reduces the size of the synchronizer but not the protocol state.

Can we make ConsensusValue itself be a candidate block hash, so it's also just a hash in the protocol state?

afck
afck approved these changes Feb 8, 2021
View reviewed changes
Copy link
Contributor

@afck afck left a comment

Choose a reason for hiding this comment

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

You're right, removing it from the protocol state, too, will be a bigger issure we can do later. This should improve memory usage.

@goral09
Copy link
Collaborator Author

goral09 commented Feb 8, 2021

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 8, 2021

Build succeeded:

@bors bors bot merged commit 46f7fb9 into casper-network:master Feb 8, 2021
@afck afck mentioned this pull request Feb 9, 2021
Merged
bors bot added a commit that referenced this pull request Feb 9, 2021
@bors @afck
Merge #889
137c070 
889: NO-TICKET: Use timestamp in candidate block hash. r=afck a=afck

I missed that when rebasing #879 on #881.

Co-authored-by: Andreas Fackler <andreas@casperlabs.io>
@goral09 goral09 deleted the no-tick_cv_hash branch February 10, 2021 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afck afck afck approved these changes

@fizyk20 fizyk20 Awaiting requested review from fizyk20

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@goral09 @afck