Optimize interpreter::blockchain::{load_contract_code, code_copy} to read contract starting from offset

CHANGELOG.md

@@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [863](https://github.com/FuelLabs/fuel-vm/pull/863): Changed StorageRead::read to load a serialized value starting from a offset. The function returns an optional value equal to the number of bytes read when defined, or none if the offset specified in input is outside the boundaries of the serialized value read.
 - [868](https://github.com/FuelLabs/fuel-vm/pull/868): Fixed error message when having a nonexistent contract in inputs. Instead of saying "contract was in inputs, but doesn't exist", the message was just "contract not in inputs". Now there's a separate error for that.
 
+### Changed
+- [847](https://github.com/FuelLabs/fuel-vm/pull/847): Changed `interpreter::blockchain::load_contract_code` and `interpreter::blockchain::code_copy` to use the new version of `StorageRead::read` where the contract is loaded into a buffer starting from an offset. The contract is copied directly into the portion of memory starting at the destination address, rather than having to be copied indirectly after being fetched from storage.
+
 ## [Version 0.58.2]
 
 ### Fixed

fuel-vm/src/interpreter/blob.rs

@@ -2,13 +2,15 @@ use fuel_asm::{
     RegisterId,
     Word,
 };
-use fuel_storage::StorageSize;
 use fuel_tx::PanicReason;
 use fuel_types::BlobId;
 
 use crate::{
     error::IoResult,
-    prelude::*,
+    interpreter::{
+        contract::blob_size,
+        memory::copy_from_storage_zero_fill,
+    },
     storage::{
         BlobData,
         InterpreterStorage,
@@ -17,7 +19,6 @@ use crate::{
 
 use super::{
     internal::inc_pc,
-    memory::copy_from_slice_zero_fill,
     split_registers,
     GetRegMut,
     Interpreter,
@@ -46,9 +47,7 @@ where
 
         let blob_id = BlobId::from(self.memory.as_ref().read_bytes(blob_id_ptr)?);
 
-        let size = <S as StorageSize<BlobData>>::size_of_value(&self.storage, &blob_id)
-            .map_err(RuntimeError::Storage)?
-            .ok_or(PanicReason::BlobNotFound)?;
+        let size = blob_size(&self.storage, &blob_id)?;
 
         self.dependent_gas_charge_without_base(gas_cost, size as Word)?;
         let (SystemRegisters { pc, .. }, mut w) = split_registers(&mut self.registers);
@@ -74,23 +73,20 @@ where
         let blob_id = BlobId::from(self.memory.as_ref().read_bytes(blob_id_ptr)?);
         let owner = self.ownership_registers();
 
-        let size = <S as StorageSize<BlobData>>::size_of_value(&self.storage, &blob_id)
-            .map_err(RuntimeError::Storage)?
-            .ok_or(PanicReason::BlobNotFound)?;
-        self.dependent_gas_charge_without_base(gas_cost, len.max(size as Word))?;
-
-        let blob = <S as StorageInspect<BlobData>>::get(&self.storage, &blob_id)
-            .map_err(RuntimeError::Storage)?
-            .ok_or(PanicReason::BlobNotFound)?;
-        let blob = blob.as_ref().as_ref();
+        let blob_len = blob_size(&self.storage, &blob_id)?;
+        let charge_len = len.max(blob_len as Word);
+        self.dependent_gas_charge_without_base(gas_cost, charge_len)?;
 
-        copy_from_slice_zero_fill(
+        copy_from_storage_zero_fill::<BlobData, _>(
             self.memory.as_mut(),
             owner,
-            blob,
+            &self.storage,
             dst_ptr,
-            blob_offset,
             len,
+            &blob_id,
+            blob_offset,
+            blob_len,
+            PanicReason::BlobNotFound,
         )?;
 
         Ok(inc_pc(self.registers.pc_mut())?)

fuel-vm/src/interpreter/blockchain.rs

@@ -13,6 +13,7 @@ use crate::{
         contract::{
             balance,
             balance_decrease,
+            blob_size,
             contract_size,
         },
         gas::{
@@ -28,7 +29,7 @@ use crate::{
             tx_id,
         },
         memory::{
-            copy_from_slice_zero_fill,
+            copy_from_storage_zero_fill,
             OwnershipRegisters,
         },
         receipts::ReceiptsCtx,
@@ -53,10 +54,7 @@ use fuel_asm::{
     Imm06,
     PanicReason,
 };
-use fuel_storage::{
-    StorageInspect,
-    StorageSize,
-};
+use fuel_storage::StorageSize;
 use fuel_tx::{
     consts::BALANCE_ENTRY_SIZE,
     BlobId,
@@ -636,8 +634,6 @@ where
             self.gas_cost,
             charge_len,
         )?;
-        let contract = super::contract::contract(self.storage, &contract_id)?;
-        let contract_bytes = contract.as_ref().as_ref();
 
         let new_sp = ssp.saturating_add(length);
         self.memory.grow_stack(new_sp)?;
@@ -650,14 +646,16 @@ where
         *self.sp = new_sp;
         *self.ssp = new_sp;
 
-        // Copy the code.
-        copy_from_slice_zero_fill(
+        copy_from_storage_zero_fill::<ContractsRawCode, _>(
             self.memory,
             owner,
-            contract_bytes,
+            self.storage,
             region_start,
-            contract_offset,
             length,
+            &contract_id,
+            contract_offset,
+            contract_len,
+            PanicReason::ContractNotFound,
         )?;
 
         // Update frame code size, if we have a stack frame (i.e. fp > 0)
@@ -711,10 +709,7 @@ where
 
         let length = bytes::padded_len_word(length_unpadded).unwrap_or(Word::MAX);
 
-        let blob_len =
-            <S as StorageSize<BlobData>>::size_of_value(self.storage, &blob_id)
-                .map_err(RuntimeError::Storage)?
-                .ok_or(PanicReason::BlobNotFound)?;
+        let blob_len = blob_size(self.storage, &blob_id)?;
 
         // Fetch the storage blob
         let profiler = ProfileGas {
@@ -731,10 +726,6 @@ where
             self.gas_cost,
             charge_len,
         )?;
-        let blob = <S as StorageInspect<BlobData>>::get(self.storage, &blob_id)
-            .map_err(RuntimeError::Storage)?
-            .ok_or(PanicReason::BlobNotFound)?;
-        let blob_bytes = blob.as_ref().as_ref();
 
         let new_sp = ssp.saturating_add(length);
         self.memory.grow_stack(new_sp)?;
@@ -748,13 +739,16 @@ where
         *self.ssp = new_sp;
 
         // Copy the code.
-        copy_from_slice_zero_fill(
+        copy_from_storage_zero_fill::<BlobData, _>(
             self.memory,
             owner,
-            blob_bytes,
+            self.storage,
             region_start,
-            blob_offset,
             length,
+            &blob_id,
+            blob_offset,
+            blob_len,
+            PanicReason::BlobNotFound,
         )?;
 
         // Update frame code size, if we have a stack frame (i.e. fp > 0)
@@ -994,9 +988,7 @@ where
         self.memory.write(self.owner, dst_addr, length)?;
         self.input_contracts.check(&contract_id)?;
 
-        let contract = super::contract::contract(self.storage, &contract_id)?;
-        let contract_bytes = contract.as_ref().as_ref();
-        let contract_len = contract_bytes.len();
+        let contract_len = contract_size(&self.storage, &contract_id)?;
         let charge_len = core::cmp::max(contract_len as u64, length);
         let profiler = ProfileGas {
             pc: self.pc.as_ref(),
@@ -1012,14 +1004,16 @@ where
             charge_len,
         )?;
 
-        // Owner checks already performed above
-        copy_from_slice_zero_fill(
+        copy_from_storage_zero_fill::<ContractsRawCode, _>(
             self.memory,
             self.owner,
-            contract.as_ref().as_ref(),
+            self.storage,
             dst_addr,
-            contract_offset,
             length,
+            &contract_id,
+            contract_offset,
+            contract_len,
+            PanicReason::ContractNotFound,
         )?;
 
         Ok(inc_pc(self.pc)?)

fuel-vm/src/interpreter/contract.rs

@@ -32,6 +32,7 @@ use crate::{
     },
     prelude::Profiler,
     storage::{
+        BlobData,
         ContractsAssetsStorage,
         ContractsRawCode,
         InterpreterStorage,
@@ -44,19 +45,17 @@ use fuel_asm::{
 };
 use fuel_storage::StorageSize;
 use fuel_tx::{
-    Contract,
     Output,
     Receipt,
 };
 use fuel_types::{
     Address,
     AssetId,
+    BlobId,
     Bytes32,
     ContractId,
 };
 
-use alloc::borrow::Cow;
-
 impl<M, S, Tx, Ecal> Interpreter<M, S, Tx, Ecal>
 where
     M: Memory,
@@ -179,19 +178,6 @@ where
     }
 }
 
-pub(crate) fn contract<'s, S>(
-    storage: &'s S,
-    contract: &ContractId,
-) -> IoResult<Cow<'s, Contract>, S::DataError>
-where
-    S: InterpreterStorage,
-{
-    storage
-        .storage_contract(contract)
-        .map_err(RuntimeError::Storage)?
-        .ok_or_else(|| PanicReason::ContractNotFound.into())
-}
-
 struct ContractBalanceCtx<'vm, S> {
     storage: &'vm S,
     memory: &'vm mut MemoryInstance,
@@ -385,15 +371,26 @@ impl<'vm, S, Tx> TransferCtx<'vm, S, Tx> {
 pub(crate) fn contract_size<S>(
     storage: &S,
     contract: &ContractId,
-) -> IoResult<u32, S::Error>
+) -> IoResult<usize, S::Error>
 where
     S: StorageSize<ContractsRawCode> + ?Sized,
 {
     let size = storage
         .size_of_value(contract)
         .map_err(RuntimeError::Storage)?
         .ok_or(PanicReason::ContractNotFound)?;
-    Ok(u32::try_from(size).map_err(|_| PanicReason::MemoryOverflow)?)
+    Ok(size)
+}
+
+pub(crate) fn blob_size<S>(storage: &S, blob_id: &BlobId) -> IoResult<usize, S::Error>
+where
+    S: StorageSize<BlobData> + ?Sized,
+{
+    let size = storage
+        .size_of_value(blob_id)
+        .map_err(RuntimeError::Storage)?
+        .ok_or(PanicReason::BlobNotFound)?;
+    Ok(size)
 }
 
 pub(crate) fn balance<S>(

fuel-vm/src/interpreter/memory.rs

@@ -35,10 +35,18 @@ use core::ops::{
     RangeTo,
 };
 
+use crate::error::{
+    IoResult,
+    RuntimeError,
+};
 use alloc::{
     vec,
     vec::Vec,
 };
+use fuel_storage::{
+    Mappable,
+    StorageRead,
+};
 
 #[cfg(test)]
 mod tests;
@@ -1027,34 +1035,47 @@ impl OwnershipRegisters {
     }
 }
 
-/// Attempt copy from slice to memory, filling zero bytes when exceeding slice boundaries.
-/// Performs overflow and memory range checks, but no ownership checks.
+/// Attempt copy from the storage to memory, filling zero bytes when exceeding slice
+/// boundaries. Performs overflow and memory range checks, but no ownership checks.
 /// Note that if `src_offset` is larger than `src.len()`, the whole range will be
 /// zero-filled.
-pub(crate) fn copy_from_slice_zero_fill<A: ToAddr, B: ToAddr>(
+#[allow(clippy::too_many_arguments)]
+pub(crate) fn copy_from_storage_zero_fill<M, S>(
     memory: &mut MemoryInstance,
     owner: OwnershipRegisters,
-    src: &[u8],
-    dst_addr: A,
-    src_offset: Word,
-    len: B,
-) -> SimpleResult<()> {
-    let range = memory.write(owner, dst_addr, len)?;
+    storage: &S,
+    dst_addr: Word,
+    dst_len: Word,
+    src_id: &M::Key,
+    src_offset: u64,
+    src_len: usize,
+    no_found_error: PanicReason,
+) -> IoResult<(), S::Error>
+where
+    M: Mappable,
+    S: StorageRead<M>,
+{
+    let write_buffer = memory.write(owner, dst_addr, dst_len)?;
+    let mut empty_offset = 0;
 
-    // Special-case the ranges that are completely out of bounds,
-    // to avoid platform-dependenct usize conversion.
-    if src_offset >= src.len() as Word {
-        range[..].fill(0);
-    } else {
-        // Safety: since we check above that this is not larger than `src.len()`,
-        // which is `usize`, the cast never truncates.
-        #[allow(clippy::cast_possible_truncation)]
-        let src_offset = src_offset as usize;
-        let src_end = src_offset.saturating_add(range.len()).min(src.len());
-        let data = src.get(src_offset..src_end).unwrap_or(&[]);
-
-        range[..data.len()].copy_from_slice(data);
-        range[data.len()..].fill(0);
+    if src_offset < src_len as Word {
+        let src_offset =
+            u32::try_from(src_offset).map_err(|_| PanicReason::MemoryOverflow)?;
+
+        let src_read_length = src_len.saturating_sub(src_offset as usize);
+        let src_read_length = src_read_length.min(write_buffer.len());
+
+        let (src_read_buffer, _) = write_buffer.split_at_mut(src_read_length);
+        storage
+            .read(src_id, src_offset as usize, src_read_buffer)
+            .transpose()
+            .ok_or(no_found_error)?
+            .map_err(RuntimeError::Storage)?;
+
+        empty_offset = src_read_length;
     }
+
+    write_buffer[empty_offset..].fill(0);
+
     Ok(())
 }