From 74e25a315cb0c6599d651dc1f96e862bf7bab2f2 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Thu, 2 May 2024 22:34:13 +0800
Subject: [PATCH] perf(internal/bits): Speedup extended commit.BitArray()
(backport #2959) (#2983)
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: Dev Ojha
Co-authored-by: Anton Kaliaev
---
...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
}