sha512.cpp improvements #24589

pull kibnakamoto wants to merge 4 commits into bitcoin:master from kibnakamoto:master changing 1 files +70 −112

kibnakamoto commented at 6:07 PM on March 16, 2022: none

I had noticed that this code's transformation function was really inefficient so I decided to remake it. instead of writing each round individually (80 rounds), I decided to add round constants to a uint64_t array of 80 values(K). Instead of w1-.w2-w3... I used a Word array and iterated through the values with a for loop. I added the chunk of data in a single line to the Word array as a big-endian(using ReadBE64 function defined in common.h) instead of using 16 lines of code for it to serve the same purpose. All these changes made the code shorter by 42(207 lines to 165) lines without affecting the capability of the code and only making it more efficient. I tested the changes I made to the Transform function and it is working efficiently. I have also made the transform function more human-readable.
Update sha512.cpp 7285592220

improved efficiency

re-defined Transform function and made the code shorter by 42 lines.

208f4b6c53

in src/crypto/sha512.cpp:104 in 6b3de2757e

 196 | +    for(int i=0;i<8;i++) {
 197 | +        s[i] += V[i];
 198 | +    }
 199 |  }
 200 | -
 201 | +

RandyMcMillan commented at 3:56 PM on March 20, 2022:

line104: remove diff junk

mzumsande commented at 5:11 PM on March 20, 2022: member

only making it more efficient

I'm not an expert on this code at all, but have you tried to benchmark this? When I run ./src/bench/bench_bitcoin -filter=SHA512 five times, I get the following on master: | ns/byte | byte/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 2.07 | 484,146,384.56 | 0.9% | 0.02 | SHA512 | 2.07 | 482,601,259.40 | 1.2% | 0.02 | SHA512 | 2.07 | 482,578,901.65 | 1.2% | 0.02 | SHA512 | 2.05 | 487,611,973.99 | 0.3% | 0.02 | SHA512 | 2.07 | 483,403,547.70 | 1.1% | 0.02 | SHA512

with this PR: | ns/byte | byte/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 3.59 | 278,202,113.28 | 1.5% | 0.04 | SHA512 | 3.64 | 274,716,067.21 | 2.8% | 0.04 | SHA512 | 3.58 | 279,035,296.85 | 1.7% | 0.04 | SHA512 | 3.59 | 278,400,931.86 | 1.6% | 0.04 | SHA512 | 3.58 | 279,089,732.09 | 1.8% | 0.04 | SHA512

which suggests that this would make it slower by a factor of ~1.7
fanquake commented at 8:59 AM on March 21, 2022: member

Closing this. It hasn't compiled since it was PR'd, and, as pointed out by @mzumsande, it's not actually an improvement at all.
fanquake closed this on Mar 21, 2022
DrahtBot locked this on Mar 21, 2023

Contributors

Labels