Rework analysis

[rakita]

@onbjerg can you run this? I guess it should be a little bit faster than the previous code, but not by a lot.

[onbjerg]

Gave this a try, seems to be slower somehow:

~/Projects/github/rari-capital/solmate main* 11s
❯ hyperfine --warmup 3 'forge test --offline --allow-failure'
Benchmark 1: forge test --offline --allow-failure
  Time (mean ± σ):      2.441 s ±  0.142 s    [User: 36.908 s, System: 0.983 s]
  Range (min … max):    2.306 s …  2.773 s    10 runs


~/Projects/github/rari-capital/solmate main* 32s
❯ hyperfine --warmup 3 '../../gakonst/foundry/target/release/forge test --offline --allow-failure'
Benchmark 1: ../../gakonst/foundry/target/release/forge test --offline --allow-failure
  Time (mean ± σ):      2.796 s ±  0.138 s    [User: 42.262 s, System: 1.028 s]
  Range (min … max):    2.575 s …  2.985 s    10 runs

Not entirely sure why it's that much slower - maybe it's because I now need to do a matches!() to check if it's a push/end of a gas block? See the diff here: foundry-rs/foundry@d751fe9

[rakita]

Fun time optimizing :)
@onbjerg i dont thing we will gain a lot by investing time here, skipping this part of code would be the best way.

[gakonst]

Could we do the following:

1. Make Contract::analyze public
2. Add a new_analyzed method to Contract which allows us to instantiate it with an already analyzed code/jumptable
3. Add a Option<...> field to EVMImpl which can store an analyzed bytecode which can be set client side with e.g a with_analyzed_code method
4. If the above option is Some, make it use the analyzed bytecode and call new_analyzed instead of new here:

   revm/crates/revm/src/evm_impl.rs

   Line 363 in cb5fbb9

   let contract = Contract::new::<SPEC>(

[rakita]

@onbjerg I made additional change, and tested it a litlle better on some random data. Can you compare it again?

[rakita]

Could we do the following:

1. Make Contract::analyze public
2. Add a new_analyzed method to Contract which allows us to instantiate it with an already analyzed code/jumptable
3. Add a Option<...> field to EVMImpl which can store an analyzed bytecode which can be set client side with e.g a with_analyzed_code method
4. If the above option is Some, make it use the analyzed bytecode and call new_analyzed instead of new here:

   revm/crates/revm/src/evm_impl.rs

   Line 363 in cb5fbb9

   let contract = Contract::new::<SPEC>(

Bytecode format needs to be implemented to address this in a proper way.

Introduce Bytecode struct that is going to contain:

pub struct Bytecode {
  bytes: Bytes, //or vec
  bytes_size: usize, // for analysis some bytecode gets padded in that sense having standalone size field is needed.
  info: BytecodeInfo,
}

pub enum BytecodeInfo {
   Default
   Analysed {
      jump_table: Vec<_>,
   }

change this:

revm/crates/revm/src/db/traits.rs

Line 14 in eb3282d

fn code_by_hash(&mut self, code_hash: H256) -> Bytes;

and insides of AccountInfo:

revm/crates/revm/src/db/traits.rs

Line 12 in eb3282d

fn basic(&mut self, address: H160) -> AccountInfo;

to contains Bytecode and make all necesary changes where needed.

[onbjerg]

@onbjerg I made additional change, and tested it a litlle better on some random data. Can you compare it again?

Can you rebase real quick? 🙏

[rakita]

@onbjerg I made additional change, and tested it a litlle better on some random data. Can you compare it again?

Can you rebase real quick? pray

rebased

[onbjerg]

Sorry for the delay, I was really busy yesterday 😬

Here's the benchmark with your patch applied:

~/Projects/github/rari-capital/solmate main 49s
❯ hyperfine --warmup 3 'forge test --offline --allow-failure' --runs 25
Benchmark 1: forge test --offline --allow-failure
  Time (mean ± σ):      2.449 s ±  0.115 s    [User: 36.758 s, System: 0.959 s]
  Range (min … max):    2.192 s …  2.653 s    25 runs

And without:

~/Projects/github/rari-capital/solmate main 2m 55s
❯ hyperfine --warmup 3 'forge test --offline --allow-failure' --runs 25
Benchmark 1: forge test --offline --allow-failure
  Time (mean ± σ):      2.107 s ±  0.115 s    [User: 34.400 s, System: 0.840 s]
  Range (min … max):    1.848 s …  2.313 s    25 runs

🤔

[rakita]

@onbjerg as the last try I will merge two-loop and check if it helps, maybe localization of a loop helps with speed idk, it is really hard to optimize and maybe test data is not the best representation, either way i want this merged so that we can start working on bytecode format that should be the proper solution for fuzzing contract problem.

[gakonst]

In general we're supportive of any kind of change in this neighborhood, let us know how we can be most helpful to keeping the ball rolling.

[rakita]

@onbjerg try this for the last time, i had a small fix here. I tried other option but it was not great, i intend to merge it as it is now.

[onbjerg]

Lil Web3 (top is this branch, bottom is latest nightly)

~/Projects/github/m1guelpf/lil-web3 main
❯ hyperfine --warmup 3 'forge test --offline --allow-failure' --runs 25
Benchmark 1: forge test --offline --allow-failure
  Time (mean ± σ):      1.040 s ±  0.024 s    [User: 1.132 s, System: 0.050 s]
  Range (min … max):    1.014 s …  1.118 s    25 runs


~/Projects/github/m1guelpf/lil-web3 main 31s
❯ hyperfine --warmup 3 'forge test --offline --allow-failure' --runs 25
Benchmark 1: forge test --offline --allow-failure
  Time (mean ± σ):      1.067 s ±  0.016 s    [User: 1.150 s, System: 0.070 s]
  Range (min … max):    1.042 s …  1.105 s    25 runs

Solmate (top is this branch, bottom is nightly)

~/Projects/github/rari-capital/solmate main
❯ hyperfine --warmup 3 'forge test --offline --allow-failure' --runs 25
Benchmark 1: forge test --offline --allow-failure
  Time (mean ± σ):      2.839 s ±  0.112 s    [User: 35.060 s, System: 0.618 s]
  Range (min … max):    2.578 s …  2.999 s    25 runs


~/Projects/github/rari-capital/solmate main 1m 19s
❯ hyperfine --warmup 3 'forge test --offline --allow-failure' --runs 25
Benchmark 1: forge test --offline --allow-failure
  Time (mean ± σ):      2.727 s ±  0.105 s    [User: 35.863 s, System: 0.726 s]
  Range (min … max):    2.499 s …  2.913 s    25 runs

Feel free to merge, I don't think there's much to gain here in terms of perf unfortunately 😞