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

Add an LRU cache to make_trie_root_and_nodes() #573

Merged
merged 1 commit into from
Apr 19, 2018
Merged

Add an LRU cache to make_trie_root_and_nodes() #573

merged 1 commit into from
Apr 19, 2018

Conversation

gsalgado
Copy link
Collaborator

@gsalgado gsalgado commented Apr 18, 2018 •
edited
Loading

No description provided.

@gsalgado gsalgado force-pushed the optimize-make_trie_root_and_nodes branch from 50492fb to 7ae56b3 Compare April 18, 2018 13:08
@gsalgado gsalgado mentioned this pull request Apr 18, 2018
Merged
@gsalgado
Copy link
Collaborator Author

Unfortunately this won't be enough by itself because we'd still be making one block.get_receipts() call for every tx applied, and that's too expensive to run for every tx.

@gsalgado
Copy link
Collaborator Author

And caching ChainDB.get_receipts() doesn't help because every time it is called (as part of VM.apply_transaction()) it is passed a different receipt_root for the trie containing only the receipts of the transactions that have already been applied

@gsalgado
Copy link
Collaborator Author

So, this won't replace #565 but it might still be beneficial, specially since Byzantium blocks may have duplicate receipt_roots. Will benchmark to find out

@gsalgado gsalgado mentioned this pull request Apr 18, 2018
Closed
@gsalgado gsalgado force-pushed the optimize-make_trie_root_and_nodes branch 2 times, most recently from 50b2479 to 0241726 Compare April 19, 2018 11:23
@gsalgado gsalgado changed the title RFC: Optimize make_trie_root_and_nodes Add an LRU cache to make_trie_root_and_nodes() Apr 19, 2018
@gsalgado
Copy link
Collaborator Author

Now that #565 has been merged and we're no longer generating/storing partial tx/receipt tries, it makes more sense to cache the whole thing here.

@gsalgado gsalgado force-pushed the optimize-make_trie_root_and_nodes branch from 0241726 to 99d3b3d Compare April 19, 2018 15:52
pipermerriam
pipermerriam approved these changes Apr 19, 2018
View reviewed changes
evm/db/trie.py Outdated
# as it's common for them to have duplicate receipt_roots. Given that, it probably makes sense to
# use a relatively small cache size here.
@functools.lru_cache(128)
def _make_trie_root_and_nodes(items: Tuple[bytes, ...]) -> Tuple[bytes, Dict]:
Copy link
Member

Choose a reason for hiding this comment

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

Return type should be Tuple[bytes, Dict[bytes, bytes]] right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed. I guess mypy considers a bare Dict to be Dict[Any, Any]

@gsalgado gsalgado force-pushed the optimize-make_trie_root_and_nodes branch from 99d3b3d to 471af62 Compare April 19, 2018 17:03
@gsalgado gsalgado merged commit 2b8da9b into ethereum:master Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pipermerriam pipermerriam pipermerriam approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@gsalgado @pipermerriam