-
Notifications
You must be signed in to change notification settings - Fork 572
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
Migrate primitive_types::U256
to ruint::Uint<256, 4>
#239
Conversation
… than ruint" This reverts commit e0fb329.
Currently, performance is degraded: ➜ revm-test git:(main) ✗ cargo run --release --bin snailtracer
Compiling revm_precompiles v1.1.1 (/Users/shekhirin/Projects/revm/crates/revm_precompiles)
Compiling revm v2.1.0 (/Users/shekhirin/Projects/revm/crates/revm)
Compiling revm-test v0.1.0 (/Users/shekhirin/Projects/revm/bins/revm-test)
Finished release [optimized] target(s) in 5.15s
Running `/Users/shekhirin/Projects/revm/target/release/snailtracer`
Hello, world!
elapsed: 37.776967ms
0: 38.939875ms
1: 38.214708ms
2: 38.134833ms
3: 38.040416ms
4: 38.034625ms
5: 37.964291ms
6: 37.87475ms
7: 37.74225ms
8: 37.708ms
9: 37.673833ms
10: 37.66675ms
11: 37.648833ms
12: 37.647833ms
13: 37.638166ms
14: 37.613708ms
15: 37.58575ms
16: 37.583833ms
17: 37.57025ms
18: 37.546ms
19: 37.543916ms
20: 37.529875ms
21: 37.506625ms
22: 37.498333ms
23: 37.466041ms
24: 37.39825ms
end! ➜ revm-test git:(ruint) ✗ cargo run --release --bin snailtracer
Compiling revm_precompiles v1.1.1 (/Users/shekhirin/Projects/revm/crates/revm_precompiles)
Compiling revm v2.1.0 (/Users/shekhirin/Projects/revm/crates/revm)
Compiling revm-test v0.1.0 (/Users/shekhirin/Projects/revm/bins/revm-test)
Finished release [optimized] target(s) in 4.28s
Running `/Users/shekhirin/Projects/revm/target/release/snailtracer`
Hello, world!
elapsed: 52.893343ms
0: 54.712708ms
1: 54.705416ms
2: 54.635166ms
3: 54.387916ms
4: 54.326208ms
5: 54.254125ms
6: 53.999916ms
7: 53.96375ms
8: 53.659083ms
9: 53.057916ms
10: 52.467791ms
11: 52.364833ms
12: 52.347541ms
13: 52.30825ms
14: 52.202083ms
15: 52.142125ms
16: 52.076208ms
17: 52.040041ms
18: 51.997583ms
19: 51.980166ms
20: 51.933583ms
21: 51.820375ms
22: 51.77ms
23: 51.766166ms
24: 51.6105ms
end! |
Try again :) EDIT: Tried it with v1.5.0. Helps a bit, but looks like something else needs optimization. Meanwhile #240 shows that at least divisions are working well. |
I have a new version of ruint and a PR for this PR that closes the perf gap. |
👀 ➜ revm-test git:(main) ✗ cargo run --release --bin snailtracer
Compiling revm_precompiles v1.1.1 (/Users/shekhirin/Projects/revm/crates/revm_precompiles)
Compiling revm v2.1.0 (/Users/shekhirin/Projects/revm/crates/revm)
Compiling revm-test v0.1.0 (/Users/shekhirin/Projects/revm/bins/revm-test)
Finished release [optimized] target(s) in 3.93s
Running `/Users/shekhirin/Projects/revm/target/release/snailtracer`
Hello, world!
elapsed: 36.713249ms
0: 36.997875ms
1: 36.807291ms
2: 36.648416ms
3: 36.543916ms
4: 36.496666ms
5: 36.495125ms
6: 36.489958ms
7: 36.487208ms
8: 36.4865ms
9: 36.483041ms
10: 36.475708ms
11: 36.467583ms
12: 36.4625ms
13: 36.447791ms
14: 36.437541ms
15: 36.43ms
16: 36.422833ms
17: 36.418666ms
18: 36.399ms
19: 36.394333ms
20: 36.39125ms
21: 36.383791ms
22: 36.376416ms
23: 36.321ms
24: 36.299583ms
end! ➜ revm-test git:(ruint) ✗ cargo run --release --bin snailtracer
Compiling revm_precompiles v1.1.1 (/Users/shekhirin/Projects/revm/crates/revm_precompiles)
Compiling revm v2.1.0 (/Users/shekhirin/Projects/revm/crates/revm)
Compiling revm-test v0.1.0 (/Users/shekhirin/Projects/revm/bins/revm-test)
Finished release [optimized] target(s) in 18.58s
Running `/Users/shekhirin/Projects/revm/target/release/snailtracer`
Hello, world!
elapsed: 41.140597ms
0: 42.403875ms
1: 42.297416ms
2: 42.245666ms
3: 42.101208ms
4: 41.783041ms
5: 41.718708ms
6: 41.625375ms
7: 41.487ms
8: 40.886916ms
9: 40.794791ms
10: 40.72425ms
11: 40.688458ms
12: 40.652541ms
13: 40.582458ms
14: 40.578666ms
15: 40.571458ms
16: 40.54475ms
17: 40.518916ms
18: 40.409708ms
19: 40.40475ms
20: 40.40275ms
21: 40.395916ms
22: 40.390041ms
23: 40.358083ms
24: 40.310541ms
end! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
balance | ||
.unwrap_or_else(|e| panic!("web3 get balance error:{:?}", e)) | ||
.unwrap_or_else(|e| panic!("web3 get balance error:{e:?}")) | ||
.0, | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.into()
should work here instead of the manual conversion.
Though there can be a bit of dependency hell around primitive_types
if multiple versions end up in the dep tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should work but you're right about the dependency hell. iiuc it's between rust-web3
, ruint
and revm
. i'd leave it as is and fix it when we get rid of primitive-types
completely in revm
by adopting the H160 -> B160, etc.
changes.
No description provided.