From c35f3c6a8b2175c2583a7b1fe9fc09f67da4da1f Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Thu, 2 May 2024 18:27:40 -0600 Subject: [PATCH] perf(internal/bits): Speedup extended commit.BitArray() (backport #2959) (#2983) (#43) Speedup ExtendedCommit.BitArray() by making a direct constructor that does not go through mutexes. I expect this to be a 10x performance improvement. (It also removes the duffcopies and reduces setIndex proportion of time here) This removes this time (which is mostly coming from mutex calls): ![image](https://github.com/cometbft/cometbft/assets/6440154/da11d57d-6bea-40ea-a0fa-9209ea3e22f8) Later on we should instead make an API that lets us randomly sample from a bit array with no bit array copying needed. (But that makes the interface messier) --- #### 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
This is an automatic backport of pull request #2959 done by [Mergify](https://mergify.com). --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Dev Ojha Co-authored-by: Anton Kaliaev (cherry picked from commit 88c490ff4e80874b6ffec6fad5ab5ee358febc8b) --- ...eedup-initialized-bitarray-construction.md | 2 ++ libs/bits/bit_array.go | 22 ++++++++++++- libs/bits/bit_array_test.go | 33 ++++++++----------- types/block.go | 8 ++--- 4 files changed, 41 insertions(+), 24 deletions(-) create mode 100644 .changelog/unreleased/improvements/2959-speedup-initialized-bitarray-construction.md diff --git a/.changelog/unreleased/improvements/2959-speedup-initialized-bitarray-construction.md b/.changelog/unreleased/improvements/2959-speedup-initialized-bitarray-construction.md new file mode 100644 index 0000000000..7c1b2181d0 --- /dev/null +++ b/.changelog/unreleased/improvements/2959-speedup-initialized-bitarray-construction.md @@ -0,0 +1,2 @@ +- `[internal/bits]` 10x speedup creating initialized bitArrays, which speedsup extendedCommit.BitArray(). This is used in consensus vote gossip. + ([\#2959](https://github.com/cometbft/cometbft/pull/2841)). diff --git a/libs/bits/bit_array.go b/libs/bits/bit_array.go index 18daf50119..f9744f9c7b 100644 --- a/libs/bits/bit_array.go +++ b/libs/bits/bit_array.go @@ -32,7 +32,27 @@ func NewBitArray(bits int) *BitArray { } } -// Size returns the number of bits in the bitarray +// NewBitArrayFromFn returns a new bit array. +// It returns nil if the number of bits is zero. +// It initializes the `i`th bit to the value of `fn(i)`. +func NewBitArrayFromFn(bits int, fn func(int) bool) *BitArray { + if bits <= 0 { + return nil + } + bA := &BitArray{ + Bits: bits, + Elems: make([]uint64, (bits+63)/64), + } + for i := 0; i < bits; i++ { + v := fn(i) + if v { + bA.Elems[i/64] |= (uint64(1) << uint(i%64)) + } + } + return bA +} + +// Size returns the number of bits in the bitarray. func (bA *BitArray) Size() int { if bA == nil { return 0 diff --git a/libs/bits/bit_array_test.go b/libs/bits/bit_array_test.go index ad6a3a9f00..99b1d97a34 100644 --- a/libs/bits/bit_array_test.go +++ b/libs/bits/bit_array_test.go @@ -19,25 +19,17 @@ var ( full64bits = full16bits + full16bits + full16bits + full16bits ) -func randBitArray(bits int) (*BitArray, []byte) { +func randBitArray(bits int) *BitArray { src := cmtrand.Bytes((bits + 7) / 8) - bA := NewBitArray(bits) - for i := 0; i < len(src); i++ { - for j := 0; j < 8; j++ { - if i*8+j >= bits { - return bA, src - } - setBit := src[i]&(1< 0 - bA.SetIndex(i*8+j, setBit) - } + srcIndexToBit := func(i int) bool { + return src[i/8]&(1< 0 } - return bA, src + return NewBitArrayFromFn(bits, srcIndexToBit) } func TestAnd(t *testing.T) { - - bA1, _ := randBitArray(51) - bA2, _ := randBitArray(31) + bA1 := randBitArray(51) + bA2 := randBitArray(31) bA3 := bA1.And(bA2) var bNil *BitArray @@ -60,9 +52,8 @@ func TestAnd(t *testing.T) { } func TestOr(t *testing.T) { - - bA1, _ := randBitArray(51) - bA2, _ := randBitArray(31) + bA1 := randBitArray(57) + bA2 := randBitArray(31) bA3 := bA1.Or(bA2) bNil := (*BitArray)(nil) @@ -70,7 +61,7 @@ func TestOr(t *testing.T) { require.Equal(t, bA1.Or(nil), bA1) require.Equal(t, bNil.Or(nil), (*BitArray)(nil)) - if bA3.Bits != 51 { + if bA3.Bits != 57 { t.Error("Expected max bits") } if len(bA3.Elems) != len(bA1.Elems) { @@ -82,6 +73,10 @@ func TestOr(t *testing.T) { t.Error("Wrong bit from bA3", i, bA1.GetIndex(i), bA2.GetIndex(i), bA3.GetIndex(i)) } } + if bA3.getNumTrueIndices() == 0 { + t.Error("Expected at least one true bit. " + + "This has a false positive rate that is less than 1 in 2^80 (cryptographically improbable).") + } } func TestSub(t *testing.T) { @@ -278,7 +273,7 @@ func TestEmptyFull(t *testing.T) { func TestUpdateNeverPanics(t *testing.T) { newRandBitArray := func(n int) *BitArray { - ba, _ := randBitArray(n) + ba := randBitArray(n) return ba } pairs := []struct { diff --git a/types/block.go b/types/block.go index 9d3436f2af..52ac292cc9 100644 --- a/types/block.go +++ b/types/block.go @@ -846,12 +846,12 @@ func (commit *Commit) Size() int { // Implements VoteSetReader. func (commit *Commit) BitArray() *bits.BitArray { if commit.bitArray == nil { - commit.bitArray = bits.NewBitArray(len(commit.Signatures)) - for i, commitSig := range commit.Signatures { + initialBitFn := func(i int) bool { // TODO: need to check the BlockID otherwise we could be counting conflicts, - // not just the one with +2/3 ! - commit.bitArray.SetIndex(i, !commitSig.Absent()) + // not just the one with +2/3 ! + return !commit.Signatures[i].Absent() } + commit.bitArray = bits.NewBitArrayFromFn(len(commit.Signatures), initialBitFn) } return commit.bitArray }