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

Update bhsh bench #2456

Merged
merged 11 commits into from
Jan 2, 2025
50 changes: 43 additions & 7 deletions benches/benches/vm_set/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,11 @@ use fuel_core_types::{
},
fuel_tx::{
ContractIdExt,
Finalizable,
Input,
Output,
Transaction,
TransactionBuilder,
Word,
},
fuel_types::*,
Expand All @@ -65,6 +68,7 @@ pub struct BenchDb {
db: GenesisDatabase,
/// Used for RAII cleanup. Contents of this directory are deleted on drop.
_tmp_dir: utils::ShallowTempDir,
latest_block: u32,
}

impl BenchDb {
Expand Down Expand Up @@ -122,18 +126,40 @@ impl BenchDb {
&block.compress(&chain_config.consensus_parameters.chain_id()),
)
.unwrap();

Ok(Self {
_tmp_dir: tmp_dir,
db: database,
latest_block: 0,
})
}

fn add_blocks(&mut self, nb_blocks: u32) {
for i in 1..=nb_blocks {
let mut block =
fuel_core::service::genesis::create_genesis_block(&Config::local_node());
Copy link
Member

Choose a reason for hiding this comment

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

qq: why are we inserting multiple genesis blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this is the only helpers that we have here and it doesn't change anything for our benchmarks has we never read the value.

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 the block were 256KB and we tried to get the height for one of them=)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One block = 256KB or the total of blocks is 256KB ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I don't understand what a bigger block will change because we just get the id field

Copy link
Collaborator

Choose a reason for hiding this comment

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

Each block 256KB. With the current implementation it changes nothing, but in the future if we change the implementation, it can be useful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okok idk how to build blocks with a specific size but will try (if you read this and know directly tell meeee) :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can add one transaction with very huge script data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

let transactions = block.transactions_mut();
// Add a dummy transaction to the block to make the block bigger.
transactions.push(Transaction::Script(
TransactionBuilder::script(vec![], vec![1; 200_000]).finalize(),
));
let config = Config::local_node();
let chain_config = config.snapshot_reader.chain_config();
self.db
.storage::<FuelBlocks>()
.insert(
&i.into(),
&block.compress(&chain_config.consensus_parameters.chain_id()),
)
.unwrap();
}
self.latest_block = nb_blocks;
}

/// Creates a `VmDatabase` instance.
fn to_vm_database(&self) -> VmStorage<StorageTransaction<GenesisDatabase>> {
let consensus = ConsensusHeader {
prev_root: Default::default(),
height: 1.into(),
height: self.latest_block.into(),
time: Tai64::UNIX_EPOCH,
generated: (),
};
Expand Down Expand Up @@ -517,14 +543,24 @@ pub fn run(c: &mut Criterion) {
VmBench::new(op::bhei(0x10)),
);

let mut db =
BenchDb::new(&VmBench::CONTRACT).expect("Unable to fill contract storage");
db.add_blocks(10000);

run_group_ref(
&mut c.benchmark_group("bhsh"),
"bhsh",
VmBench::new(op::bhsh(0x10, RegId::ZERO)).with_prepare_script(vec![
op::movi(0x10, Bytes32::LEN.try_into().unwrap()),
op::aloc(0x10),
op::move_(0x10, RegId::HP),
]),
VmBench::new(op::bhsh(RegId::HP, 0x11))
.prepend_prepare_script(vec![
// Store number of bytes we want to alloc for the future result
op::movi(0x10, Bytes32::LEN.try_into().unwrap()),
// Allocate space for the future result
op::aloc(0x10),
// Add block height to 0x11
op::movi(0x11, 0xc9),
])
.with_height(0xca.into())
.with_db(db.to_vm_database()),
);

run_group_ref(
Expand Down
2 changes: 1 addition & 1 deletion benches/src/default_gas_costs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub fn default_gas_costs() -> GasCostsValues {
andi: 2,
bal: 274,
bhei: 2,
bhsh: 2,
bhsh: 32,
Copy link
Contributor

Choose a reason for hiding this comment

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

how has this been calculated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the benchmark I modified

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant what steps did you follow to get this value? Asking for knowledge sharing purposes.

Copy link
Contributor Author

@AurelienFT AurelienFT Nov 26, 2024

Choose a reason for hiding this comment

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

I found it pretty well explained here : https://github.com/FuelLabs/fuel-core/blob/master/benches/README.md

However a little trick that green told me is to comment all the other vm opcode benches to win some time (except noop that is required as a base). Maybe we should mark it in the readme.

burn: 7566,
cb: 2,
cfsi: 2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"andi": 2,
"bal": 366,
"bhei": 2,
"bhsh": 2,
"bhsh": 32,
"burn": 33949,
"cb": 2,
"cfei": 2,
Expand Down
2 changes: 1 addition & 1 deletion bin/fuel-core/chainspec/local-testnet/chain_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"andi": 2,
"bal": 274,
"bhei": 2,
"bhsh": 2,
"bhsh": 32,
"burn": 7566,
"cb": 2,
"cfsi": 2,
Expand Down
Loading