Skip to content

perf(internal/bits): Speedup extended commit.BitArray() (backport #2959)#2983

Merged
melekes merged 3 commits intov0.37.xfrom
mergify/bp/v0.37.x/pr-2959
May 2, 2024
Merged

perf(internal/bits): Speedup extended commit.BitArray() (backport #2959)#2983
melekes merged 3 commits intov0.37.xfrom
mergify/bp/v0.37.x/pr-2959

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented May 2, 2024

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

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

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

This is an automatic backport of pull request #2959 done by [Mergify](https://mergify.com).

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

(cherry picked from commit edb297b)

# Conflicts:
#	libs/bits/bit_array.go
#	libs/bits/bit_array_test.go
#	types/block.go
@mergify mergify bot requested a review from a team as a code owner May 2, 2024 13:23
@mergify mergify bot added the conflicts label May 2, 2024
@mergify
Copy link
Contributor Author

mergify bot commented May 2, 2024

Cherry-pick of edb297b has failed:

On branch mergify/bp/v0.37.x/pr-2959
Your branch is up to date with 'origin/v0.37.x'.

You are currently cherry-picking commit edb297be1.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .changelog/unreleased/improvements/2959-speedup-initialized-bitarray-construction.md

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   libs/bits/bit_array.go
	both modified:   libs/bits/bit_array_test.go
	both modified:   types/block.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@melekes melekes merged commit 1562cdb into v0.37.x May 2, 2024
@melekes melekes deleted the mergify/bp/v0.37.x/pr-2959 branch May 2, 2024 14:34
czarcas7ic pushed a commit to osmosis-labs/cometbft that referenced this pull request May 2, 2024
…etbft#2959) (cometbft#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
<hr>This is an automatic backport of pull request cometbft#2959 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>
czarcas7ic added a commit to osmosis-labs/cometbft that referenced this pull request May 3, 2024
…etbft#2959) (cometbft#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
<hr>This is an automatic backport of pull request cometbft#2959 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
mergify bot added a commit to osmosis-labs/cometbft that referenced this pull request May 3, 2024
…etbft#2959) (cometbft#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
<hr>This is an automatic backport of pull request cometbft#2959 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 88c490f)
mergify bot added a commit to osmosis-labs/cometbft that referenced this pull request May 3, 2024
…etbft#2959) (cometbft#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
<hr>This is an automatic backport of pull request cometbft#2959 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 88c490f)
czarcas7ic added a commit to osmosis-labs/cometbft that referenced this pull request May 3, 2024
…etbft#2959) (cometbft#2983) (#43) (#54)

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
<hr>This is an automatic backport of pull request cometbft#2959 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 88c490f)

Co-authored-by: Adam Tucker <adam@osmosis.team>
czarcas7ic added a commit to osmosis-labs/cometbft that referenced this pull request May 3, 2024
…etbft#2959) (cometbft#2983) (#43) (#55)

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
<hr>This is an automatic backport of pull request cometbft#2959 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 88c490f)

Co-authored-by: Adam Tucker <adam@osmosis.team>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants