Quite often the hash is never used, and I cut down a custom RPC from 46 minutes to 21 by eliminating it. Adding a CTransaction destructor checking its final fHaveHash state shows it also occurs quite often with the master reference code.
CTransaction: Skip hashing until it is needed #6225
pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:skip_unnec_txhash changing 2 files +5 −1-
luke-jr commented at 4:26 AM on June 3, 2015: member
-
CTransaction: Skip hashing until it is needed a6ed0202b8
-
sipa commented at 4:29 AM on June 3, 2015: member
This is actually not thread-safe in theory.
-
-
sipa commented at 5:55 AM on June 3, 2015: member
Yes. Its hash it computed at creation time, and afterwards it is entirely immutable, so it is trivially safe to use in a multithreaded way. Delaying the computation of the hash may in theory cause problems when two threads ask the hash simultaneously for the first time, but in practice it is likely fine. I'm surprised it makes a difference for a practical application, though.
-
laanwj commented at 10:31 AM on June 3, 2015: member
Delaying the computation of the hash may in theory cause problems when two threads ask the hash simultaneously for the first time, but in practice it is likely fine
I don't want to gamble on this. Stability, even against obscure race conditions, is much more important than performance.
Isn't there a way you can rewrite your "custom RPC" in a more efficient way without changing CTransaction (eg using MutableTransaction as long as the hash is not needed)?
-
luke-jr commented at 6:32 PM on June 3, 2015: member
@laanwj Is there a way to parse a block without using CTransaction for the contents? I'm not aware of any. If thread safety is a concern (do we actually rely on it?), locking some shared mutex in UpdateHash should be sufficient and probably harmless to performance?
-
laanwj commented at 6:39 PM on June 3, 2015: member
Not currently. But if there's a use-case for high-speed iteration through blocks without hashing the transactions, maybe that could be added in a way that doesn't conflict with consensus usages.
There's something to be said for separating the primitive/consensus data types from what is used in other processing. It is unacceptably risky to change the functioning of a primitive data type to speed up non-consensus data processing. That should be separate.
Locking/unlocking a mutex can have significant overhead, possibly worse than hashing a few bytes.
-
laanwj commented at 4:44 PM on June 5, 2015: member
Closing this, too risky.
- laanwj closed this on Jun 5, 2015
- MarcoFalke locked this on Sep 8, 2021
