The use of std::string kills perf in many ways - every construction and modification may perform a heap allocation. That may also throw an exception, and ultimately terminate the process.
I hacked up a change that uses a char [BLOCK_BYTES] instead of a std::string, and I consistently get a 25% reduction in time for the slow test. To enter the new implementation, I introduced a new update() overload.
This new implementation must be a base, and the existing public methods should call into it. Then I noticed a few things in the code that I prefer to fix as I go:
- Freely hanging methods that takes references (to instance members). I want to make those private methods of the class.
- The
final() method is not cohesive - it does 3 things: it finishes the work, produces a result (as a std::string), and resets the sate. I would like to split all those 3 steps in separate methods.
This line of thinking is leading me to a new class sha1 or even better - put it in a namespace - sha1::hash. We can keep the existing class SHA1 with the public API it has today. It will simply maintain a member of sha1::hash, and will delegate all the work to it.
To summarize it, I want to have an efficient low-level class, so that clients who care about perf and reliability, can use it directly. We can keep the existing, wrapper around for convenience and back-compat purposes.
Please let me know what you think about this.
The use of
std::stringkills perf in many ways - every construction and modification may perform a heap allocation. That may also throw an exception, and ultimately terminate the process.I hacked up a change that uses a
char [BLOCK_BYTES]instead of astd::string, and I consistently get a 25% reduction in time for the slow test. To enter the new implementation, I introduced a newupdate()overload.This new implementation must be a base, and the existing public methods should call into it. Then I noticed a few things in the code that I prefer to fix as I go:
final()method is not cohesive - it does 3 things: it finishes the work, produces a result (as astd::string), and resets the sate. I would like to split all those 3 steps in separate methods.This line of thinking is leading me to a new class
sha1or even better - put it in a namespace -sha1::hash. We can keep the existing classSHA1with the public API it has today. It will simply maintain a member ofsha1::hash, and will delegate all the work to it.To summarize it, I want to have an efficient low-level class, so that clients who care about perf and reliability, can use it directly. We can keep the existing, wrapper around for convenience and back-compat purposes.
Please let me know what you think about this.