Add bytecode hash in interpreter #1888

[FredCoen]

Resolves #1888

[codspeed-hq]

CodSpeed Performance Report

Merging #1952 will not alter performance

_{Comparing FredCoen:Add-bytecode-hash-in-Interpreter-#1888 (afc98d0) with main (d2a3b5b)}

Summary

✅ 8 untouched benchmarks

[FredCoen]

@rakita i am not sure if option is the best way to do this? Also let me know if we should set the value in the getter directly !

[rakita]

Hey, check out the ExtBytecode, it is better place for the hash. And allow ExtBytecode to be initialized with the hash, if this is regenerated every time it is a performance hit.

[FredCoen]

thanks!

"allow ExtBytecode to be initialized with the hash" -> i introduced new_with_hash to allow this, otherwise calling the hash() method returns the hash and cashes it on the ExtByteCode. Is this what you had in mind?

For some reason it caused a regression on the ecrecover precompile perf. I am not sure why though. Do you have any idea @rakita ?

[rakita]

thanks!

"allow ExtBytecode to be initialized with the hash" -> i introduced new_with_hash to allow this, otherwise calling the hash() method returns the hash and cashes it on the ExtByteCode. Is this what you had in mind?

Yes! It would be good for fn hash() to return Option as there is already a function to generate hash in Bytecode.

For some reason it caused a regression on the ecrecover precompile perf. I am not sure why though. Do you have any idea @rakita ?

It is not related, it can be ignored. The test is kinda flaky.

[FredCoen]

hey i removed the cached hash test. Should i add anything else? CI passes where do you see it being flaky on your side locally?

[rakita]

hey i removed the cached hash test. Should i add anything else? CI passes where do you see it being flaky on your side locally?

Perfect!

The recovery test is flaky as nothing is changed with precompiles but it shows a diff, we can ignore it.

Have added the ExtBytecode::new_with_hash usege and will PR merge after CI

[rakita]

lgtm!