diff --git a/.changelog/unreleased/improvements/3119-speedup-valset-GetByAddress-usages.md b/.changelog/unreleased/improvements/3119-speedup-valset-GetByAddress-usages.md new file mode 100644 index 0000000000..93350080d5 --- /dev/null +++ b/.changelog/unreleased/improvements/3119-speedup-valset-GetByAddress-usages.md @@ -0,0 +1,2 @@ +- [`types`] Make a new method `GetByAddressMut` for `ValSet`, which does not copy the returned validator. + ([\#3119](https://github.com/cometbft/cometbft/issues/3119) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b1b11c26..9b9147c5cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ * [#86](https://github.com/osmosis-labs/cometbft/pull/86) Comment out expensive debug logs * [#91](https://github.com/osmosis-labs/cometbft/pull/91) perf(consensus): Minor improvement by making add vote only do one peer set mutex call, not 3 (#3156) * [#93](https://github.com/osmosis-labs/cometbft/pull/93) perf(consensus): Make some consensus reactor messages take RLock's not WLock's (#3159) +* [#95](https://github.com/osmosis-labs/cometbft/pull/95) perf(types) Make a new method `GetByAddressMut` for `ValSet`, which does not copy the returned validator. (#3129) + ## v0.37.4-v25-osmo-5 diff --git a/consensus/state.go b/consensus/state.go index d885be7342..d712c91ad6 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -1834,7 +1834,7 @@ func (cs *State) recordMetrics(height int64, block *types.Block) { ) for _, ev := range block.Evidence.Evidence { if dve, ok := ev.(*types.DuplicateVoteEvidence); ok { - if _, val := cs.Validators.GetByAddress(dve.VoteA.ValidatorAddress); val != nil { + if _, val := cs.Validators.GetByAddressMut(dve.VoteA.ValidatorAddress); val != nil { byzantineValidatorsCount++ byzantineValidatorsPower += val.VotingPower } @@ -2381,7 +2381,7 @@ func (cs *State) calculatePrevoteMessageDelayMetrics() { var votingPowerSeen int64 for _, v := range pl { - _, val := cs.Validators.GetByAddress(v.ValidatorAddress) + _, val := cs.Validators.GetByAddressMut(v.ValidatorAddress) votingPowerSeen += val.VotingPower if votingPowerSeen >= cs.Validators.TotalVotingPower()*2/3+1 { cs.metrics.QuorumPrevoteDelay.With("proposer_address", cs.Validators.GetProposer().Address.String()).Set(v.Timestamp.Sub(cs.Proposal.Timestamp).Seconds()) diff --git a/consensus/types/round_state.go b/consensus/types/round_state.go index 6749d4265a..b992964716 100644 --- a/consensus/types/round_state.go +++ b/consensus/types/round_state.go @@ -121,7 +121,7 @@ func (rs *RoundState) RoundStateSimple() RoundStateSimple { } addr := rs.Validators.GetProposer().Address - idx, _ := rs.Validators.GetByAddress(addr) + idx, _ := rs.Validators.GetByAddressMut(addr) return RoundStateSimple{ HeightRoundStep: fmt.Sprintf("%d/%d/%d", rs.Height, rs.Round, rs.Step), @@ -140,7 +140,7 @@ func (rs *RoundState) RoundStateSimple() RoundStateSimple { // NewRoundEvent returns the RoundState with proposer information as an event. func (rs *RoundState) NewRoundEvent() types.EventDataNewRound { addr := rs.Validators.GetProposer().Address - idx, _ := rs.Validators.GetByAddress(addr) + idx, _ := rs.Validators.GetByAddressMut(addr) return types.EventDataNewRound{ Height: rs.Height, diff --git a/internal/test/commit.go b/internal/test/commit.go index 07b288ae66..989a086f17 100644 --- a/internal/test/commit.go +++ b/internal/test/commit.go @@ -57,7 +57,7 @@ func MakeCommit(blockID types.BlockID, height int64, round int32, valSet *types. } addr := pk.Address() - idx, _ := valSet.GetByAddress(addr) + idx, _ := valSet.GetByAddressMut(addr) if idx < 0 { return nil, fmt.Errorf("validator with address %s not in validator set", addr) } diff --git a/types/validation.go b/types/validation.go index 00902229b0..c15134ae0b 100644 --- a/types/validation.go +++ b/types/validation.go @@ -247,7 +247,7 @@ func verifyCommitBatch( if lookUpByIndex { val = vals.Validators[idx] } else { - valIdx, val = vals.GetByAddress(commitSig.ValidatorAddress) + valIdx, val = vals.GetByAddressMut(commitSig.ValidatorAddress) // if the signature doesn't belong to anyone in the validator set // then we just skip over it diff --git a/types/validator_set.go b/types/validator_set.go index 671693f9d5..95cfe546ce 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -268,9 +268,21 @@ func (vals *ValidatorSet) HasAddress(address []byte) bool { // GetByAddress returns an index of the validator with address and validator // itself (copy) if found. Otherwise, -1 and nil are returned. func (vals *ValidatorSet) GetByAddress(address []byte) (index int32, val *Validator) { + i, val := vals.GetByAddressMut(address) + if i == -1 { + return -1, nil + } + return i, val.Copy() +} + +// GetByAddressMut returns an index of the validator with address and the +// direct validator object if found. Mutations on this return value affect the validator set. +// This method should be used by callers who will not mutate Val. +// Otherwise, -1 and nil are returned. +func (vals *ValidatorSet) GetByAddressMut(address []byte) (index int32, val *Validator) { for idx, val := range vals.Validators { if bytes.Equal(val.Address, address) { - return int32(idx), val.Copy() + return int32(idx), val } } return -1, nil @@ -431,7 +443,7 @@ func verifyUpdates( ) (tvpAfterUpdatesBeforeRemovals int64, err error) { delta := func(update *Validator, vals *ValidatorSet) int64 { - _, val := vals.GetByAddress(update.Address) + _, val := vals.GetByAddressMut(update.Address) if val != nil { return update.VotingPower - val.VotingPower } @@ -478,7 +490,7 @@ func numNewValidators(updates []*Validator, vals *ValidatorSet) int { func computeNewPriorities(updates []*Validator, vals *ValidatorSet, updatedTotalVotingPower int64) { for _, valUpdate := range updates { address := valUpdate.Address - _, val := vals.GetByAddress(address) + _, val := vals.GetByAddressMut(address) if val == nil { // add val // Set ProposerPriority to -C*totalVotingPower (with C ~= 1.125) to make sure validators can't @@ -543,7 +555,7 @@ func verifyRemovals(deletes []*Validator, vals *ValidatorSet) (votingPower int64 removedVotingPower := int64(0) for _, valUpdate := range deletes { address := valUpdate.Address - _, val := vals.GetByAddress(address) + _, val := vals.GetByAddressMut(address) if val == nil { return removedVotingPower, fmt.Errorf("failed to find validator %X to remove", address) } diff --git a/types/vote_set.go b/types/vote_set.go index 7ffebc16ef..4a90842f16 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -404,7 +404,7 @@ func (voteSet *VoteSet) GetByAddress(address []byte) *Vote { } voteSet.mtx.Lock() defer voteSet.mtx.Unlock() - valIndex, val := voteSet.valSet.GetByAddress(address) + valIndex, val := voteSet.valSet.GetByAddressMut(address) if val == nil { panic("GetByAddress(address) returned nil") }