From 25519079a5332e39a68c4e5b7a77f477ee6d0f89 Mon Sep 17 00:00:00 2001 From: Jeff Winkler Date: Mon, 25 Oct 2021 21:28:59 +0000 Subject: [PATCH 1/3] hotstuff: improve the latency of hotstuff verification This will remove the requirement for a mutex, and just use atomic instructions instead. --- consensus/hotstuff/verification/common.go | 53 +++++++++++++++++------ 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/consensus/hotstuff/verification/common.go b/consensus/hotstuff/verification/common.go index 6d211afd4e8..fa8add926fa 100644 --- a/consensus/hotstuff/verification/common.go +++ b/consensus/hotstuff/verification/common.go @@ -2,7 +2,9 @@ package verification import ( "fmt" - "sync" + "sync/atomic" + "unsafe" + _ "unsafe" "github.com/onflow/flow-go/consensus/hotstuff/model" "github.com/onflow/flow-go/crypto" @@ -68,24 +70,42 @@ func checkVotesValidity(votes []*model.Vote) error { return nil } +type aggregate struct { + lastStakingSigners map[flow.Identifier]*flow.Identity + lastStakingKey crypto.PublicKey +} + // stakingKeysAggregator is a structure that aggregates the staking // public keys for QC verifications. type stakingKeysAggregator struct { - lastStakingSigners map[flow.Identifier]*flow.Identity - lastStakingKey crypto.PublicKey - sync.RWMutex + current unsafe.Pointer // *aggregate type + inner *aggregate } // creates a new staking keys aggregator func newStakingKeysAggregator() *stakingKeysAggregator { - aggregator := &stakingKeysAggregator{ + aggregator := &stakingKeysAggregator{} + + aggregator.inner = &aggregate{ lastStakingSigners: map[flow.Identifier]*flow.Identity{}, lastStakingKey: NeutralBLSPublicKey(), - RWMutex: sync.RWMutex{}, } + + aggregator.current = unsafe.Pointer(aggregator.inner) + return aggregator } +// use this function to obtain the current config +func (s *stakingKeysAggregator) getCurrent() *aggregate { + return (*aggregate)(atomic.LoadPointer(&s.current)) +} + +// periodically and sets aggregate struct as current using this function. +func (s *stakingKeysAggregator) updateCurrent(agg *aggregate) { + atomic.StorePointer(&s.current, unsafe.Pointer(agg)) +} + // aggregatedStakingKey returns the aggregated public key of the input signers. func (s *stakingKeysAggregator) aggregatedStakingKey(signers flow.IdentityList) (crypto.PublicKey, error) { @@ -98,10 +118,10 @@ func (s *stakingKeysAggregator) aggregatedStakingKey(signers flow.IdentityList) // algorithm. The worst case happens when the 2/3 latest signers and the 2/3 new signers only // have 1/3 in common (the minimum common ratio). - s.RLock() - lastSet := s.lastStakingSigners - lastKey := s.lastStakingKey - s.RUnlock() + inner := s.getCurrent() + + lastSet := inner.lastStakingSigners + lastKey := inner.lastStakingKey // get the signers delta and update the last list for the next comparison newSignerKeys, missingSignerKeys, updatedSignerSet := identitiesDeltaKeys(signers, lastSet) @@ -119,10 +139,15 @@ func (s *stakingKeysAggregator) aggregatedStakingKey(signers flow.IdentityList) // update the latest list and public key. The current thread may overwrite the result of another thread // but the greedy algorithm remains valid. - s.Lock() - s.lastStakingSigners = updatedSignerSet - s.lastStakingKey = updatedKey - s.Unlock() + + // create a new inner struct to hold the data, in a thread-safe way + nextInner := &aggregate{ + lastStakingSigners: updatedSignerSet, + lastStakingKey: updatedKey, + } + + s.updateCurrent(nextInner) + return updatedKey, nil } From d12fa689f9e7faf9ecec1c9b86b396dcbaa781a7 Mon Sep 17 00:00:00 2001 From: Jeff Winkler Date: Mon, 25 Oct 2021 16:39:08 -0500 Subject: [PATCH 2/3] Update common.go --- consensus/hotstuff/verification/common.go | 1 - 1 file changed, 1 deletion(-) diff --git a/consensus/hotstuff/verification/common.go b/consensus/hotstuff/verification/common.go index fa8add926fa..0fafe85615b 100644 --- a/consensus/hotstuff/verification/common.go +++ b/consensus/hotstuff/verification/common.go @@ -4,7 +4,6 @@ import ( "fmt" "sync/atomic" "unsafe" - _ "unsafe" "github.com/onflow/flow-go/consensus/hotstuff/model" "github.com/onflow/flow-go/crypto" From db03671ad0a108df5a51e1428d83391a14cf3533 Mon Sep 17 00:00:00 2001 From: Jeff Winkler Date: Sun, 7 Nov 2021 00:23:05 -0500 Subject: [PATCH 3/3] Update common.go --- consensus/hotstuff/verification/common.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/consensus/hotstuff/verification/common.go b/consensus/hotstuff/verification/common.go index 0fafe85615b..86afe9173f3 100644 --- a/consensus/hotstuff/verification/common.go +++ b/consensus/hotstuff/verification/common.go @@ -100,7 +100,7 @@ func (s *stakingKeysAggregator) getCurrent() *aggregate { return (*aggregate)(atomic.LoadPointer(&s.current)) } -// periodically and sets aggregate struct as current using this function. +// periodically sets aggregate struct as current func (s *stakingKeysAggregator) updateCurrent(agg *aggregate) { atomic.StorePointer(&s.current, unsafe.Pointer(agg)) } @@ -145,6 +145,7 @@ func (s *stakingKeysAggregator) aggregatedStakingKey(signers flow.IdentityList) lastStakingKey: updatedKey, } + // swap the struct out s.updateCurrent(nextInner) return updatedKey, nil