[Backport] Base58 performance improvements #1676
Merged
Conversation
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
- fix Decode call (req. only one param) - add constructor for base58c->CExtKey coming from btc@7cb1f9f7eb8162a792b4b87bba99fa21c682582e
coming from btc@8d2af54eccda83bfe86bb14069ad54d0e4ba357c
Improve DecodeBase58 performance the same way as commit 3252208 did for EncodeBase58.
SetString seems to be passing the length of the wrong variable to memory_cleanse, resulting in the last byte of the temporary buffer not being securely erased.
Fuzzbawls
changed the title
random-zebra
approved these changes
Jun 15, 2020
random-zebra
left a comment
random-zebra
left a comment
There was a problem hiding this comment.
ACK aa633c8
Really nice improvement.
Here's some number:
With master (at 4c829bb):
#Benchmark,count,min(ns),max(ns),average(ns),min_cycles,max_cycles,average_cycles
Base58CheckEncode,131072,7708,8182,7845,20041,21275,20399
Base58Decode,294912,3526,3642,3584,9168,9470,9321
Base58Encode,196608,5491,5620,5544,14278,14612,14415
With this PR (rebased on master)
#Benchmark,count,min(ns),max(ns),average(ns),min_cycles,max_cycles,average_cycles
Base58CheckEncode,212992,4852,5097,4898,12618,13252,12737
Base58Decode,393216,2600,2747,2674,6762,7142,6953
Base58Encode,327680,3189,3583,3252,8292,9315,8455
So, encoding is 37.5% and 41% faster (respectively for Base58CheckEncode and Base58Encode), and decoding is 25% faster, on average.
Fuzzbawls
approved these changes
Jun 27, 2020
Collaborator
Fuzzbawls
left a comment
Fuzzbawls
left a comment
There was a problem hiding this comment.
ACK aa633c8
Master:
#Benchmark,count,min(ns),max(ns),average(ns),min_cycles,max_cycles,average_cycles
Base58CheckEncode,229376,4496,4529,4517,16185,16307,16261
Base58Decode,851968,1215,1224,1217,4374,4409,4384
Base58Encode,327680,3094,3145,3107,11139,11325,11187
Master + This PR:
#Benchmark,count,min(ns),max(ns),average(ns),min_cycles,max_cycles,average_cycles
Base58CheckEncode,360448,2891,2925,2900,10410,10533,10443
Base58Decode,1441792,713,721,717,2570,2597,2582
Base58Encode,589824,1767,1784,1777,6364,6423,6398
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Coming from: