core/rawdb: reduce allocations in rawdb.ReadHeaderNumber

[MariusVanDerWijden]

This is something interesting I came across during my benchmarks, we spent ~3.8% of all allocations allocating the header number on the heap.

(pprof) list GetHeaderByHash
Total: 38197204475
ROUTINE ======================== github.com/ethereum/go-ethereum/core.(*BlockChain).GetHeaderByHash in github.com/ethereum/go-ethereum/core/blockchain_reader.go
         0 5786566117 (flat, cum) 15.15% of Total
         .          .     79:func (bc *BlockChain) GetHeaderByHash(hash common.Hash) *types.Header {
         . 5786566117     80: return bc.hc.GetHeaderByHash(hash)
         .          .     81:}
         .          .     82:
         .          .     83:// GetHeaderByNumber retrieves a block header from the database by number,
         .          .     84:// caching it (associated with its hash) if found.
         .          .     85:func (bc *BlockChain) GetHeaderByNumber(number uint64) *types.Header {
ROUTINE ======================== github.com/ethereum/go-ethereum/core.(*HeaderChain).GetHeaderByHash in github.com/ethereum/go-ethereum/core/headerchain.go
         0 5786566117 (flat, cum) 15.15% of Total
         .          .    404:func (hc *HeaderChain) GetHeaderByHash(hash common.Hash) *types.Header {
         . 1471264309    405: number := hc.GetBlockNumber(hash)
         .          .    406: if number == nil {
         .          .    407:  return nil
         .          .    408: }
         . 4315301808    409: return hc.GetHeader(hash, *number)
         .          .    410:}
         .          .    411:
         .          .    412:// HasHeader checks if a block header is present in the database or not.
         .          .    413:// In theory, if header is present in the database, all relative components
         .          .    414:// like td and hash->number should be present too.
(pprof) list GetBlockNumber
Total: 38197204475
ROUTINE ======================== github.com/ethereum/go-ethereum/core.(*HeaderChain).GetBlockNumber in github.com/ethereum/go-ethereum/core/headerchain.go
  94438817 1471264309 (flat, cum)  3.85% of Total
         .          .    100:func (hc *HeaderChain) GetBlockNumber(hash common.Hash) *uint64 {
  94438817   94438817    101: if cached, ok := hc.numberCache.Get(hash); ok {
         .          .    102:  return &cached
         .          .    103: }
         . 1376270828    104: number := rawdb.ReadHeaderNumber(hc.chainDb, hash)
         .          .    105: if number != nil {
         .     554664    106:  hc.numberCache.Add(hash, *number)
         .          .    107: }
         .          .    108: return number
         .          .    109:}
         .          .    110:
         .          .    111:type headerWriteResult struct {
(pprof) list ReadHeaderNumber
Total: 38197204475
ROUTINE ======================== github.com/ethereum/go-ethereum/core/rawdb.ReadHeaderNumber in github.com/ethereum/go-ethereum/core/rawdb/accessors_chain.go
 204606513 1376270828 (flat, cum)  3.60% of Total
         .          .    146:func ReadHeaderNumber(db ethdb.KeyValueReader, hash common.Hash) *uint64 {
 109577863 1281242178    147: data, _ := db.Get(headerNumberKey(hash))
         .          .    148: if len(data) != 8 {
         .          .    149:  return nil
         .          .    150: }
  95028650   95028650    151: number := binary.BigEndian.Uint64(data)
         .          .    152: return &number
         .          .    153:}
         .          .    154:
         .          .    155:// WriteHeaderNumber stores the hash->number mapping.
         .          .    156:func WriteHeaderNumber(db ethdb.KeyValueWriter, hash common.Hash, number uint64) {

Opening this to discuss the idea, I know that rawdb.EmptyNumber is not a great name for the variable, open to suggestions

[omerfirmak]

why can't we just make this return (uint64, bool) to signify that the uint64 returned is valid or not?

[omerfirmak]

or we could use math.MaxUint64 as the sentinel value

[MariusVanDerWijden]

Yeah, thats a way better idea

[cskiraly]

Here we should be careful. Previously we changed the undelying value, now we change the copy. Is this OK?

[cskiraly]

This is fine, it was newly allocated anyway.

[cskiraly]

This is fine, it was newly allocated anyway.