Skip to content

Conversation

@wjx
Copy link
Contributor

@wjx wjx commented Sep 15, 2016

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.

Improve DecodeBase58 performance the same way as commit 3252208 did
for EncodeBase58.
@fanquake
Copy link
Member

@wjx Can you provide any numbers as to the performance improvement? Is it inline with what was seen in #7656?

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++) {
Copy link
Member

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.

Copy link
Contributor Author

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.

@laanwj
Copy link
Member

laanwj commented Sep 22, 2016

code review ACK e892dc1

@jonasschnelli
Copy link
Contributor

Code Review utACK e892dc1

@sipa
Copy link
Member

sipa commented Oct 24, 2016

utACK e892dc1

@fanquake
Copy link
Member

fanquake commented Nov 7, 2016

Benchmarks

Before

Base58Decode,786432,0.000000720581738,0.000001479576895,0.000001459018677
Base58Decode,786432,0.000000731757609,0.000001487409463,0.000001467602791
Base58Decode,786432,0.000000714662747,0.000001470549250,0.000001451156398
Base58Decode,786432,0.000000723695848,0.000001477301339,0.000001463891143

After

Base58Decode,1048576,0.000000478641596,0.000001010337655,0.000000991023853
Base58Decode,1048576,0.000000484878910,0.000000999572876,0.000000988008424
Base58Decode,1048576,0.000000494663254,0.000001009095286,0.000000993179128
Base58Decode,1048576,0.000000960510079,0.000000998141331,0.000000973840770

}
// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

SPC before 1000

@paveljanik
Copy link
Contributor

paveljanik commented Nov 7, 2016

Confirming the numbers:

$ pwd; while :; do ./bench_bitcoin | grep -m1 Base58Decode; done
/tmp/bitcoin-master/src/bench
Base58Decode,786432,0.000000705200364,0.000001495651304,0.000001435880222
Base58Decode,786432,0.000000754334906,0.000001533042450,0.000001507242814
Base58Decode,786432,0.000000738342351,0.000001584410711,0.000001496629011
...
$ pwd; while :; do ./bench_bitcoin | grep -m1 Base58Decode; done
/tmp/bitcoin-8736/src/bench
Base58Decode,1310720,0.000000458023351,0.000000963798811,0.000000941479630
Base58Decode,1048576,0.000000513755367,0.000001057205736,0.000000997559710
Base58Decode,1048576,0.000000455460395,0.000001009773769,0.000000967173719
...

@paveljanik
Copy link
Contributor

ACK e892dc1

@laanwj laanwj merged commit e892dc1 into bitcoin:master Nov 7, 2016
laanwj added a commit that referenced this pull request Nov 7, 2016
e892dc1 Use prefix operator in for loop of DecodeBase58. (Jiaxing Wang)
159ed95 base58: Improve DecodeBase58 performance. (Jiaxing Wang)
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants