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
Reuse VM memory across executions #1888
base: master
Are you sure you want to change the base?
Conversation
pub(crate) fn get_vm_memory() -> Memory { | ||
POOL.with(|pool| { | ||
let mut pool = pool.borrow_mut(); | ||
pool.pop().unwrap_or_else(Memory::new) |
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.
We should probably have some kind constraint on the max number of items allowed in each pool? Otherwise it should wait if there are none available.
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.
We didn't do this earlier, and I don't see why it would make sense to track the items here, instead of just limiting concurrency of the executor itself. That said, keeping a pool per thread is by itself problematic and not useful, as:
- The threads are not CPU-pinned anyway
- The async runtime might decide to create new threads arbitarily, which would essentially cause a memory leak here
So I' changed the code to not keep separate pools in 5caec42
Closes #1878. Requires FuelLabs/fuel-vm#732 and a new fuel-vm release.
Introduces a per-thread pool for allocated VM memory instances. Whenever a new VM instance is needed for a tx, the memory is taken from this pool. After the tx has been executed, the memory is reset to an empty state without deallocating, and returned to the pool.
A per-thread pool was chosen to no pass allocations between threads unnecessarily.
Checklist
Before requesting review