-
Notifications
You must be signed in to change notification settings - Fork 293
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
evmone speedups #11
evmone speedups #11
Conversation
memory::allocate_memory returns uint8_t* instead of std::pair
added more memory tests stack over/underflow checked against explicit variables fixed bug where out of bounds memory indices were being written to
state.stack is no longer initialized by default removed a conditional branch in analyze loop added memory.cpp
push data converted in analysis stage
Hey, this looks great! I didn't expect this happen so quickly. I will definitely integrate your changes, but it will probably take me some time. To clarify one thing: there is some code duplication in the implementation (especially around calls). It was done on purpose because what I want to focus on now is add more unit tests to have full code coverage. |
Nice, @zac-williamson. The first four were on my to-do-if-I-ever-had-time list. I think Geth handles the fourth one with a small (<=4K) bitmap, if I understand you. |
I think the one optimization I still saw to do (and maybe it's in there but I missed it) is to pre-swap the PUSH constants. |
Heya, thanks for the feedback! I did squeeze that optimization in too, should have documented it. It's in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a possible DoS vulnerabilities, and fixing it might be a performance improvement.
/// | ||
/// The deque container is used because pointers to its elements are not | ||
/// invalidated when the container grows. | ||
std::deque<bytes32> args_storage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How big can this container get? Just the amount of push data in the program? Could a flat array be used to avoid the overhead (and possible DoS vulnerability) of intermittently growing the deque? Ot is setting aside that much memory, potentially almost the whole program, not worth it?
The 0.1 version of evmone has been tagged to be used as the base line for future optimizations. |
@zac-williamson I'd like discuss some changes you have here. |
After all these years, I'm finally getting threaded code support: #495. |
Hi @chfast!
I've been tinkering around with your excellent evm interpreter, and have made a few tweaks and additions that speed up the benchmarks by 1.25x - 3x. Perhaps they would be of interest? This branch has the following changes/additions:
The results have been quite interesting. On my machine (8th gen i7), the sha1_divs benchmark is the weakest performer, with only a ~25% speed increase. The blake2b benchmark, on the other hand, is ~3x faster at the top end, chewing through 3.56 billion gas per second for the blake2b_shifts/65536 benchmark.
If you would like to integrate these changes into evmone, I'd be happy to fix up anything that you think needs attention.