base58: Improve DecodeBase58 performance.#8736
Merged
laanwj merged 2 commits intobitcoin:masterfrom Nov 7, 2016
Merged
Conversation
Improve DecodeBase58 performance the same way as commit 3252208 did for EncodeBase58.
Member
fanquake
reviewed
Sep 16, 2016
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++) { |
Member
There was a problem hiding this comment.
Comments from the original PR suggest you should use ++i here.
Contributor
Author
There was a problem hiding this comment.
Updated, and benchmark number attached.
Member
|
code review ACK e892dc1 |
Contributor
|
Code Review utACK e892dc1 |
Member
|
utACK e892dc1 |
Member
|
Benchmarks BeforeBase58Decode,786432,0.000000720581738,0.000001479576895,0.000001459018677 AfterBase58Decode,1048576,0.000000478641596,0.000001010337655,0.000000991023853 |
paveljanik
reviewed
Nov 7, 2016
| } | ||
| // 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. |
Contributor
|
Confirming the numbers: |
Contributor
|
ACK e892dc1 |
laanwj
added a commit
that referenced
this pull request
Nov 7, 2016
random-zebra
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Jun 27, 2020
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.