wire: Optimize writes to hashers #3590
Conversation
jrick
force-pushed
the
optimize_hashing
branch
4 times, most recently
from
84a4633 to
27b36d8
Compare
This special cases writes to bytes.Buffer, which is always the writer type written to by WriteMessageN. There are several optimizations that can be implemented by special casing this type: First, pulling temporary short buffers from binary freelist can be skipped entirely, and instead the binary encoding of integers can be appended directly to its existing capacity. This avoids the synchronization cost to add and remove buffers from the free list, and for applications which only ever write wire messages with WriteMessageN, the allocation and ongoing garbage collection scanning cost to for these buffers can be completely skipped. Second, special casing the buffer type in WriteVarString saves us from creating a temporary heap copy of a string, as the buffer's WriteString method can be used instead. Third, special casing the buffer allows WriteMessageN to calculate the serialize size and grow its buffer so all remaining appends for writing block and transactions will not have to reallocate the buffer's backing allocation. This same optimization can be applied to other messages in the future.
This adds two additional type cases to the optimized writes for BLAKE-256 (used by transactions and the original PoW algorithm) and BLAKE-3 (used by the current PoW algorithm). It also updates both the block header and transaction code to use these hashers during the calculation of block and transaction hashes.
jrick
force-pushed
the
optimize_hashing
branch
from
27b36d8 to
c4ba173
Compare
davecgh
left a comment
davecgh
left a comment
There was a problem hiding this comment.
I believe the go.mod will need an update because this makes blake256 a direct dep instead of indirect via chainhash.
| witnessHash := msg.mustHash(hasher, TxSerializeOnlyWitness) | ||
| hasher.Reset() | ||
|
|
||
| hasher.Write(prefixHash[:]) |
There was a problem hiding this comment.
It doesn't matter much since they ultimately call the same internal code, but these should probably use the error-free WriteBytes since it doesn't an error or use the number of bytes written.
| _ = writeBlockHeader(buf, 0, h) | ||
|
|
||
| return chainhash.HashH(buf.Bytes()) | ||
| hasher := blake256.NewHasher256() |
There was a problem hiding this comment.
The comment on the function should be updated to explicitly state it uses BLAKE-256 now because it returns chainhash.Hash. Previously, it would use whatever hashing method chainhash implemented and that capability was even used to easily change the hash function when Decred was first forked from the btcd code.
This silently breaks that ability.
| func (msg *MsgTx) TxHash() chainhash.Hash { | ||
| // TxHash should always calculate a non-witnessed hash. | ||
| return chainhash.HashH(msg.mustSerialize(TxSerializeNoWitness)) | ||
| return msg.mustHash(blake256.NewHasher256(), TxSerializeNoWitness) |
There was a problem hiding this comment.
The same applies regarding commenting the func that the hash function now is explicitly BLAKE-256 since it no longer follows what chainhash does.
I suppose an alternative would be to add code to chainhash to provide a hasher, but I'm fairly certain that would just bring back an allocation because it would have to return a generic hash.Hasher or similar to avoid a direct dependency in the public API on the concrete type.
2 participants
This adds two additional type cases to the optimized writes for BLAKE-256 (used by transactions and the original PoW algorithm) and BLAKE-3 (used by the current PoW algorithm). It also updates both the block header and transaction code to use these hashers during the calculation of block and transaction hashes.
Requires #3589.