Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(internal/bits): Speedup extended commit.BitArray() (backport #29… #43

Merged
merged 1 commit into from
May 3, 2024

Conversation

czarcas7ic
Copy link
Member

…59) (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

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



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

…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 czarcas7ic added S:backport/v24 backport to the osmo-v24/v0.37.4 branch S:backport/v25 backport to the osmo-v25/v0.37.4 branch labels May 2, 2024
@czarcas7ic czarcas7ic marked this pull request as ready for review May 2, 2024 23:35
@czarcas7ic czarcas7ic merged commit 88c490f into osmo/v0.37.4 May 3, 2024
18 checks passed
mergify bot pushed a commit 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 pushed a commit 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 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 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
S:backport/v24 backport to the osmo-v24/v0.37.4 branch S:backport/v25 backport to the osmo-v25/v0.37.4 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant