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

Calculate memory offsets in u64 #447

Open
matklad opened this issue Feb 15, 2022 · 2 comments
Open

Calculate memory offsets in u64 #447

matklad opened this issue Feb 15, 2022 · 2 comments

Comments

@matklad
Copy link
Contributor

matklad commented Feb 15, 2022

EVM mload and mstore instructions seem relatively hot. Their arguments are u256, but this obviously can't work -- the memory needs to be limited somehow.

Reading the geth code, it seems the relevant bits are

I think what that says is that:

  • there's no intrinsic memory limit in EVM
  • but there's a implicit memory limit due to gas cost -- extending memory costs gas, and you can't have enough gas to pay for 1<<64 bytes of memory
  • so the actual offset calculation is carried out in u64

Switthing sputnik to use u64/usize for all offsets should speed it up somewhat significantly I think.

@matklad
Copy link
Contributor Author

matklad commented Feb 16, 2022

Ok, so the wins here are rather meaningful, about 6% of WASM gas.

I am in a bit of a doubt here though -- the correctness of the change would hinge on the fact that any memory offset larger than u32 is going to run out of gas, and, in sputnik, this is a non-local property, as gas counting and execution are very far apart from each other.

The design of oding seems to be more performant and correct here, as it simultanously executes the instruction and subtracts the gas cost, making this constraint local:

https://github.com/vorot93/evmodin/blob/e24f66db6292e4366caf57643b22238e8a2e2aa8/src/instructions/memory.rs#L66-L99

But refactoring sputnik to odin's design is probably a too big of a change...

@matklad
Copy link
Contributor Author

matklad commented Feb 18, 2022

PR implementing this: aurora-is-near/sputnikvm#9

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

No branches or pull requests

1 participant