chore(gas_costs): update local chain config with nightly benchmark results #2932
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
github-actions
bot
requested review from
xgreenx,
Dentosal and
MitchTurner
as code owners
There was a problem hiding this comment.
All opcodes became 2-3 times more expensive(except several).
We need to investigate why. I assume, maybe because of optimizations to fuel-vm(and new runner) baseline became more cheap, that automatically made all opcodes more expensive. @FuelLabs/client Who wants to do that?=D
We need to run block_target_gas.rs tests and see what is the average execution time for all opcodes with new gas prices.
I will approve for now, just to see what results it will produce next time.
| } | ||
| }, | ||
| "new_storage_per_byte": 63, | ||
| "new_storage_per_byte": 1, |
There was a problem hiding this comment.
It would be nice to use hardcoded value here. @rymnc Is that possible somehow to update it?=D Maybe we could make it part of the CI, or make it part of the collect binary
There was a problem hiding this comment.
hmm, we can try. idk how to do it but i can check
| "ji": 1, | ||
| "jmp": 1, | ||
| "jne": 2, | ||
| "jnei": 2, | ||
| "jnzi": 2, | ||
| "jmpf": 2, | ||
| "jmpb": 2, | ||
| "jnzf": 2, | ||
| "jnzb": 2, | ||
| "jnef": 2, | ||
| "jneb": 2, | ||
| "jnei": 1, | ||
| "jnzi": 1, | ||
| "jmpf": 1, | ||
| "jmpb": 1, | ||
| "jnzf": 1, | ||
| "jnzb": 1, | ||
| "jnef": 1, | ||
| "jneb": 1, |
There was a problem hiding this comment.
We don't run benchmarks for all opcodes. Only for several of them:
In the past, I was updating manually them to much one value. Maybe we should add benchmarks for them, or update collect binary to use the same value for them=)
There was a problem hiding this comment.
@FuelLabs/client Who wants to do that?=D
There was a problem hiding this comment.
I can do that tomorrow morning. If I understand well some of the opcode such as jnzf should have a benchmark but don't have and we want them to have one now or use the same value as another opcode that is doing something similar to the missing one ?
| "cfe": { | ||
| "LightOperation": { | ||
| "base": 10, | ||
| "units_per_gas": 62 | ||
| "base": 3, | ||
| "units_per_gas": 256 | ||
| } | ||
| }, | ||
| "cfei": { | ||
| "LightOperation": { | ||
| "base": 10, | ||
| "units_per_gas": 66 | ||
| "base": 3, | ||
| "units_per_gas": 394 | ||
| } | ||
| }, |
There was a problem hiding this comment.
I'm not sure why cfe became more cheaper...=D
|
I can also check that tomorrow morning for the |
|
this PR should probably be closed. we need to also update snapshot tests when we modify the local chain config. i've tracked the set of follow ups here: #2933 |
Updated benchmark results