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

Share memory between calls #650

Closed
wants to merge 4 commits into from

Conversation

thedevbirb
Copy link
Contributor

@thedevbirb thedevbirb commented Aug 26, 2023

I tried to give a shot to #445 just for fun and as a challenge with this draft PR, which is still WIP.
This introduces a new struct called SharedMemory which is indeed shared between calls. I'd like a feedback on my implementation to see what it can done better.

Here are some implementation choices I made:

  • its API matches almost completely the 'old' interpreter Memory implementation
  • memory is allocated using this estimate https://2π.com/22/eth-max-mem/ (more on this below)
  • it has a pointer current_slice which is internally used to refer to the portion of data reserved for the current context. This requires two unsafe methods get_current_slice and get_current_slice_mut which deferences the raw pointer
  • current_slice pointer is updated when entering a new context (which is when a new Interpreter instance is created) using the use_new_memory method (every name is still WIP too) and when exiting it, which happens when the return_value method of Interpreter is called
  • in order to make SharedMemory work properly, it is wrapped in a Rc for dealing with recursive calls and with an RefCell to allow mutability

Little problem: now test passes because I always allocate u32::MAX bytes. If I use the estimation however they fail because some tests have an impressive gas limit (at least 4_503_599_627_349_304), and my system fails to allocate so much memory (34_359_713_280 bytes). I don't really know how to handle this at the moment

Thanks in advance for any feedback!

@thedevbirb thedevbirb force-pushed the shared_memory branch 5 times, most recently from d9c077a to d76a2a2 Compare August 26, 2023 14:47
@thedevbirb
Copy link
Contributor Author

Closing this as I've found some stuff that needs more work than I thought

@thedevbirb thedevbirb closed this Aug 28, 2023
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.

1 participant