diff --git a/soroban-env-host/benches/common/cost_types/mod.rs b/soroban-env-host/benches/common/cost_types/mod.rs index 94b72b715..32a7aed47 100644 --- a/soroban-env-host/benches/common/cost_types/mod.rs +++ b/soroban-env-host/benches/common/cost_types/mod.rs @@ -12,7 +12,6 @@ mod num_ops; mod recover_ecdsa_secp256k1_key; mod val_deser; mod val_ser; -mod val_xdr_conv; mod vec_ops; mod verify_ed25519_sig; mod visit_object; @@ -33,7 +32,6 @@ pub(crate) use num_ops::*; pub(crate) use recover_ecdsa_secp256k1_key::*; pub(crate) use val_deser::*; pub(crate) use val_ser::*; -pub(crate) use val_xdr_conv::*; pub(crate) use vec_ops::*; pub(crate) use verify_ed25519_sig::*; pub(crate) use visit_object::*; diff --git a/soroban-env-host/benches/common/cost_types/val_xdr_conv.rs b/soroban-env-host/benches/common/cost_types/val_xdr_conv.rs deleted file mode 100644 index 3e7ef3563..000000000 --- a/soroban-env-host/benches/common/cost_types/val_xdr_conv.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::common::HostCostMeasurement; -use rand::{rngs::StdRng, RngCore}; -use soroban_env_host::{cost_runner::ValXdrConvRun, xdr::ScVal, Host, Val}; - -pub(crate) struct ValXdrConvMeasure; - -// This measures the costs of conversion of one non-object Val type to and from XDR. -impl HostCostMeasurement for ValXdrConvMeasure { - type Runner = ValXdrConvRun; - - fn new_random_case(_host: &Host, rng: &mut StdRng, _input: u64) -> (Option, ScVal) { - let v = rng.next_u32(); - let rv: Val = v.into(); - let scv: ScVal = ScVal::U32(v); - (Some(rv), scv) - } -} diff --git a/soroban-env-host/benches/common/mod.rs b/soroban-env-host/benches/common/mod.rs index 772ae9b0b..e8c852d54 100644 --- a/soroban-env-host/benches/common/mod.rs +++ b/soroban-env-host/benches/common/mod.rs @@ -73,7 +73,6 @@ pub(crate) fn for_each_host_cost_measurement( call_bench::(&mut params)?; call_bench::(&mut params)?; call_bench::(&mut params)?; - call_bench::(&mut params)?; call_bench::(&mut params)?; call_bench::(&mut params)?; call_bench::(&mut params)?; diff --git a/soroban-env-host/src/cost_runner/cost_types/mod.rs b/soroban-env-host/src/cost_runner/cost_types/mod.rs index c9134e696..8c641699b 100644 --- a/soroban-env-host/src/cost_runner/cost_types/mod.rs +++ b/soroban-env-host/src/cost_runner/cost_types/mod.rs @@ -12,7 +12,6 @@ mod num_ops; mod recover_ecdsa_secp256k1_key; mod val_deser; mod val_ser; -mod val_xdr_conv; mod vec_ops; mod verify_ed25519_sig; mod visit_object; @@ -33,7 +32,6 @@ pub use num_ops::*; pub use recover_ecdsa_secp256k1_key::*; pub use val_deser::*; pub use val_ser::*; -pub use val_xdr_conv::*; pub use vec_ops::*; pub use verify_ed25519_sig::*; pub use visit_object::*; diff --git a/soroban-env-host/src/cost_runner/cost_types/val_xdr_conv.rs b/soroban-env-host/src/cost_runner/cost_types/val_xdr_conv.rs deleted file mode 100644 index a740cbeac..000000000 --- a/soroban-env-host/src/cost_runner/cost_types/val_xdr_conv.rs +++ /dev/null @@ -1,28 +0,0 @@ -use std::hint::black_box; - -use crate::{cost_runner::CostRunner, xdr::ContractCostType, xdr::ScVal, Val}; - -pub struct ValXdrConvRun; - -impl CostRunner for ValXdrConvRun { - const COST_TYPE: ContractCostType = ContractCostType::ValXdrConv; - - type SampleType = (Option, ScVal); - - type RecycledType = (Option<(ScVal, Val)>, ScVal); - - fn run_iter(host: &crate::Host, _iter: u64, sample: Self::SampleType) -> Self::RecycledType { - let sv = black_box(host.from_host_val(sample.0.unwrap()).unwrap()); - let rv = black_box(host.to_host_val(&sample.1).unwrap()); - (Some((sv, rv)), sample.1) - } - - fn run_baseline_iter( - host: &crate::Host, - _iter: u64, - sample: Self::SampleType, - ) -> Self::RecycledType { - black_box(host.charge_budget(Self::COST_TYPE, None).unwrap()); - black_box((None, sample.1)) - } -} diff --git a/soroban-env-host/src/host/conversion.rs b/soroban-env-host/src/host/conversion.rs index 2e83834eb..47cbf5c9d 100644 --- a/soroban-env-host/src/host/conversion.rs +++ b/soroban-env-host/src/host/conversion.rs @@ -1,6 +1,6 @@ use std::rc::Rc; -use super::metered_clone::{self, MeteredClone, MeteredContainer, MeteredIterator}; +use super::metered_clone::{charge_shallow_copy, MeteredClone, MeteredContainer, MeteredIterator}; use crate::budget::AsBudget; use crate::err; use crate::host_object::{HostMap, HostObject, HostVec}; @@ -324,7 +324,7 @@ impl Host { } pub(crate) fn host_map_to_scmap(&self, map: &HostMap) -> Result { - metered_clone::charge_heap_alloc::(map.len() as u64, self.as_budget())?; + Vec::::charge_bulk_init_cpy(map.len() as u64, self.as_budget())?; let mut mv = Vec::with_capacity(map.len()); for (k, v) in map.iter(self)? { let key = self.from_host_val(*k)?; @@ -365,15 +365,11 @@ impl<'a> Convert, Object> for Host { impl Host { pub(crate) fn from_host_val(&self, val: Val) -> Result { - // Charges a single unit to for the Val -> ScVal conversion. - // The actual conversion logic occurs in the `common` crate, which - // translates a u64 into another form defined by the xdr. - // For an `Object`, the actual structural conversion (such as byte - // cloning) occurs in `from_host_obj` and is metered there. // This is the depth limit checkpoint for `Val`->`ScVal` conversion. + // Metering of val conversion happens only if an object is encountered, + // and is done inside `from_host_obj`. let _span = tracy_span!("Val to ScVal"); self.budget_cloned().with_limited_depth(|_| { - self.charge_budget(ContractCostType::ValXdrConv, None)?; ScVal::try_from_val(self, &val).map_err(|_| { self.err( ScErrorType::Value, @@ -388,8 +384,9 @@ impl Host { pub(crate) fn to_host_val(&self, v: &ScVal) -> Result { let _span = tracy_span!("ScVal to Val"); // This is the depth limit checkpoint for `ScVal`->`Val` conversion. + // Metering of val conversion happens only if an object is encountered, + // and is done inside `to_host_obj`. self.budget_cloned().with_limited_depth(|_| { - self.charge_budget(ContractCostType::ValXdrConv, None)?; v.try_into_val(self).map_err(|_| { self.err( ScErrorType::Value, @@ -405,12 +402,6 @@ impl Host { unsafe { let objref: Object = ob.into(); self.unchecked_visit_val_obj(objref, |ob| { - // This accounts for conversion of "primitive" objects (e.g U64) - // and the "shell" of a complex object (ScMap). Any non-trivial - // work such as byte cloning, has to be accounted for and - // metered in indivial match arms. - // `ValXdrConv` is const cost in both cpu and mem. The input=0 will be ignored. - self.charge_budget(ContractCostType::ValXdrConv, None)?; let val = match ob { None => { return Err(self.err( @@ -422,10 +413,7 @@ impl Host { } Some(ho) => match ho { HostObject::Vec(vv) => { - metered_clone::charge_heap_alloc::( - vv.len() as u64, - self.as_budget(), - )?; + Vec::::charge_bulk_init_cpy(vv.len() as u64, self.as_budget())?; let sv = vv.iter().map(|e| self.from_host_val(*e)).collect::, HostError, @@ -434,23 +422,36 @@ impl Host { ScVal::Vec(Some(ScVec(self.map_err(sv.try_into())?))) } HostObject::Map(mm) => ScVal::Map(Some(self.host_map_to_scmap(mm)?)), - HostObject::U64(u) => ScVal::U64(*u), - HostObject::I64(i) => ScVal::I64(*i), + HostObject::U64(u) => { + charge_shallow_copy::(1, self.as_budget())?; + ScVal::U64(*u) + } + HostObject::I64(i) => { + charge_shallow_copy::(1, self.as_budget())?; + ScVal::I64(*i) + } HostObject::TimePoint(tp) => { ScVal::Timepoint(tp.metered_clone(self.as_budget())?) } HostObject::Duration(d) => { ScVal::Duration(d.metered_clone(self.as_budget())?) } - HostObject::U128(u) => ScVal::U128(UInt128Parts { - hi: int128_helpers::u128_hi(*u), - lo: int128_helpers::u128_lo(*u), - }), - HostObject::I128(i) => ScVal::I128(Int128Parts { - hi: int128_helpers::i128_hi(*i), - lo: int128_helpers::i128_lo(*i), - }), + HostObject::U128(u) => { + charge_shallow_copy::(1, self.as_budget())?; + ScVal::U128(UInt128Parts { + hi: int128_helpers::u128_hi(*u), + lo: int128_helpers::u128_lo(*u), + }) + } + HostObject::I128(i) => { + charge_shallow_copy::(1, self.as_budget())?; + ScVal::I128(Int128Parts { + hi: int128_helpers::i128_hi(*i), + lo: int128_helpers::i128_lo(*i), + }) + } HostObject::U256(u) => { + charge_shallow_copy::(2, self.as_budget())?; let (hi_hi, hi_lo, lo_hi, lo_lo) = u256_into_pieces(*u); ScVal::U256(UInt256Parts { hi_hi, @@ -460,6 +461,7 @@ impl Host { }) } HostObject::I256(i) => { + charge_shallow_copy::(2, self.as_budget())?; let (hi_hi, hi_lo, lo_hi, lo_lo) = i256_into_pieces(*i); ScVal::I256(Int256Parts { hi_hi, @@ -482,12 +484,10 @@ impl Host { } pub(crate) fn to_host_obj(&self, ob: &ScValObjRef<'_>) -> Result { - // `ValXdrConv` is const cost in both cpu and mem. The input=0 will be ignored. - self.charge_budget(ContractCostType::ValXdrConv, None)?; let val: &ScVal = (*ob).into(); match val { ScVal::Vec(Some(v)) => { - metered_clone::charge_heap_alloc::(v.len() as u64, self.as_budget())?; + Vec::::charge_bulk_init_cpy(v.len() as u64, self.as_budget())?; let mut vv = Vec::with_capacity(v.len()); for e in v.iter() { vv.push(self.to_host_val(e)?) @@ -495,7 +495,7 @@ impl Host { Ok(self.add_host_object(HostVec::from_vec(vv)?)?.into()) } ScVal::Map(Some(m)) => { - metered_clone::charge_heap_alloc::<(Val, Val)>(m.len() as u64, self.as_budget())?; + Vec::<(Val, Val)>::charge_bulk_init_cpy(m.len() as u64, self.as_budget())?; let mut mm = Vec::with_capacity(m.len()); for pair in m.iter() { let k = self.to_host_val(&pair.key)?; @@ -516,26 +516,44 @@ impl Host { "map body missing", &[], )), - ScVal::U64(u) => Ok(self.add_host_object(*u)?.into()), - ScVal::I64(i) => Ok(self.add_host_object(*i)?.into()), + ScVal::U64(u) => { + charge_shallow_copy::(1, self.as_budget())?; + Ok(self.add_host_object(*u)?.into()) + } + ScVal::I64(i) => { + charge_shallow_copy::(1, self.as_budget())?; + Ok(self.add_host_object(*i)?.into()) + } ScVal::Timepoint(t) => Ok(self .add_host_object(t.metered_clone(self.as_budget())?)? .into()), ScVal::Duration(d) => Ok(self .add_host_object(d.metered_clone(self.as_budget())?)? .into()), - ScVal::U128(u) => Ok(self - .add_host_object(int128_helpers::u128_from_pieces(u.hi, u.lo))? - .into()), - ScVal::I128(i) => Ok(self - .add_host_object(int128_helpers::i128_from_pieces(i.hi, i.lo))? - .into()), - ScVal::U256(u) => Ok(self - .add_host_object(u256_from_pieces(u.hi_hi, u.hi_lo, u.lo_hi, u.lo_lo))? - .into()), - ScVal::I256(i) => Ok(self - .add_host_object(i256_from_pieces(i.hi_hi, i.hi_lo, i.lo_hi, i.lo_lo))? - .into()), + ScVal::U128(u) => { + charge_shallow_copy::(1, self.as_budget())?; + Ok(self + .add_host_object(int128_helpers::u128_from_pieces(u.hi, u.lo))? + .into()) + } + ScVal::I128(i) => { + charge_shallow_copy::(1, self.as_budget())?; + Ok(self + .add_host_object(int128_helpers::i128_from_pieces(i.hi, i.lo))? + .into()) + } + ScVal::U256(u) => { + charge_shallow_copy::(2, self.as_budget())?; + Ok(self + .add_host_object(u256_from_pieces(u.hi_hi, u.hi_lo, u.lo_hi, u.lo_lo))? + .into()) + } + ScVal::I256(i) => { + charge_shallow_copy::(2, self.as_budget())?; + Ok(self + .add_host_object(i256_from_pieces(i.hi_hi, i.hi_lo, i.lo_hi, i.lo_lo))? + .into()) + } ScVal::Bytes(b) => Ok(self .add_host_object(b.metered_clone(self.as_budget())?)? .into()), diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs index 2da40c6d5..8f7e3f007 100644 --- a/soroban-env-host/src/test/budget_metering.rs +++ b/soroban-env-host/src/test/budget_metering.rs @@ -13,7 +13,7 @@ use soroban_test_wasms::VEC; fn xdr_object_conversion() -> Result<(), HostError> { let host = Host::test_host() .test_budget(100_000, 100_000) - .enable_model(ContractCostType::ValXdrConv, 10, 0, 1, 0); + .enable_model(ContractCostType::HostMemCpy, 1, 0, 1, 0); let scmap: ScMap = host.map_err( vec![ ScMapEntry { @@ -28,16 +28,14 @@ fn xdr_object_conversion() -> Result<(), HostError> { .try_into(), )?; host.to_host_val(&ScVal::Map(Some(scmap)))?; - host.with_budget(|budget| { - // NB: It might seem like this should be 5 rather han 6 - // but due to the fact that one can convert an "object" or - // a "value" on separate paths that both need metering, - // we wind up double-counting the conversion of "objects". - // Possibly this should be improved in the future. - assert_eq!(budget.get_tracker(ContractCostType::ValXdrConv)?.0, 6); - assert_eq!(budget.get_cpu_insns_consumed()?, 60); - assert_eq!(budget.get_mem_bytes_consumed()?, 6); + // 2 iterations, 1 for the vec cpy, 1 for the bulk bytes cpy + assert_eq!(budget.get_tracker(ContractCostType::HostMemCpy)?.0, 2); + // 72 bytes copied for the ScVal->Val conversion: 24 (Vec bytes) + 2 (map entries) x (8 (padding bytes) + 8 (key bytes) + 8 (val bytes)) + assert_eq!( + budget.get_tracker(ContractCostType::HostMemCpy)?.1, + Some(72) + ); Ok(()) })?; Ok(())