Skip to content
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

Merged
merged 20 commits into from
Oct 29, 2022

Conversation

shekhirin
Copy link
Collaborator

No description provided.

@shekhirin
Copy link
Collaborator Author

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!

@recmo
Copy link
Contributor

recmo commented Oct 24, 2022

Try again :) snailtracer is heavy on division. I just published v1.5.0 which optimizes divisions a lot.

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.

@recmo recmo mentioned this pull request Oct 25, 2022
@recmo
Copy link
Contributor

recmo commented Oct 25, 2022

I have a new version of ruint and a PR for this PR that closes the perf gap.

shekhirin#1

@shekhirin
Copy link
Collaborator Author

👀

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!

Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

README.md Show resolved Hide resolved
@shekhirin shekhirin marked this pull request as ready for review October 25, 2022 17:49
balance
.unwrap_or_else(|e| panic!("web3 get balance error:{:?}", e))
.unwrap_or_else(|e| panic!("web3 get balance error:{e:?}"))
.0,
),
Copy link
Contributor

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.

Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants