Skip to content

Commit 9290142

Browse files
authored
perf: reduce allocations through memoization (#3219)
* make perf changes to memoize and remove gc pressure * add memoize elsewehre * lint * hash changes * update and changelog
1 parent 8b3f473 commit 9290142

File tree

13 files changed

+137
-14
lines changed

13 files changed

+137
-14
lines changed

block/internal/cache/generic_cache.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ func (c *Cache) isSeen(hash string) bool {
6161
func (c *Cache) setSeen(hash string, height uint64) {
6262
c.mu.Lock()
6363
defer c.mu.Unlock()
64+
if existing, ok := c.hashByHeight[height]; ok && existing == hash {
65+
c.hashes[existing] = true
66+
return
67+
}
6468
c.hashes[hash] = true
6569
c.hashByHeight[height] = hash
6670
}

block/internal/executing/executor.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,9 @@ func (e *Executor) initializeState() error {
263263
if err != nil {
264264
return fmt.Errorf("failed to get header at %d for sync check: %w", state.LastBlockHeight, err)
265265
}
266-
if !bytes.Equal(header.Hash(), raftState.Hash) {
267-
return fmt.Errorf("invalid state: block hash mismatch at height %d: raft=%x local=%x", state.LastBlockHeight, raftState.Hash, header.Hash())
266+
headerHash := header.MemoizeHash()
267+
if !bytes.Equal(headerHash, raftState.Hash) {
268+
return fmt.Errorf("invalid state: block hash mismatch at height %d: raft=%x local=%x", state.LastBlockHeight, raftState.Hash, headerHash)
268269
}
269270
}
270271
}
@@ -358,8 +359,9 @@ func (e *Executor) initializeState() error {
358359
if err != nil {
359360
return fmt.Errorf("get header at %d: %w", newState.LastBlockHeight, err)
360361
}
361-
if !bytes.Equal(header.Hash(), raftState.Hash) {
362-
return fmt.Errorf("CRITICAL: content mismatch after replay! local=%x raft=%x. This indicates a 'Dual-Store Conflict' where data diverged from Raft", header.Hash(), raftState.Hash)
362+
headerHash := header.MemoizeHash()
363+
if !bytes.Equal(headerHash, raftState.Hash) {
364+
return fmt.Errorf("CRITICAL: content mismatch after replay! local=%x raft=%x. This indicates a 'Dual-Store Conflict' where data diverged from Raft", headerHash, raftState.Hash)
363365
}
364366
}
365367
}
@@ -917,8 +919,9 @@ func (e *Executor) IsSyncedWithRaft(raftState *raft.RaftBlockState) (int, error)
917919
return 0, fmt.Errorf("get header for sync check at height %d: %w", raftState.Height, err)
918920
}
919921

920-
if !bytes.Equal(header.Hash(), raftState.Hash) {
921-
return 0, fmt.Errorf("block hash mismatch: %s != %s", header.Hash(), raftState.Hash)
922+
headerHash := header.MemoizeHash()
923+
if !bytes.Equal(headerHash, raftState.Hash) {
924+
return 0, fmt.Errorf("block hash mismatch: %s != %s", headerHash, raftState.Hash)
922925
}
923926

924927
return 0, nil

block/internal/syncing/da_retriever.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ func (r *daRetriever) tryDecodeHeader(bz []byte, daHeight uint64) *types.SignedH
312312
// Optimistically mark as DA included
313313
// This has to be done for all fetched DA headers prior to validation because P2P does not confirm
314314
// da inclusion. This is not an issue, as an invalid header will be rejected. There cannot be hash collisions.
315-
headerHash := header.Hash().String()
315+
headerHash := header.MemoizeHash().String()
316316
r.cache.SetHeaderDAIncluded(headerHash, daHeight, header.Height())
317317

318318
r.logger.Debug().

block/internal/syncing/p2p_handler.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ func (h *P2PHandler) ProcessHeight(ctx context.Context, height uint64, heightInC
100100
return err
101101
}
102102

103+
// Memoize hash before the header enters the event pipeline so that downstream
104+
// callers (processHeightEvent, TrySyncNextBlock) get cache hits.
105+
p2pHeader.MemoizeHash()
106+
103107
// further header validation (signature) is done in validateBlock.
104108
// we need to be sure that the previous block n-1 was executed before validating block n
105109
event := common.DAHeightEvent{

block/internal/syncing/syncer.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,8 +1166,9 @@ func (s *Syncer) IsSyncedWithRaft(raftState *raft.RaftBlockState) (int, error) {
11661166
s.logger.Error().Err(err).Uint64("height", raftState.Height).Msg("failed to get header for sync check")
11671167
return 0, fmt.Errorf("get header for sync check at height %d: %w", raftState.Height, err)
11681168
}
1169-
if !bytes.Equal(header.Hash(), raftState.Hash) {
1170-
return 0, fmt.Errorf("header hash mismatch: %x vs %x", header.Hash(), raftState.Hash)
1169+
headerHash := header.Hash()
1170+
if !bytes.Equal(headerHash, raftState.Hash) {
1171+
return 0, fmt.Errorf("header hash mismatch: %x vs %x", headerHash, raftState.Hash)
11711172
}
11721173

11731174
return 0, nil

pkg/store/cached_store.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ func (cs *CachedStore) GetHeader(ctx context.Context, height uint64) (*types.Sig
9797
return nil, err
9898
}
9999

100+
header.MemoizeHash()
101+
100102
// Add to cache
101103
cs.headerCache.Add(height, header)
102104

@@ -116,6 +118,8 @@ func (cs *CachedStore) GetBlockData(ctx context.Context, height uint64) (*types.
116118
return nil, nil, err
117119
}
118120

121+
header.MemoizeHash()
122+
119123
// Add to cache
120124
cs.blockDataCache.Add(height, &blockDataEntry{header: header, data: data})
121125

types/hashing.go

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,27 +41,65 @@ func (h *Header) HashLegacy() (Hash, error) {
4141
return hash[:], nil
4242
}
4343

44-
// Hash returns hash of the header
44+
// Hash returns the header hash. It reuses a memoized value if one has already
45+
// been prepared via MemoizeHash, but it does not write to the header itself.
4546
func (h *Header) Hash() Hash {
4647
if h == nil {
4748
return nil
4849
}
50+
if h.cachedHash != nil {
51+
return h.cachedHash
52+
}
4953

50-
slimHash, err := h.HashSlim()
51-
if err != nil {
54+
return h.computeHash()
55+
}
56+
57+
// MemoizeHash computes the header hash and stores it on the header for future
58+
// Hash() calls. Call this before publishing the header to shared goroutines or
59+
// caches.
60+
//
61+
// If a Header struct is reused (e.g. overwritten via FromProto or field
62+
// assignment), call InvalidateHash() first to clear the cached value before
63+
// calling MemoizeHash again. Failure to do so will return the stale cached hash.
64+
func (h *Header) MemoizeHash() Hash {
65+
if h == nil {
5266
return nil
5367
}
68+
if h.cachedHash != nil {
69+
return h.cachedHash
70+
}
5471

72+
hash := h.computeHash()
73+
if hash != nil {
74+
h.cachedHash = hash
75+
}
76+
return hash
77+
}
78+
79+
func (h *Header) computeHash() Hash {
80+
// Legacy hash takes precedence when legacy fields are present (backwards
81+
// compatibility). Slim hash is the canonical hash for all other headers.
5582
if h.Legacy != nil && !h.Legacy.IsZero() {
56-
legacyHash, err := h.HashLegacy()
57-
if err == nil {
83+
if legacyHash, err := h.HashLegacy(); err == nil {
5884
return legacyHash
5985
}
6086
}
6187

88+
slimHash, err := h.HashSlim()
89+
if err != nil {
90+
return nil
91+
}
6292
return slimHash
6393
}
6494

95+
// InvalidateHash clears the memoized hash, forcing recomputation on the next
96+
// Hash() call. Must be called after any mutation of Header fields.
97+
func (h *Header) InvalidateHash() {
98+
if h != nil {
99+
h.cachedHash = nil
100+
}
101+
}
102+
65103
// Hash returns hash of the Data
66104
func (d *Data) Hash() Hash {
67105
// Ignoring the marshal error for now to satisfy the go-header interface

types/hashing_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ func TestHeaderHash(t *testing.T) {
2121
}
2222

2323
hash1 := header.Hash()
24+
assert.Nil(t, header.cachedHash, "Hash() should not memoize")
25+
26+
memoizedHash := header.MemoizeHash()
27+
assert.Equal(t, hash1, memoizedHash)
28+
assert.NotNil(t, header.cachedHash, "MemoizeHash() should store the computed value")
2429

2530
headerBytes, err := header.MarshalBinary()
2631
require.NoError(t, err)
@@ -31,6 +36,8 @@ func TestHeaderHash(t *testing.T) {
3136
assert.Equal(t, Hash(expectedHash[:]), hash1, "Header hash should match manual calculation")
3237

3338
header.BaseHeader.Height = 2
39+
header.InvalidateHash()
40+
assert.Nil(t, header.cachedHash)
3441
hash2 := header.Hash()
3542
assert.NotEqual(t, hash1, hash2, "Different headers should have different hashes")
3643
}
@@ -143,3 +150,17 @@ func TestHeaderHashWithBytes(t *testing.T) {
143150
require.NoError(t, targetHeader.UnmarshalBinary(headerBytes))
144151
assert.Equal(t, hash1, targetHeader.Hash(), "HeaderHash should produce same result as Header.Hash()")
145152
}
153+
154+
func TestHeaderCloneDropsCachedHash(t *testing.T) {
155+
header := &Header{
156+
BaseHeader: BaseHeader{Height: 1, Time: 1234567890},
157+
DataHash: []byte("datahash"),
158+
}
159+
160+
header.MemoizeHash()
161+
require.NotNil(t, header.cachedHash)
162+
163+
clone := header.Clone()
164+
assert.Nil(t, clone.cachedHash, "Clone should not copy memoized hash state")
165+
assert.Equal(t, header.Hash(), clone.Hash())
166+
}

types/header.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ func HeaderFromContext(ctx context.Context) (Header, bool) {
2323
return Header{}, false
2424
}
2525

26+
// Clear the memoized hash on the returned copy. The caller may mutate
27+
// fields and call Hash() without knowing to call InvalidateHash() first,
28+
// which would return a stale cached value.
29+
h.InvalidateHash()
2630
return h, true
2731
}
2832

@@ -82,6 +86,10 @@ type Header struct {
8286
// representation but may still be required for backwards compatible binary
8387
// serialization (e.g. legacy signing payloads).
8488
Legacy *LegacyHeaderFields
89+
90+
// cachedHash holds the memoized result of MemoizeHash(). nil means cold.
91+
// Any caller that mutates header fields must call InvalidateHash() to clear it.
92+
cachedHash Hash
8593
}
8694

8795
// New creates a new Header.
@@ -250,6 +258,7 @@ func (h *Header) ApplyLegacyDefaults() {
250258
h.Legacy = &LegacyHeaderFields{}
251259
}
252260
h.Legacy.EnsureDefaults()
261+
h.InvalidateHash()
253262
}
254263

255264
// Clone creates a deep copy of the header, ensuring all mutable slices are
@@ -262,6 +271,7 @@ func (h Header) Clone() Header {
262271
clone.ValidatorHash = cloneBytes(h.ValidatorHash)
263272
clone.ProposerAddress = cloneBytes(h.ProposerAddress)
264273
clone.Legacy = h.Legacy.Clone()
274+
clone.cachedHash = nil
265275

266276
return clone
267277
}

types/serialization.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ func (h *Header) FromProto(other *pb.Header) error {
257257
if other == nil {
258258
return errors.New("header is nil")
259259
}
260+
h.InvalidateHash()
260261
if other.Version != nil {
261262
h.Version.Block = other.Version.Block
262263
h.Version.App = other.Version.App

0 commit comments

Comments
 (0)