From 6f2b26f407170f1e71cb2eecf041338fd0b6adba Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Wed, 28 Aug 2024 13:07:44 -0400 Subject: [PATCH 1/4] Rounding improvement for budget model with iterations --- soroban-env-host/src/budget/dimension.rs | 2 +- soroban-env-host/src/budget/model.rs | 54 ++++++++++++++++------ soroban-env-host/src/host/declared_size.rs | 2 +- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/soroban-env-host/src/budget/dimension.rs b/soroban-env-host/src/budget/dimension.rs index 516be287e..a6cd4f5d1 100644 --- a/soroban-env-host/src/budget/dimension.rs +++ b/soroban-env-host/src/budget/dimension.rs @@ -165,7 +165,7 @@ impl BudgetDimension { is_shadow: IsShadowMode, ) -> Result { let cm = self.get_cost_model(ty)?; - let amount = cm.evaluate(input)?.saturating_mul(iterations); + let amount = cm.evaluate(iterations, input)?; #[cfg(all(not(target_family = "wasm"), feature = "tracy"))] if _is_cpu.0 { diff --git a/soroban-env-host/src/budget/model.rs b/soroban-env-host/src/budget/model.rs index 4cda325f1..355b8e40f 100644 --- a/soroban-env-host/src/budget/model.rs +++ b/soroban-env-host/src/budget/model.rs @@ -6,13 +6,16 @@ use core::fmt::{Debug, Display}; /// We provide a "cost model" object that evaluates a linear expression: /// -/// f(x) = a + b * Option +/// f(x) = I * (a + b * Option) /// -/// Where a, b are "fixed" parameters at construction time (extracted from an -/// on-chain cost schedule, so technically not _totally_ fixed) and Option -/// is some abstract input variable -- say, event counts or object sizes -- -/// provided at runtime. If the input cannot be defined, i.e., the cost is -/// constant, input-independent, then pass in `None` as the input. +/// I is a simple scale factor that represents number of iterations the linear +/// model inside the parantesis is repeated. +/// +/// a, b are "fixed" parameters at construction time (extracted from an on-chain +/// cost schedule, so technically not _totally_ fixed) and Option is some +/// abstract input variable -- say, event counts or object sizes -- provided at +/// runtime. If the input cannot be defined, i.e., the cost is constant, +/// input-independent, then pass in `None` as the input. /// /// The same `CostModel` type, i.e. `CostType` (applied to different parameters /// and variables) is used for calculating memory as well as CPU time. @@ -22,10 +25,10 @@ use core::fmt::{Debug, Display}; /// sufficiently by a linear model and 2. they together encompass the vast /// majority of available operations done by the `env` -- the host and the VM. /// -/// The parameters for a `CostModel` are calibrated empirically. See this crate's -/// benchmarks for more details. +/// The parameters for a `CostModel` are calibrated empirically. See this +/// crate's benchmarks for more details. pub trait HostCostModel { - fn evaluate(&self, input: Option) -> Result; + fn evaluate(&self, iterations: u64, input: Option) -> Result; #[cfg(any(test, feature = "testutils", feature = "bench"))] fn reset(&mut self); @@ -110,14 +113,17 @@ impl TryFrom for MeteredCostComponent { } impl HostCostModel for MeteredCostComponent { - fn evaluate(&self, input: Option) -> Result { - let const_term = self.const_term; + fn evaluate(&self, iterations: u64, input: Option) -> Result { + let const_term = self.const_term.saturating_mul(iterations); match input { Some(input) => { let mut res = const_term; if !self.lin_term.is_zero() { - let lin_cost = self.lin_term.saturating_mul(input).unscale(); - res = res.saturating_add(lin_cost) + let lin_cost = self + .lin_term + .saturating_mul(input) + .saturating_mul(iterations); + res = res.saturating_add(lin_cost.unscale()) } Ok(res) } @@ -131,3 +137,25 @@ impl HostCostModel for MeteredCostComponent { self.lin_term = ScaledU64(0); } } + +mod test { + #[allow(unused)] + use super::{HostCostModel, MeteredCostComponent, ScaledU64}; + + #[test] + fn test_model_evaluation_with_rounding() { + let test_model = MeteredCostComponent { + const_term: 3, + lin_term: ScaledU64(5), + }; + // low iteration + low input + // the constant part is 3, the linear part is 5 >> 7 == 0, total is 3 + assert_eq!(3, test_model.evaluate(1, Some(1)).unwrap()); + // low iteration + high input + // the contant part is 3, the linear part is (26 * 5) >> 7 == 1, total is 4 + assert_eq!(4, test_model.evaluate(1, Some(26)).unwrap()); + // high iteration + low input + // the constant part is 26 * 3 == 78, the linear part is (26 * 5) >> 7 == 1, total is 79 + assert_eq!(79, test_model.evaluate(26, Some(1)).unwrap()); + } +} diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index b4bd542ea..245fd9000 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -412,7 +412,7 @@ mod test { #[cfg(target_arch = "x86_64")] expect!["512"].assert_eq(size_of::().to_string().as_str()); #[cfg(target_arch = "aarch64")] - expect!["496"].assert_eq(size_of::().to_string().as_str()); + expect!["488"].assert_eq(size_of::().to_string().as_str()); expect!["16"].assert_eq(size_of::
().to_string().as_str()); expect!["1"].assert_eq(size_of::().to_string().as_str()); From 6db9590ca2531da25443015a2fea66dcc44666f4 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Fri, 6 Sep 2024 12:50:18 -0400 Subject: [PATCH 2/4] Update soroban-env-host/src/budget/model.rs Co-authored-by: Siddharth Suresh --- soroban-env-host/src/budget/model.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/soroban-env-host/src/budget/model.rs b/soroban-env-host/src/budget/model.rs index 355b8e40f..5f6623aa9 100644 --- a/soroban-env-host/src/budget/model.rs +++ b/soroban-env-host/src/budget/model.rs @@ -9,7 +9,7 @@ use core::fmt::{Debug, Display}; /// f(x) = I * (a + b * Option) /// /// I is a simple scale factor that represents number of iterations the linear -/// model inside the parantesis is repeated. +/// model inside the parentheses is repeated. /// /// a, b are "fixed" parameters at construction time (extracted from an on-chain /// cost schedule, so technically not _totally_ fixed) and Option is some From 54044305cb55b15f883ebbc7ec2a477089a5f7b7 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Fri, 6 Sep 2024 15:12:08 -0400 Subject: [PATCH 3/4] gate the aarch64 size change by rust version --- soroban-env-host/src/host/declared_size.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index 245fd9000..51389765a 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -411,8 +411,19 @@ mod test { expect!["16"].assert_eq(size_of::().to_string().as_str()); #[cfg(target_arch = "x86_64")] expect!["512"].assert_eq(size_of::().to_string().as_str()); + + #[rustversion::before(1.79)] + #[cfg(target_arch = "aarch64")] + fn check_aarch64_size_that_changed_at_rust_1_79() { + expect!["496"].assert_eq(size_of::().to_string().as_str()); + } + #[rustversion::since(1.79)] #[cfg(target_arch = "aarch64")] - expect!["488"].assert_eq(size_of::().to_string().as_str()); + fn check_aarch64_size_that_changed_at_rust_1_79() { + expect!["488"].assert_eq(size_of::().to_string().as_str()); + } + check_aarch64_size_that_changed_at_rust_1_79(); + expect!["16"].assert_eq(size_of::
().to_string().as_str()); expect!["1"].assert_eq(size_of::().to_string().as_str()); From cc4ec3febdfe98eb7d495dd00d288c350730c331 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Fri, 6 Sep 2024 15:19:44 -0400 Subject: [PATCH 4/4] fixup! gate the aarch64 size change by rust version --- soroban-env-host/src/host/declared_size.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/soroban-env-host/src/host/declared_size.rs b/soroban-env-host/src/host/declared_size.rs index 51389765a..d5f2faa2c 100644 --- a/soroban-env-host/src/host/declared_size.rs +++ b/soroban-env-host/src/host/declared_size.rs @@ -422,6 +422,7 @@ mod test { fn check_aarch64_size_that_changed_at_rust_1_79() { expect!["488"].assert_eq(size_of::().to_string().as_str()); } + #[cfg(target_arch = "aarch64")] check_aarch64_size_that_changed_at_rust_1_79(); expect!["16"].assert_eq(size_of::
().to_string().as_str());