Update bhsh bench #2456
Update bhsh bench #2456
Conversation
AurelienFT
requested review from
xgreenx,
Dentosal and
MitchTurner
as code owners
AurelienFT
added
the
no changelog
| fn add_blocks(&mut self, nb_blocks: u32) { | ||
| for i in 1..=nb_blocks { | ||
| let block = | ||
| fuel_core::service::genesis::create_genesis_block(&Config::local_node()); |
There was a problem hiding this comment.
qq: why are we inserting multiple genesis blocks?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
It would be nice if the block were 256KB and we tried to get the height for one of them=)
There was a problem hiding this comment.
One block = 256KB or the total of blocks is 256KB ?
There was a problem hiding this comment.
Because I don't understand what a bigger block will change because we just get the id field
There was a problem hiding this comment.
Each block 256KB. With the current implementation it changes nothing, but in the future if we change the implementation, it can be useful
There was a problem hiding this comment.
Okok idk how to build blocks with a specific size but will try (if you read this and know directly tell meeee) :)
There was a problem hiding this comment.
You can add one transaction with very huge script data
| @@ -8,7 +8,7 @@ pub fn default_gas_costs() -> GasCostsValues { | |||
| andi: 2, | |||
| bal: 274, | |||
| bhei: 2, | |||
| bhsh: 2, | |||
| bhsh: 32, | |||
There was a problem hiding this comment.
how has this been calculated?
There was a problem hiding this comment.
With the benchmark I modified
There was a problem hiding this comment.
I meant what steps did you follow to get this value? Asking for knowledge sharing purposes.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no suggestions.
Linked Issues/PRs
#2237
Description
This PR is on top of
add_zk_opcodesso that the change of gas cost will be efective with the newGasCostsValueV5. The benchmark was incorrect because offuel-core/crates/storage/src/vm_storage.rs
Line 291 in afbfef2
0. If we use theDBBenchit works because the block height used is 1. However, if we want to add blocks in the db we also need theDBBenchto use an higher block height. This PR solves all of this.This PR also simplify and comment the preparation code.
Checklist
Before requesting review
After merging, notify other teams
[Add or remove entries as needed]