Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanity-check gtf opcode #1503

Merged
merged 11 commits into from
Nov 22, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Description of the upcoming release here.

### Added

- [#1503](https://github.com/FuelLabs/fuel-core/pull/1503): Add `gtf` opcode sanity check
- [#1490](https://github.com/FuelLabs/fuel-core/pull/1490): Add push and pop benchmarks.
- [#1485](https://github.com/FuelLabs/fuel-core/pull/1485): Prepare rc release of fuel core v0.21
- [#1476](https://github.com/FuelLabs/fuel-core/pull/1453): Add the majority of the "other" benchmarks for contract opcodes.
Expand Down
143 changes: 136 additions & 7 deletions benches/benches/block_target_gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,35 +88,57 @@ const BASE: u64 = 100_000;

pub struct SanityBenchmarkRunnerBuilder;

pub struct SharedSanityBenchmarkFactory {
pub struct SharedSanityBenchmarkRunnerBuilder {
service: FuelService,
rt: tokio::runtime::Runtime,
contract_id: ContractId,
rng: rand::rngs::StdRng,
}

pub struct MultiContractBenchmarkRunnerBuilder {
service: FuelService,
rt: tokio::runtime::Runtime,
contract_ids: Vec<ContractId>,
rng: rand::rngs::StdRng,
}

impl SanityBenchmarkRunnerBuilder {
/// Creates a factory for benchmarks that share a service with a contract, `contract_id`, pre-
/// deployed.
pub fn new_shared(contract_id: ContractId) -> SharedSanityBenchmarkFactory {
pub fn new_shared(contract_id: ContractId) -> SharedSanityBenchmarkRunnerBuilder {
let state_size = crate::utils::get_state_size();
let (service, rt) = service_with_contract_id(state_size, contract_id);
let rng = rand::rngs::StdRng::seed_from_u64(2322u64);
SharedSanityBenchmarkFactory {
SharedSanityBenchmarkRunnerBuilder {
service,
rt,
contract_id,
rng,
}
}

pub fn new_with_many_contracts(
contract_ids: Vec<ContractId>,
) -> MultiContractBenchmarkRunnerBuilder {
let state_size = 1000; // Arbitrary small state size
let (service, rt) = service_with_many_contracts(state_size, contract_ids.clone());
let rng = rand::rngs::StdRng::seed_from_u64(2322u64);
MultiContractBenchmarkRunnerBuilder {
service,
rt,
contract_ids,
rng,
}
}
}

impl SharedSanityBenchmarkFactory {
impl SharedSanityBenchmarkRunnerBuilder {
fn build(&mut self) -> SanityBenchmark {
SanityBenchmark {
service: &mut self.service,
rt: &self.rt,
rng: &mut self.rng,
contract_ids: vec![self.contract_id],
extra_inputs: vec![],
extra_outputs: vec![],
}
Expand All @@ -135,10 +157,24 @@ impl SharedSanityBenchmarkFactory {
}
}

impl MultiContractBenchmarkRunnerBuilder {
pub fn build(&mut self) -> SanityBenchmark {
SanityBenchmark {
service: &mut self.service,
rt: &self.rt,
rng: &mut self.rng,
contract_ids: self.contract_ids.clone(),
extra_inputs: vec![],
extra_outputs: vec![],
}
}
}

pub struct SanityBenchmark<'a> {
service: &'a mut FuelService,
rt: &'a tokio::runtime::Runtime,
rng: &'a mut rand::rngs::StdRng,
contract_ids: Vec<ContractId>,
extra_inputs: Vec<Input>,
extra_outputs: Vec<Output>,
}
Expand Down Expand Up @@ -167,7 +203,7 @@ impl<'a> SanityBenchmark<'a> {
script,
script_data,
self.service,
VmBench::CONTRACT,
self.contract_ids,
self.rt,
self.rng,
self.extra_inputs,
Expand Down Expand Up @@ -345,6 +381,97 @@ fn service_with_contract_id(
(service, rt)
}

fn service_with_many_contracts(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have three almost similar functions to run benchmarks. Maybe we can somehow join them into one customizable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good call. I collapsed another function but I forgot this one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a bunch of refactoring around the services and the run functions.

state_size: u64,
contract_ids: Vec<ContractId>,
) -> (FuelService, tokio::runtime::Runtime) {
use fuel_core::database::vm_database::IncreaseStorageKey;
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.unwrap();
let _drop = rt.enter();
let mut database = Database::rocksdb();
let mut config = Config::local_node();
config
.chain_conf
.consensus_parameters
.tx_params
.max_gas_per_tx = TARGET_BLOCK_GAS_LIMIT;

let contract_configs = contract_ids
.iter()
.map(|contract_id| ContractConfig {
contract_id: *contract_id,
code: vec![],
salt: Default::default(),
state: None,
balances: None,
tx_id: None,
output_index: None,
tx_pointer_block_height: None,
tx_pointer_tx_idx: None,
})
.collect::<Vec<_>>();
config.chain_conf.initial_state.as_mut().unwrap().contracts = Some(contract_configs);

config
.chain_conf
.consensus_parameters
.predicate_params
.max_gas_per_predicate = TARGET_BLOCK_GAS_LIMIT;
config.chain_conf.block_gas_limit = TARGET_BLOCK_GAS_LIMIT;
config.chain_conf.consensus_parameters.gas_costs = GasCosts::new(default_gas_costs());
config
.chain_conf
.consensus_parameters
.fee_params
.gas_per_byte = 0;
config.utxo_validation = false;
config.block_production = Trigger::Instant;

let mut storage_key = primitive_types::U256::zero();
let mut key_bytes = Bytes32::zeroed();

for contract_id in contract_ids.iter() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initializing state and balances for all contracts will kill us=D It's okay to init only for ContractId::zeroed().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will it kill us? It works. I'm not doing full state like for the other tests--I can still reduce it some as well. But I need separate contract ids for each input/output. Each contract needs to exist in the DB as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10M state for 254 contracts will take a lot of time to initialize=) We only need to init state for the ContractId::zeroed(). You can simply deploy other contracts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I said, I'm not doing the full 10M state for all of them:

    pub fn new_with_many_contracts(
        contract_ids: Vec<ContractId>,
    ) -> MultiContractBenchmarkRunnerBuilder {
        let state_size = 1000; // Arbitrary small state size
        let (service, rt) = service_with_many_contracts(state_size, contract_ids.clone());
        let rng = rand::rngs::StdRng::seed_from_u64(2322u64);
        MultiContractBenchmarkRunnerBuilder {
            service,
            rt,
            contract_ids,
            rng,
        }
    }

as opposed to what we're doing for the shared contract:

    pub fn new_shared(contract_id: ContractId) -> SharedSanityBenchmarkRunnerBuilder {
        let state_size = crate::utils::get_state_size();
        let (service, rt) = service_with_contract_id(state_size, contract_id);
        let rng = rand::rngs::StdRng::seed_from_u64(2322u64);
        SharedSanityBenchmarkRunnerBuilder {
            service,
            rt,
            contract_id,
            rng,
        }
    }

We could restructure to specify the state size for each contract though if we wanted. I don't think it's hurting any of our benchmarks significantly, currently though.

database
.init_contract_state(
contract_id,
(0..state_size).map(|_| {
storage_key.to_big_endian(key_bytes.as_mut());
storage_key.increase().unwrap();
(key_bytes, key_bytes)
}),
)
.unwrap();

let mut storage_key = primitive_types::U256::zero();
let mut sub_id = Bytes32::zeroed();
database
.init_contract_balances(
contract_id,
(0..state_size).map(|k| {
storage_key.to_big_endian(sub_id.as_mut());

let asset = if k % 2 == 0 {
VmBench::CONTRACT.asset_id(&sub_id)
} else {
let asset_id = AssetId::new(*sub_id);
storage_key.increase().unwrap();
asset_id
};
(asset, k / 2 + 1_000)
}),
)
.unwrap();
}

let service = fuel_core::service::FuelService::new(database, config.clone())
.expect("Unable to start a FuelService");
service.start().expect("Unable to start the service");
(service, rt)
}

// Runs benchmark for `script` with prepared `service` and specified contract (by `contract_id`) which should be
// included in service.
// Also include additional inputs and outputs in transaction
Expand All @@ -355,7 +482,7 @@ fn run_with_service_with_extra_inputs(
script: Vec<Instruction>,
script_data: Vec<u8>,
service: &fuel_core::service::FuelService,
contract_id: ContractId,
contract_ids: Vec<ContractId>,
rt: &tokio::runtime::Runtime,
rng: &mut rand::rngs::StdRng,
extra_inputs: Vec<Input>,
Expand All @@ -382,20 +509,22 @@ fn run_with_service_with_extra_inputs(
Default::default(),
Default::default(),
);
for contract_id in &contract_ids {
let input_count = tx_builder.inputs().len();

let contract_input = Input::contract(
UtxoId::default(),
Bytes32::zeroed(),
Bytes32::zeroed(),
TxPointer::default(),
contract_id,
*contract_id,
);
let contract_output = Output::contract(input_count as u8, Bytes32::zeroed(), Bytes32::zeroed());

tx_builder
.add_input(contract_input)
.add_output(contract_output);
}

for input in &extra_inputs {
tx_builder.add_input(input.clone());
Expand Down
5 changes: 5 additions & 0 deletions benches/benches/block_target_gas_set/min_fee.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
use super::*;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can exclude this file for now=)


pub fn run_min_fee(group: &mut BenchmarkGroup<WallTime>) {
todo!();
}
20 changes: 19 additions & 1 deletion benches/benches/block_target_gas_set/other.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,23 @@ pub fn run_other(group: &mut BenchmarkGroup<WallTime>) {
.run(id, group, instructions, script_data.clone());
}

// gtf: TODO: As part of parent issue (https://github.com/FuelLabs/fuel-core/issues/1386)
// gtf
{
let count = 254;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we updated the corresponding benchmark to get the right price for GTF=)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean using GTFArgs::InputContractOutputIndex instead of GTFArgs::ScriptData in the vm_set benchmark? So they are testing the same argument for gtf?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because we need to use the price for the GTF worst case. The getter for script data is fast

let correct_index = count; // Have the last index be the correct one. The builder includes an extra input, so it's the 255th index (254).

let contract_ids = (0..count)
.map(|x| ContractId::from([x as u8; 32]))
.collect::<Vec<_>>();

let instructions = vec![
op::movi(0x11, correct_index as u32),
op::gtf_args(0x10, 0x11, GTFArgs::InputContractOutputIndex),
op::jmpb(RegId::ZERO, 0),
];

SanityBenchmarkRunnerBuilder::new_with_many_contracts(contract_ids)
.build()
.run("other/gtf", group, instructions, vec![]);
}
}