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

[WIP] Improve RLP encode/decode core types #12579

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

racytech
Copy link
Contributor

@racytech racytech commented Nov 2, 2024

This PR will fix RLP related issues, removing some unused parts and improving some other parts of the code. Benchmarks on Log type encoding/decoding shows that it makes sense to keep using hand-written code.

@racytech
Copy link
Contributor Author

racytech commented Nov 2, 2024

Moved RLP logic in core types to separate file

@racytech
Copy link
Contributor Author

racytech commented Nov 4, 2024

Moved often used rlp functions from ./rlp/encode to ./erigon-lib/rlp/encode

@AskAlexSharov
Copy link
Collaborator

Benchmarks on Log type encoding/decoding shows that it makes sense to keep using hand-written code. - did you compare it with reflection-based code or with rlpgen-based?

@racytech
Copy link
Contributor Author

racytech commented Nov 4, 2024

With reflection-based, on Log struct only

@racytech
Copy link
Contributor Author

racytech commented Nov 4, 2024

let me test it against rlpgen

@racytech
Copy link
Contributor Author

racytech commented Nov 4, 2024

RLPgen: 492243439 48.56 ns/op 0 B/op 0 allocs/op
ByHand: 312318904 79.10 ns/op 144 B/op 4 allocs/op

We should move towards rlpgen, if no objections, didn't know what that is :)

@racytech
Copy link
Contributor Author

racytech commented Nov 4, 2024

Silkworm based approach + reusable buffer: 667179529 35.36 ns/op 0 B/op 0 allocs/op

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.

2 participants