-
Notifications
You must be signed in to change notification settings - Fork 38.7k
base58: Improve DecodeBase58 performance. #8736
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
Conversation
Improve DecodeBase58 performance the same way as commit 3252208 did for EncodeBase58.
src/base58.cpp
Outdated
| int carry = ch - pszBase58; | ||
| for (std::vector<unsigned char>::reverse_iterator it = b256.rbegin(); it != b256.rend(); it++) { | ||
| int i = 0; | ||
| for (std::vector<unsigned char>::reverse_iterator it = b256.rbegin(); (carry != 0 || i < length) && (it != b256.rend()); it++, i++) { |
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.
Comments from the original PR suggest you should use ++i here.
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.
Updated, and benchmark number attached.
|
code review ACK e892dc1 |
|
Code Review utACK e892dc1 |
|
utACK e892dc1 |
|
Benchmarks BeforeBase58Decode,786432,0.000000720581738,0.000001479576895,0.000001459018677 AfterBase58Decode,1048576,0.000000478641596,0.000001010337655,0.000000991023853 |
| } | ||
| // Allocate enough space in big-endian base256 representation. | ||
| std::vector<unsigned char> b256(strlen(psz) * 733 / 1000 + 1); // log(58) / log(256), rounded up. | ||
| int size = strlen(psz) * 733 /1000 + 1; // log(58) / log(256), rounded up. |
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.
SPC before 1000
|
Confirming the numbers: |
|
ACK e892dc1 |
aa633c8 CBase58Data::SetString: cleanse the full vector (Kaz Wesley) 0b77f8a Improve readability of DecodeBase58Check(...) (practicalswift) dadadee Use prefix operator in for loop of DecodeBase58. (Jiaxing Wang) 305c382 base58: Improve DecodeBase58 performance. (Jiaxing Wang) 3cb418b Improve EncodeBase58 performance (João Barbosa) 4d17f71 don't try to decode invalid encoded ext keys (Jonas Schnelli) eef4ec6 extend bip32 tests to cover Base58c/CExtKey decode (furszy) 7239aaf fix and extend CBitcoinExtKeyBase template - fix Decode call (req. only one param) - add constructor for base58c->CExtKey (furszy) 13f09c3 remove unused inv from ConnectTip() (furszy) Pull request description: Coming from: * bitcoin#6468 . * bitcoin#7656 . * bitcoin#7922 * bitcoin#8736 . * bitcoin#10961 . ACKs for top commit: random-zebra: ACK aa633c8 Fuzzbawls: ACK aa633c8 Tree-SHA512: 3add3106a847b0b3d768df2c4ab5eae7d009e3820998fb5a4cd274169c64622e83ecd14dca51c31f3f6053199834129a1c6920b7ef1193632339297a041173d6
Improve DecodeBase58 performance the same way as commit 3252208 did
for EncodeBase58.
bench_bitcoin result for Base58Decode before and after the patch, each tested three times:
Before the patch:
Benchmark,count,min,max,average
Base58Decode,524288,0.000000976440788,0.000002053217031,0.000001960330792
Base58Decode,524288,0.000000979518518,0.000001978729415,0.000001956941560
Base58Decode,524288,0.000000986496161,0.000001988708391,0.000001964757757
After the patch:
Benchmark,count,min,max,average
Base58Decode,786432,0.000000660018486,0.000001336600690,0.000001329504509
Base58Decode,786432,0.000000668867870,0.000001348496880,0.000001338565805
Base58Decode,786432,0.000000656273187,0.000001334869012,0.000001324702074
That's about 32% decrease for the average time.