Skip to content

Commit

Permalink
perf(consensus/blockstore): Remove validate basic call from LoadBlock…
Browse files Browse the repository at this point in the history
…Meta (backport cometbft#2964) (cometbft#2998)

Our gossip block parts routine calls `Blockstore.LoadBlockMeta` (as do
things in blocksync). This currently takes some time due to
ValidateBasic:

![image](https://github.com/cometbft/cometbft/assets/6440154/2f4471b0-0744-44de-ab94-4a9a4596712c)

However note that we only save validated data to the blockstore. We only
do it in:
- Commit:
https://github.com/cometbft/cometbft/blob/main/internal/consensus/state.go#L1867
- Blocksync after validation:
https://github.com/cometbft/cometbft/blob/main/internal/consensus/state.go#L1867

Hence the validate basic time is wasted.

This should eventually just go to an LRU cache to even avoid the proto
unmarshalling (as should ~everything in blockstore) but we want this
anyway to reduce the computational overhead,

WRT live consensus, in main this only helps with catchup for live
syncing nodes, I haven't checked on v0.47.x, but the cpuprofile suggests
it may help with active block gossip time as well?

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2964 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
  • Loading branch information
3 people committed May 4, 2024
1 parent 1c81cc9 commit 10dfc54
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[blockstore]` Remove a redundant `Header.ValidateBasic` call in `LoadBlockMeta`, 75% reducing this time.
([\#2964](https://github.com/cometbft/cometbft/pull/2964))
2 changes: 1 addition & 1 deletion store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (bs *BlockStore) LoadBlockMeta(height int64) *types.BlockMeta {
panic(fmt.Errorf("unmarshal to cmtproto.BlockMeta: %w", err))
}

blockMeta, err := types.BlockMetaFromProto(pbbm)
blockMeta, err := types.BlockMetaFromTrustedProto(pbbm)
if err != nil {
panic(fmt.Errorf("error from proto blockMeta: %w", err))
}
Expand Down
10 changes: 9 additions & 1 deletion types/block_meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ func (bm *BlockMeta) ToProto() *cmtproto.BlockMeta {
}

func BlockMetaFromProto(pb *cmtproto.BlockMeta) (*BlockMeta, error) {
bm, err := BlockMetaFromTrustedProto(pb)
if err != nil {
return nil, err
}
return bm, bm.ValidateBasic()
}

func BlockMetaFromTrustedProto(pb *cmtproto.BlockMeta) (*BlockMeta, error) {
if pb == nil {
return nil, errors.New("blockmeta is empty")
}
Expand All @@ -62,7 +70,7 @@ func BlockMetaFromProto(pb *cmtproto.BlockMeta) (*BlockMeta, error) {
bm.Header = h
bm.NumTxs = int(pb.NumTxs)

return bm, bm.ValidateBasic()
return bm, nil
}

// ValidateBasic performs basic validation.
Expand Down
2 changes: 1 addition & 1 deletion types/block_meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestBlockMeta_ToProto(t *testing.T) {
t.Run(tt.testName, func(t *testing.T) {
pb := tt.bm.ToProto()

bm, err := BlockMetaFromProto(pb)
bm, err := BlockMetaFromTrustedProto(pb)

if !tt.expErr {
require.NoError(t, err, tt.testName)
Expand Down

0 comments on commit 10dfc54

Please sign in to comment.