Skip to content

Commit

Permalink
Fix #1372 - give ParsedModule a declared size, meter its Rc alloc
Browse files Browse the repository at this point in the history
  • Loading branch information
graydon committed Mar 27, 2024
1 parent d35b5e1 commit f03d0e4
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 21 deletions.
6 changes: 2 additions & 4 deletions soroban-env-host/benches/common/cost_types/vm_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use soroban_env_host::{
vm::{ParsedModule, VersionedContractCodeCostInputs},
xdr, Host,
};
use std::rc::Rc;

// Protocol 20 coarse cost model.
pub(crate) struct VmInstantiationMeasure;
Expand Down Expand Up @@ -40,10 +39,9 @@ macro_rules! impl_measurement_for_instantiation_cost_type {
.unwrap(),
)
}
let module = Rc::new(
let module =
ParsedModule::new_with_isolated_engine(_host, &wasm, cost_inputs.clone())
.unwrap(),
);
.unwrap();
VmInstantiationSample {
id: Some(id),
wasm,
Expand Down
4 changes: 2 additions & 2 deletions soroban-env-host/src/cost_runner/cost_types/vm_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,15 @@ mod v21 {
_iter: u64,
sample: Self::SampleType,
) -> Self::RecycledType {
let module = black_box(Rc::new(
let module = black_box(
ParsedModule::new(
host,
sample.module.module.engine(),
&sample.wasm[..],
sample.module.cost_inputs.clone(),
)
.unwrap(),
));
);
(Some(module), sample.wasm)
}

Expand Down
4 changes: 4 additions & 0 deletions soroban-env-host/src/host/declared_size.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::{
host::{frame::Context, Events},
host_object::HostObject,
storage::AccessType,
vm::ParsedModule,
xdr::{
AccountEntry, AccountId, Asset, BytesM, ContractCodeEntry, ContractDataDurability,
ContractEvent, ContractExecutable, ContractIdPreimage, CreateContractArgs, Duration,
Expand Down Expand Up @@ -130,6 +131,7 @@ impl_declared_size_type!(AccountAuthorizationTrackerSnapshot, 40);
impl_declared_size_type!(InvokerContractAuthorizationTracker, 192);
impl_declared_size_type!(InternalDiagnosticArg, 64);
impl_declared_size_type!(InternalDiagnosticEvent, 88);
impl_declared_size_type!(ParsedModule, 304);

// xdr types
impl_declared_size_type!(TimePoint, 8);
Expand Down Expand Up @@ -446,6 +448,7 @@ mod test {
);
expect!["64"].assert_eq(size_of::<InternalDiagnosticArg>().to_string().as_str());
expect!["88"].assert_eq(size_of::<InternalDiagnosticEvent>().to_string().as_str());
expect!["304"].assert_eq(size_of::<ParsedModule>().to_string().as_str());

// xdr types
expect!["8"].assert_eq(size_of::<TimePoint>().to_string().as_str());
Expand Down Expand Up @@ -642,6 +645,7 @@ mod test {
assert_mem_size_le_declared_size!(InvokerContractAuthorizationTracker);
assert_mem_size_le_declared_size!(InternalDiagnosticArg);
assert_mem_size_le_declared_size!(InternalDiagnosticEvent);
assert_mem_size_le_declared_size!(ParsedModule);

// xdr types
assert_mem_size_le_declared_size!(TimePoint);
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ impl Vm {
VmInstantiationTimer::new(host.clone());
let config = get_wasmi_config(host.as_budget())?;
let engine = wasmi::Engine::new(&config);
let parsed_module = Rc::new(ParsedModule::new(host, &engine, wasm, cost_inputs)?);
let parsed_module = ParsedModule::new(host, &engine, wasm, cost_inputs)?;
Self::instantiate(host, contract_id, parsed_module)
}

Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/vm/module_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl ModuleCache {
&[],
));
}
let parsed_module = Rc::new(ParsedModule::new(host, &self.engine, &wasm, cost_inputs)?);
let parsed_module = ParsedModule::new(host, &self.engine, &wasm, cost_inputs)?;
self.modules =
self.modules
.insert(contract_id.metered_clone(host)?, parsed_module, host)?;
Expand Down
33 changes: 20 additions & 13 deletions soroban-env-host/src/vm/parsed_module.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{
err,
host::metered_clone::MeteredAlloc,
meta::{self, get_ledger_protocol_version},
xdr::{ContractCostType, Limited, ReadXdr, ScEnvMetaEntry, ScErrorCode, ScErrorType},
Host, HostError, DEFAULT_XDR_RW_LIMITS,
Expand All @@ -8,7 +9,7 @@ use crate::{
use wasmi::{Engine, Module};

use super::{ModuleCache, MAX_VM_ARGS};
use std::io::Cursor;
use std::{io::Cursor, rc::Rc};

#[derive(Debug, Clone)]
pub enum VersionedContractCodeCostInputs {
Expand Down Expand Up @@ -153,32 +154,38 @@ impl ParsedModule {
engine: &Engine,
wasm: &[u8],
cost_inputs: VersionedContractCodeCostInputs,
) -> Result<Self, HostError> {
) -> Result<Rc<Self>, HostError> {
cost_inputs.charge_for_parsing(host)?;
let (module, proto_version) = Self::parse_wasm(host, engine, wasm)?;
Ok(Self {
module,
proto_version,
cost_inputs,
})
Rc::metered_new(
Self {
module,
proto_version,
cost_inputs,
},
host,
)
}

#[cfg(any(test, feature = "testutils"))]
pub fn new_with_isolated_engine(
host: &Host,
wasm: &[u8],
cost_inputs: VersionedContractCodeCostInputs,
) -> Result<Self, HostError> {
) -> Result<Rc<Self>, HostError> {
use crate::budget::AsBudget;
let config = crate::vm::get_wasmi_config(host.as_budget())?;
let engine = Engine::new(&config);
cost_inputs.charge_for_parsing(host)?;
let (module, proto_version) = Self::parse_wasm(host, &engine, wasm)?;
Ok(Self {
module,
proto_version,
cost_inputs,
})
Rc::metered_new(
Self {
module,
proto_version,
cost_inputs,
},
host,
)
}

/// Parse the Wasm blob into a [Module] and its protocol number, checking its interface version
Expand Down

0 comments on commit f03d0e4

Please sign in to comment.