Fixed benchmarks for the 0.25.3 (#1870)

Closes #1807 The PR updates benchmarks to work properly with new VM rules and fixes the issue with non-optimal `InMemoryTransaction` behavior for the `size_of_value` method. Because of the new VM rules: the memory that we are accessing should be initialized. The `cfe` opcode in the benchmarks expands the stack to allocate the memory that will later be used by the instruction that we are benchmarking. ### Before requesting review - [x] I have reviewed the code myself --------- Co-authored-by: Hannes Karppila <[email protected]> Co-authored-by: Brandon Vrooman <[email protected]>
FuelLabs · Apr 30, 2024 · f12ee07 · f12ee07
1 parent cd88ecd
commit f12ee07
Show file tree

Hide file tree

Showing 14 changed files with 177 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [#1866](https://github.com/FuelLabs/fuel-core/pull/1866): Fixed a runtime panic that occurred when restarting a node. The panic happens when the relayer database is already populated, and the relayer attempts an empty commit during start up. This invalid commit is removed in this PR.
 - [#1871](https://github.com/FuelLabs/fuel-core/pull/1871): Fixed `block` endpoint to return fetch the blocks from both databases after regenesis.
 - [#1856](https://github.com/FuelLabs/fuel-core/pull/1856): Replaced instances of `Union` with `Enum` for GraphQL definitions of `ConsensusParametersVersion` and related types. This is needed because `Union` does not support multiple `Version`s inside discriminants or empty variants. 
+- [#1870](https://github.com/FuelLabs/fuel-core/pull/1870): Fixed benchmarks for the `0.25.3`. 
+- [#1870](https://github.com/FuelLabs/fuel-core/pull/1870): Improves the performance of getting the size of the contract from the `InMemoryTransaction`.
 - [#1851](https://github.com/FuelLabs/fuel-core/pull/1851/): Provided migration capabilities (enabled addition of new column families) to RocksDB instance.
 
 ### Added 

diff --git a/benches/benches/block_target_gas_set/crypto.rs b/benches/benches/block_target_gas_set/crypto.rs
@@ -144,6 +144,7 @@ pub fn run_crypto(group: &mut BenchmarkGroup<WallTime>) {
                 op::movi(0x11, 32),
                 op::aloc(0x11),
                 op::movi(0x10, i),
+                op::cfe(0x10),
                 op::s256(RegId::HP, RegId::ZERO, 0x10),
                 op::jmpb(RegId::ZERO, 0),
             ]
@@ -161,6 +162,7 @@ pub fn run_crypto(group: &mut BenchmarkGroup<WallTime>) {
                 op::movi(0x11, 32),
                 op::aloc(0x11),
                 op::movi(0x10, i),
+                op::cfe(0x10),
                 op::k256(RegId::HP, RegId::ZERO, 0x10),
                 op::jmpb(RegId::ZERO, 0),
             ]

diff --git a/benches/benches/vm.rs b/benches/benches/vm.rs
@@ -89,3 +89,14 @@ fn vm(c: &mut Criterion) {
 
 criterion_group!(benches, vm);
 criterion_main!(benches);
+
+// If you want to debug the benchmarks, you can run them with code below:
+// But first you need to comment `criterion_group` and `criterion_main` macros above.
+//
+// fn main() {
+//     let mut criterio = Criterion::default();
+//     blockchain::run(&mut criterio);
+// }
+//
+// #[test]
+// fn dummy() {}
diff --git a/benches/benches/vm_initialization.rs b/benches/benches/vm_initialization.rs
@@ -3,6 +3,7 @@ use criterion::{
     Criterion,
     Throughput,
 };
+use fuel_core_storage::InterpreterStorage;
 use fuel_core_types::{
     fuel_asm::{
         op,
@@ -22,7 +23,10 @@ use fuel_core_types::{
         checked_transaction::{
             Checked,
             IntoChecked,
+            Ready,
         },
+        constraints::reg_key::Reg,
+        consts::VM_MAX_RAM,
         interpreter::NotSupportedEcal,
         Interpreter,
     },
@@ -75,36 +79,76 @@ fn transaction<R: Rng>(
 
 pub fn vm_initialization(c: &mut Criterion) {
     let mut rng = StdRng::seed_from_u64(8586);
-
+    let consensus_params = ConsensusParameters::default();
     let mut group = c.benchmark_group("vm_initialization");
 
-    let consensus_params = ConsensusParameters::default();
-    let mut i = 5usize;
-    loop {
+    // Increase the size of the script to measure the performance of the VM initialization
+    // with a large script. THe largest allowed script is 64 KB = 2^16 bytes.
+    const TX_SIZE_POWER_OF_TWO: usize = 16;
+
+    for i in 5..=TX_SIZE_POWER_OF_TWO {
         let size = 8 * (1 << i);
-        if size as u64 > consensus_params.script_params().max_script_data_length() {
-            break
-        }
+
         let script = vec![op::ret(1); size / Instruction::SIZE]
             .into_iter()
             .collect();
         let script_data = vec![255; size];
         let tx = transaction(&mut rng, script, script_data, &consensus_params);
         let tx_size = tx.transaction().size();
+        let tx = tx.test_into_ready();
+
         let name = format!("vm_initialization_with_tx_size_{}", tx_size);
         group.throughput(Throughput::Bytes(tx_size as u64));
         group.bench_function(name, |b| {
             b.iter(|| {
-                let mut vm = black_box(
-                    Interpreter::<_, Script, NotSupportedEcal>::with_memory_storage(),
-                );
-                let ready_tx = tx.clone().test_into_ready();
-                black_box(vm.init_script(ready_tx))
-                    .expect("Should be able to execute transaction");
+                unoptimized_vm_initialization_with_allocating_full_range_of_memory(&tx);
             })
         });
-        i += 1;
     }
 
     group.finish();
 }
+
+fn unoptimized_vm_initialization_with_allocating_full_range_of_memory(
+    ready_tx: &Ready<Script>,
+) {
+    let vm = black_box(Interpreter::<_, Script, NotSupportedEcal>::with_memory_storage());
+
+    black_box(initialize_vm_with_allocated_full_range_of_memory(
+        black_box(ready_tx.clone()),
+        vm,
+    ));
+}
+
+fn initialize_vm_with_allocated_full_range_of_memory<S>(
+    ready_tx: Ready<Script>,
+    mut vm: Interpreter<S, Script>,
+) -> Interpreter<S, Script>
+where
+    S: InterpreterStorage,
+{
+    vm.init_script(ready_tx)
+        .expect("Should be able to execute transaction");
+
+    const POWER_OF_TWO_OF_HALF_VM: u64 = 25;
+    const VM_MEM_HALF: u64 = 1 << POWER_OF_TWO_OF_HALF_VM;
+    assert_eq!(VM_MEM_HALF, VM_MAX_RAM / 2);
+
+    for i in 0..=POWER_OF_TWO_OF_HALF_VM {
+        let stack = 1 << i;
+        let heap = VM_MAX_RAM - stack;
+
+        vm.memory_mut()
+            .grow_stack(stack)
+            .expect("Should be able to grow stack");
+        vm.memory_mut()
+            .grow_heap(Reg::new(&0), heap)
+            .expect("Should be able to grow heap");
+    }
+
+    vm.memory_mut()
+        .grow_heap(Reg::new(&0), 0)
+        .expect("Should be able to grow heap");
+
+    vm
+}
diff --git a/benches/benches/vm_set/blockchain.rs b/benches/benches/vm_set/blockchain.rs
@@ -265,6 +265,8 @@ pub fn run(c: &mut Criterion) {
             op::addi(0x11, 0x11, WORD_SIZE.try_into().unwrap()),
             op::addi(0x11, 0x11, AssetId::LEN.try_into().unwrap()),
             op::movi(0x12, i as u32),
+            // Allocate space for storage values in the memory
+            op::cfei(i as u32 * Bytes32::LEN as u32),
         ];
         let mut bench = VmBench::contract_using_db(
             rng,
@@ -393,6 +395,7 @@ pub fn run(c: &mut Criterion) {
             op::addi(0x11, 0x11, WORD_SIZE.try_into().unwrap()),
             op::movi(0x12, 100_000),
             op::movi(0x13, i.try_into().unwrap()),
+            op::cfe(0x13),
             op::movi(0x14, i.try_into().unwrap()),
             op::movi(0x15, i.try_into().unwrap()),
             op::add(0x15, 0x15, 0x15),

diff --git a/benches/benches/vm_set/crypto.rs b/benches/benches/vm_set/crypto.rs
@@ -82,7 +82,12 @@ pub fn run(c: &mut Criterion) {
             &mut bench_k256,
             format!("{i}"),
             VmBench::new(op::k256(RegId::HP, RegId::ZERO, 0x10)).with_prepare_script(
-                vec![op::movi(0x11, 32), op::aloc(0x11), op::movi(0x10, *i)],
+                vec![
+                    op::movi(0x11, 32),
+                    op::aloc(0x11),
+                    op::movi(0x10, *i),
+                    op::cfe(0x10),
+                ],
             ),
         );
     }
@@ -95,7 +100,12 @@ pub fn run(c: &mut Criterion) {
             &mut bench_s256,
             format!("{i}"),
             VmBench::new(op::s256(RegId::HP, RegId::ZERO, 0x10)).with_prepare_script(
-                vec![op::movi(0x11, 32), op::aloc(0x11), op::movi(0x10, *i)],
+                vec![
+                    op::movi(0x11, 32),
+                    op::aloc(0x11),
+                    op::movi(0x10, *i),
+                    op::cfe(0x10),
+                ],
             ),
         );
     }

diff --git a/benches/benches/vm_set/flow.rs b/benches/benches/vm_set/flow.rs
@@ -70,7 +70,7 @@ pub fn run(c: &mut Criterion) {
             format!("{i}"),
             VmBench::contract(rng, op::retd(RegId::ONE, 0x10))
                 .unwrap()
-                .with_post_call(vec![op::movi(0x10, *i)])
+                .with_post_call(vec![op::movi(0x10, *i), op::cfe(0x10)])
                 .with_call_receipts(receipts_ctx.clone()),
         );
     }
@@ -84,7 +84,7 @@ pub fn run(c: &mut Criterion) {
             format!("{i}"),
             VmBench::contract(rng, op::retd(RegId::ONE, 0x10))
                 .unwrap()
-                .with_post_call(vec![op::movi(0x10, *i)])
+                .with_post_call(vec![op::movi(0x10, *i), op::cfe(0x10)])
                 .with_call_receipts(receipts_ctx.clone()),
         );
     }
@@ -118,7 +118,7 @@ pub fn run(c: &mut Criterion) {
             &mut logd,
             format!("{i}"),
             VmBench::new(op::logd(0x10, 0x11, RegId::ZERO, 0x13))
-                .with_prepare_script(vec![op::movi(0x13, *i)])
+                .with_prepare_script(vec![op::movi(0x13, *i), op::cfe(0x13)])
                 .with_call_receipts(receipts_ctx.clone()),
         );
     }

diff --git a/benches/benches/vm_set/mem.rs b/benches/benches/vm_set/mem.rs
@@ -93,6 +93,7 @@ pub fn run(c: &mut Criterion) {
             VmBench::new(op::mcp(0x10, RegId::ZERO, 0x11)).with_prepare_script(vec![
                 op::movi(0x11, *i),
                 op::aloc(0x11),
+                op::cfe(0x11),
                 op::move_(0x10, RegId::HP),
             ]),
         );
@@ -117,6 +118,7 @@ pub fn run(c: &mut Criterion) {
                 vec![
                     op::movi(0x11, *i),
                     op::aloc(0x11),
+                    op::cfe(0x11),
                     op::move_(0x10, RegId::HP),
                 ],
             ),
@@ -132,6 +134,7 @@ pub fn run(c: &mut Criterion) {
         let mut prepare_script =
             vec![op::move_(0x11, RegId::ZERO), op::move_(0x12, RegId::ZERO)];
         prepare_script.extend(set_full_word(0x13, i));
+        prepare_script.extend(vec![op::cfe(0x13)]);
 
         run_group_ref(
             &mut mem_meq,

diff --git a/benches/src/lib.rs b/benches/src/lib.rs
@@ -1,7 +1,8 @@
-pub mod default_gas_costs;
-pub mod import;
-
-pub use fuel_core_storage::vm_storage::VmStorage;
+use fuel_core::database::GenesisDatabase;
+use fuel_core_storage::transactional::{
+    IntoTransaction,
+    StorageTransaction,
+};
 use fuel_core_types::{
     fuel_asm::{
         op,
@@ -29,11 +30,16 @@ use fuel_core_types::{
         *,
     },
 };
+use std::{
+    iter,
+    mem,
+};
 
-use fuel_core::database::GenesisDatabase;
-use fuel_core_storage::transactional::StorageTransaction;
+pub mod default_gas_costs;
+pub mod import;
+
+pub use fuel_core_storage::vm_storage::VmStorage;
 pub use rand::Rng;
-use std::iter;
 
 const LARGE_GAS_LIMIT: u64 = u64::MAX - 1001;
 
@@ -424,6 +430,9 @@ impl TryFrom<VmBench> for VmBenchPrepared {
 
             db.deploy_contract_with_id(&[], &Contract::default(), &contract_id)?;
         }
+        let transaction = mem::take(db.database_mut());
+        let database = transaction.commit().expect("Failed to commit transaction");
+        *db.database_mut() = database.into_transaction();
 
         inputs.into_iter().for_each(|i| {
             tx.add_input(i);

diff --git a/bin/fuel-core/chainspec/dev-testnet/state_transition_bytecode.wasm b/bin/fuel-core/chainspec/dev-testnet/state_transition_bytecode.wasm
diff --git a/bin/fuel-core/chainspec/testnet/state_transition_bytecode.wasm b/bin/fuel-core/chainspec/testnet/state_transition_bytecode.wasm
diff --git a/crates/fuel-core/src/database.rs b/crates/fuel-core/src/database.rs
@@ -494,7 +494,7 @@ where
             // all next commits should be linked(the height should increase each time by one).
         }
         (Some(prev_height), None) => {
-            // In production, we shouldn't have cases where we call `commit_chagnes` with intermediate changes.
+            // In production, we shouldn't have cases where we call `commit_changes` with intermediate changes.
             // The commit always should contain all data for the corresponding height.
             return Err(DatabaseError::NewHeightIsNotSet {
                 prev_height: prev_height.as_u64(),

diff --git a/crates/storage/src/transactional.rs b/crates/storage/src/transactional.rs
@@ -227,20 +227,54 @@ impl<Storage> Modifiable for InMemoryTransaction<Storage> {
     }
 }
 
+impl<Column, S> InMemoryTransaction<S>
+where
+    Column: StorageColumn,
+    S: KeyValueInspect<Column = Column>,
+{
+    fn get_from_changes(&self, key: &[u8], column: Column) -> Option<&WriteOperation> {
+        let k = key.to_vec();
+        self.changes
+            .get(&column.id())
+            .and_then(|btree| btree.get(&k))
+    }
+}
+
 impl<Column, S> KeyValueInspect for InMemoryTransaction<S>
 where
     Column: StorageColumn,
     S: KeyValueInspect<Column = Column>,
 {
     type Column = Column;
 
+    fn exists(&self, key: &[u8], column: Self::Column) -> StorageResult<bool> {
+        if let Some(operation) = self.get_from_changes(key, column) {
+            match operation {
+                WriteOperation::Insert(_) => Ok(true),
+                WriteOperation::Remove => Ok(false),
+            }
+        } else {
+            self.storage.exists(key, column)
+        }
+    }
+
+    fn size_of_value(
+        &self,
+        key: &[u8],
+        column: Self::Column,
+    ) -> StorageResult<Option<usize>> {
+        if let Some(operation) = self.get_from_changes(key, column) {
+            match operation {
+                WriteOperation::Insert(value) => Ok(Some(value.len())),
+                WriteOperation::Remove => Ok(None),
+            }
+        } else {
+            self.storage.size_of_value(key, column)
+        }
+    }
+
     fn get(&self, key: &[u8], column: Self::Column) -> StorageResult<Option<Value>> {
-        let k = key.to_vec();
-        if let Some(operation) = self
-            .changes
-            .get(&column.id())
-            .and_then(|btree| btree.get(&k))
-        {
+        if let Some(operation) = self.get_from_changes(key, column) {
             match operation {
                 WriteOperation::Insert(value) => Ok(Some(value.clone())),
                 WriteOperation::Remove => Ok(None),
@@ -249,6 +283,31 @@ where
             self.storage.get(key, column)
         }
     }
+
+    fn read(
+        &self,
+        key: &[u8],
+        column: Self::Column,
+        buf: &mut [u8],
+    ) -> StorageResult<Option<usize>> {
+        if let Some(operation) = self.get_from_changes(key, column) {
+            match operation {
+                WriteOperation::Insert(value) => {
+                    let read = value.len();
+                    if read != buf.len() {
+                        return Err(crate::Error::Other(anyhow::anyhow!(
+                            "Buffer size is not equal to the value size"
+                        )));
+                    }
+                    buf.copy_from_slice(value.as_ref());
+                    Ok(Some(read))
+                }
+                WriteOperation::Remove => Ok(None),
+            }
+        } else {
+            self.storage.read(key, column, buf)
+        }
+    }
 }
 
 impl<Column, S> KeyValueMutate for InMemoryTransaction<S>

diff --git a/crates/types/src/lib.rs b/crates/types/src/lib.rs
@@ -33,6 +33,7 @@ pub mod fuel_vm {
     #[doc(no_inline)]
     pub use fuel_vm_private::{
         checked_transaction,
+        constraints,
         consts,
         crypto,
         double_key,