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

Limit VMResult cases when concrete #447

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Limit VMResult cases when concrete #447

merged 1 commit into from
Feb 5, 2024

Conversation

arcz
Copy link
Collaborator

@arcz arcz commented Feb 1, 2024

Description

This builds on top of #427. Better types using VMType allowed to eliminate dead code and optimize concrete execution with specialized branch. The speedup is ~10% on ethereum-tests.

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

@arcz arcz requested review from d-xo and msooseth February 1, 2024 23:27
Copy link
Collaborator

@msooseth msooseth left a comment

Choose a reason for hiding this comment

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

Actually makes the code cleaner and easier to read, too. I like it a lot, thanks!

@@ -1251,32 +1251,11 @@ getCodeLocation vm = (vm.state.contract, vm.state.pc)
query :: Query t s -> EVM t s ()
query = assign #result . Just . HandleEffect . Query

choose :: Choose t s -> EVM t s ()
choose :: Choose s -> EVM Symbolic s ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Notes to self: so choose is only defined for Symbolic now.

@@ -2718,6 +2715,10 @@ instance VMOps Concrete where

whenSymbolicElse _ a = a

partial _ = internalError "won't happen during concrete exec"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, so partial cannot happen during concrete exec

@@ -2589,6 +2565,27 @@ instance VMOps Symbolic where
toGas _ = ()
whenSymbolicElse a _ = a

partial e = assign #result (Just (Unfinished e))
Copy link
Collaborator

Choose a reason for hiding this comment

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

partial now only needs to be defined for symbolic exec

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the long run, partial should not be part of VMOps but it needs a slight refactor to other functions

@@ -46,7 +45,7 @@ data Action t s a where
Wait :: Query t s -> Action t s ()

-- | Multiple things can happen
Ask :: Choose t s -> Action t s ()
Ask :: Choose s -> Action Symbolic s ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is nice actually.

@msooseth msooseth merged commit e938e47 into main Feb 5, 2024
7 checks passed
@arcz arcz deleted the limit-vmresult branch February 6, 2024 14:53
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