grin icon indicating copy to clipboard operation
grin copied to clipboard

block.hash() unnecessarily expensive to call

Open antiochp opened this issue 5 years ago • 0 comments

We call write() on Writeable when serializing things to bytes. Not just for serializing full data but also when we serialize during a call to hash().

Block hash -> BlockHeader hash -> ProofOfWork hash -> Proof hash

Every call to block.hash() or header.hash() delegates to the following code to produce the data to be hashed -

https://github.com/mimblewimble/grin/blob/098d25e5696d1d97e4fbfabb80f27063caccd0f3/core/src/pow/types.rs#L455-L472

We build the bitvec "on the fly" every time we call hash(). Ideally hash() on any type is as cheap as possible.

I suspect, but have no benchmarks or data to confirm, that this is relatively expensive over time. We make a lot of calls to block.hash() and header.hash().

Possible improvement -

  1. Consider initializing the bitvec when we initially read the Proof instance.
  2. Use this "cached" value when we write it in Hash serialization mode.
  3. Ignore the "cached" value when writing full data.
  4. Make sure nonces or edge_bits cannot be written to or modified in unexpected ways, leaving the cache stale.

Related - https://github.com/mimblewimble/grin/issues/2584

Maybe we don't need to cache "hashes" in a general way (yet), maybe simply making block.hash() cheap will give us most of the benefit for the least effort.

antiochp avatar Jun 29 '20 07:06 antiochp