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

Reuse VM memory across executions #1888

Merged
merged 29 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
81dc78a
Reuse VM memory across executions
Dentosal May 8, 2024
42105ce
Add changelog entry
Dentosal May 8, 2024
e14cf0b
Merge branch 'master' into dento/reuse-vm-memory
Dentosal May 8, 2024
5caec42
Switch to a unified pool instead of per-thread one
Dentosal May 9, 2024
6340785
Move VmPool to fuel-vm, use it for predicates as well
Dentosal May 10, 2024
22fa926
Cleanup
Dentosal May 10, 2024
8c276e9
Cleanup
Dentosal May 10, 2024
0b11719
Fix benches
Dentosal May 10, 2024
4c6f29b
Add changes from the latest fuel-vm side update
Dentosal May 13, 2024
22f3ba4
Match fuel-vm changes
Dentosal May 14, 2024
4f9340c
Merge branch 'master' into dento/reuse-vm-memory
Dentosal May 14, 2024
57d2049
cargo sort
Dentosal May 14, 2024
0e41a28
Merge branch 'refs/heads/master' into dento/reuse-vm-memory
xgreenx Jun 3, 2024
a762483
Use `fuel-vm 0.51.0`
xgreenx Jun 3, 2024
131a91e
Merge branch 'master' into dento/reuse-vm-memory
xgreenx Jun 3, 2024
98c7495
Make clippy happy
xgreenx Jun 3, 2024
395077b
Fix CI
xgreenx Jun 3, 2024
36fa8be
Fix CI
xgreenx Jun 3, 2024
3baea26
Fix CI
xgreenx Jun 3, 2024
b13578f
Fixed VM initialization benchmark
xgreenx Jun 3, 2024
eb65aba
Merge branch 'master' into dento/reuse-vm-memory
xgreenx Jun 3, 2024
5196bab
Use release 0.51
xgreenx Jun 3, 2024
2e4bc5b
Merge remote-tracking branch 'origin/dento/reuse-vm-memory' into dent…
xgreenx Jun 3, 2024
c2034bb
Make clippy happy
xgreenx Jun 3, 2024
ec2a65b
Fixed becnhmark for aloc opcode
xgreenx Jun 3, 2024
fc15915
Fixed becnhmark for aloc opcode
xgreenx Jun 3, 2024
6000f11
Not include cloning of memory into the benchamrk results
xgreenx Jun 3, 2024
56915e7
Return back old benchmark
xgreenx Jun 3, 2024
8680666
Merge branch 'master' into dento/reuse-vm-memory
xgreenx Jun 4, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- [#1886](https://github.com/FuelLabs/fuel-core/pull/1886): Use ref to `Block` in validation code
- [#1876](https://github.com/FuelLabs/fuel-core/pull/1876): Updated benchmark to include the worst scenario for `CROO` opcode. Also include consensus parameters in bench output.

### Changed

- [#1879](https://github.com/FuelLabs/fuel-core/pull/1879): Return the old behaviour for the `discovery_works` test.
- [#1848](https://github.com/FuelLabs/fuel-core/pull/1848): Added `version` field to the `Block` and `BlockHeader` GraphQL entities. Added corresponding `version` field to the `Block` and `BlockHeader` client types in `fuel-core-client`.

### Changed

- [#1873](https://github.com/FuelLabs/fuel-core/pull/1873/): Separate dry runs from block production in executor code, remove `ExecutionKind` and `ExecutionType`, remove `thread_block_transaction` concept, remove `PartialBlockComponent` type, refactor away `inner` functions.
- [#1888](https://github.com/FuelLabs/fuel-core/pull/1888): optimization: Reuse VM memory across executions.

## [Version 0.26.0]

Expand Down
24 changes: 8 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,6 @@ itertools = { version = "0.12", default-features = false }
insta = "1.8"
tempfile = "3.4"
tikv-jemallocator = "0.5"

[patch.crates-io]
fuel-vm-private = { package = "fuel-vm", git = "https://github.com/FuelLabs/fuel-vm", branch = "dento/memory-reset" }
47 changes: 36 additions & 11 deletions crates/services/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use crate::{
TransactionsSource,
},
refs::ContractRef,
vm_pool,
};
use core::mem;
use fuel_core_storage::{
column::Column,
kv_store::KeyValueInspect,
Expand Down Expand Up @@ -1414,11 +1416,6 @@ where
coinbase_contract_id,
);

let mut vm = Interpreter::with_storage(
vm_db,
InterpreterParams::new(gas_price, &self.consensus_params),
);

let gas_costs = self.consensus_params.gas_costs();
let fee_params = self.consensus_params.fee_params();

Expand All @@ -1430,33 +1427,61 @@ where
.collect();
let ready_tx = checked_tx.into_ready(gas_price, gas_costs, fee_params)?;

let vm_result: StateTransition<_> = vm
let mut vm = Interpreter::with_storage(
vm_db,
InterpreterParams::new(gas_price, &self.consensus_params),
);

// We get memory from the pool and are careful to return it there when done
*vm.memory_mut() = vm_pool::get_vm_memory();

// This returns the VM memory to the pool to before returning
macro_rules! recycling_try {
($expr:expr) => {
match $expr {
Ok(val) => val,
Err(e) => {
vm_pool::recycle_vm_memory(mem::take(vm.memory_mut()));
return Err(e.into())
}
}
};
}

let vm_result: StateTransition<_> = recycling_try!(vm
.transact(ready_tx)
.map_err(|error| ExecutorError::VmExecution {
error: error.to_string(),
transaction_id: tx_id,
})?
.into();
}))
.into();
let reverted = vm_result.should_revert();

let (state, mut tx, receipts): (_, Tx, _) = vm_result.into_inner();
#[cfg(debug_assertions)]
{
tx.precompute(&self.consensus_params.chain_id())?;
recycling_try!(tx.precompute(&self.consensus_params.chain_id()));
debug_assert_eq!(tx.id(&self.consensus_params.chain_id()), tx_id);
}

Self::update_input_used_gas(predicate_gas_used, tx_id, &mut tx)?;
recycling_try!(Self::update_input_used_gas(
predicate_gas_used,
tx_id,
&mut tx
));

// We always need to update inputs with storage state before execution,
// because VM zeroes malleable fields during the execution.
self.compute_inputs(tx.inputs_mut(), storage_tx)?;
recycling_try!(self.compute_inputs(tx.inputs_mut(), storage_tx));

// only commit state changes if execution was a success
if !reverted {
self.log_backtrace(&vm, &receipts);
vm_pool::recycle_vm_memory(mem::take(vm.memory_mut()));
let changes = sub_block_db_commit.into_changes();
storage_tx.commit_changes(changes)?;
} else {
vm_pool::recycle_vm_memory(mem::take(vm.memory_mut()));
}

self.update_tx_outputs(storage_tx, tx_id, &mut tx)?;
Expand Down
3 changes: 2 additions & 1 deletion crates/services/executor/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
#![deny(clippy::arithmetic_side_effects)]
#![deny(clippy::cast_possible_truncation)]
#![deny(unused_crate_dependencies)]
#![deny(warnings)]
// #![deny(warnings)]

pub mod executor;
pub mod ports;
pub mod refs;
mod vm_pool;

#[cfg(test)]
fuel_core_trace::enable_tracing!();
39 changes: 39 additions & 0 deletions crates/services/executor/src/vm_pool.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use std::cell::RefCell;

use fuel_core_types::fuel_vm::interpreter::Memory;

thread_local! {
static POOL: RefCell<Vec<Memory>> = RefCell::new(Vec::new());
}

/// Gets a new VM memory instance from the pool.
pub(crate) fn get_vm_memory() -> Memory {
POOL.with(|pool| {
let mut pool = pool.borrow_mut();
pool.pop().unwrap_or_else(Memory::new)
Copy link
Member

@Voxelot Voxelot May 8, 2024

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.

Copy link
Member Author

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:

  1. The threads are not CPU-pinned anyway
  2. 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

})
}

/// Recycles a VM memory instance back into the pool.
pub(crate) fn recycle_vm_memory(mut mem: Memory) {
POOL.with(|pool| {
mem.reset();
let mut pool = pool.borrow_mut();
pool.push(mem);
})
}

#[test]
fn test_vm_pool() {
let mut mem = get_vm_memory();
mem.grow_stack(1024).expect("Unable to grow stack");
mem.write_bytes_noownerchecks(0, [1, 2, 3, 4])
.expect("Unable to write stack");
let ptr1 = mem.stack_raw() as *const _ as *const u8 as usize;
recycle_vm_memory(mem);

// Make sure we get the same memory allocation back
let mem = get_vm_memory();
let ptr2 = mem.stack_raw() as *const _ as *const u8 as usize;
assert_eq!(ptr1, ptr2);
}
Loading