Skip to content

perf(consensus): Reuse an internal buffer for block building#3162

Merged
melekes merged 6 commits intocometbft:mainfrom
osmosis-labs:dev/reuse_buffer_block_building
Jun 4, 2024
Merged

perf(consensus): Reuse an internal buffer for block building#3162
melekes merged 6 commits intocometbft:mainfrom
osmosis-labs:dev/reuse_buffer_block_building

Conversation

@ValarDragon
Copy link
Contributor

@ValarDragon ValarDragon commented May 31, 2024

Makes an internal buffer that we can re-use when building blocks. This seems to save on average 75 microseconds per block for osmosis blocks. (Ranging between 1-2 block parts in the relevant time range) This should be scaling roughly linear with block size.

This lets us remove one allocation cost per complete block coming in. We only need to re-allocate on the next "biggest ever seen" block we see.


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

@ValarDragon ValarDragon requested a review from a team as a code owner May 31, 2024 16:43
@ValarDragon ValarDragon requested a review from a team May 31, 2024 16:43
@ValarDragon ValarDragon marked this pull request as draft May 31, 2024 16:49
@ValarDragon ValarDragon marked this pull request as ready for review June 1, 2024 00:51
@melekes melekes added this pull request to the merge queue Jun 4, 2024
Merged via the queue into cometbft:main with commit 53da94d Jun 4, 2024
ValarDragon added a commit to osmosis-labs/cometbft that referenced this pull request Jun 4, 2024
…t#3162)

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

Makes an internal buffer that we can re-use when building blocks. This
seems to save on average 75 microseconds per block for osmosis blocks.
(Ranging between 1-2 block parts in the relevant time range) This should
be scaling roughly linear with block size.

This lets us remove one allocation cost per complete block coming in. We
only need to re-allocate on the next "biggest ever seen" block we see.

---

- [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
ValarDragon added a commit to osmosis-labs/cometbft that referenced this pull request Jun 4, 2024
perf(consensus): Reuse an internal buffer for block building (cometbft#3162)
mergify bot pushed a commit to osmosis-labs/cometbft that referenced this pull request Jun 4, 2024
…t#3162)

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

Makes an internal buffer that we can re-use when building blocks. This
seems to save on average 75 microseconds per block for osmosis blocks.
(Ranging between 1-2 block parts in the relevant time range) This should
be scaling roughly linear with block size.

This lets us remove one allocation cost per complete block coming in. We
only need to re-allocate on the next "biggest ever seen" block we see.

---

- [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 16839d8)
ValarDragon added a commit to osmosis-labs/cometbft that referenced this pull request Jun 4, 2024
…/pr-99

perf(consensus): Reuse an internal buffer for block building (cometbft#3162) (backport #99)
@melekes
Copy link
Collaborator

melekes commented Jul 10, 2024

@mergify backport v1.x

@mergify
Copy link
Contributor

mergify bot commented Jul 10, 2024

backport v1.x

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Jul 10, 2024
<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

Makes an internal buffer that we can re-use when building blocks. This
seems to save on average 75 microseconds per block for osmosis blocks.
(Ranging between 1-2 block parts in the relevant time range) This should
be scaling roughly linear with block size.

This lets us remove one allocation cost per complete block coming in. We
only need to re-allocate on the next "biggest ever seen" block we see.

---

#### 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 53da94d)

# Conflicts:
#	.changelog/unreleased/improvements/3162-reuse-internal-buffer-for-block-building.md
melekes added a commit that referenced this pull request Jul 10, 2024
…#3162) (#3476)

Makes an internal buffer that we can re-use when building blocks. This
seems to save on average 75 microseconds per block for osmosis blocks.
(Ranging between 1-2 block parts in the relevant time range) This should
be scaling roughly linear with block size.

This lets us remove one allocation cost per complete block coming in. We
only need to re-allocate on the next "biggest ever seen" block we see.

---

#### 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 #3162 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>
ValarDragon added a commit to osmosis-labs/cometbft that referenced this pull request Aug 19, 2024
…t#3162)

<!--

Please add a reference to the issue that this PR addresses and indicate
which
files are most critical to review. If it fully addresses a particular
issue,
please include "Closes #XXX" (where "XXX" is the issue number).

If this PR is non-trivial/large/complex, please ensure that you have
either
created an issue that the team's had a chance to respond to, or had some
discussion with the team prior to submitting substantial pull requests.
The team
can be reached via GitHub Discussions or the Cosmos Network Discord
server in
the #cometbft channel. GitHub Discussions is preferred over Discord as
it
allows us to keep track of conversations topically.
https://github.com/cometbft/cometbft/discussions

If the work in this PR is not aligned with the team's current
priorities, please
be advised that it may take some time before it is merged - especially
if it has
not yet been discussed with the team.

See the project board for the team's current priorities:
https://github.com/orgs/cometbft/projects/1

-->

Makes an internal buffer that we can re-use when building blocks. This
seems to save on average 75 microseconds per block for osmosis blocks.
(Ranging between 1-2 block parts in the relevant time range) This should
be scaling roughly linear with block size.

This lets us remove one allocation cost per complete block coming in. We
only need to re-allocate on the next "biggest ever seen" block we see.

---

- [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
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.

2 participants