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

Add encode cache #37

Merged
merged 3 commits into from
Feb 8, 2019
Merged

Add encode cache #37

merged 3 commits into from
Feb 8, 2019

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Feb 6, 2019

What was wrong?

#36 part 1: Add cache for encode.

How was it fixed?

Basically copied the same logic from pyrlp.

Cute Animal Picture

jumping-cute-playing-animals

Copy link
Contributor

@jannikluhn jannikluhn left a comment

Choose a reason for hiding this comment

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

Approved, even though I'm still not convinced that caching like this is very helpful. In practice, in which situations are we going to serialize the same object multiple times? I can only think of hashing, but if that's the only reason we could just cache the hash.

Another thought: We could set cached_ssz in ssz.decode as well, then we could avoid serialization in some cases completely (e.g. we get something from the network and send it to other peers).

ssz/codec.py Outdated Show resolved Hide resolved
jannikluhn and others added 2 commits February 6, 2019 20:20
Co-Authored-By: hwwhww <hwwang156@gmail.com>
@hwwhww
Copy link
Contributor Author

hwwhww commented Feb 6, 2019

Approved, even though I'm still not convinced that caching like this is very helpful. In practice, in which situations are we going to serialize the same object multiple times? I can only think of hashing, but if that's the only reason we could just cache the hash.

Yeah no good use case in my mind either in client side.

Another thought: We could set cached_ssz in ssz.decode as well, then we could avoid serialization in some cases completely (e.g. we get something from the network and send it to other peers).

Oh yes just noticed that there are also cache for decode: ethereum/pyrlp#95 (and https://github.com/ethereum/pyrlp/blob/7904316f3da420d8dda214dfe8fa168a8e866cfb/rlp/codec.py#L245-L258). Will take a look.

@hwwhww hwwhww changed the title Add serialization cache Add encode cache Feb 8, 2019
@hwwhww hwwhww merged commit 60865ce into ethereum:master Feb 8, 2019
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.

None yet

3 participants