Skip to content

perf: Micro optimization to save one allocation per packet (backport …#75

Merged
ValarDragon merged 2 commits intoosmo/v0.37.4from
bp/3019
May 25, 2024
Merged

perf: Micro optimization to save one allocation per packet (backport …#75
ValarDragon merged 2 commits intoosmo/v0.37.4from
bp/3019

Conversation

@ValarDragon
Copy link
Member

cometbft#3018) (cometbft#3023)

This PR is a slight optimization to save one allocation per packet. We have much more worthwhile performance improvements to pursue, just driveby noticed it as I was reading through the code. (Though I am surprised this did add up to 1 second in total -- 4% of the processing time)

This re-uses the byte reader's allocation across all ReadMsg's. There is no concurrenct access possible under safe usage (also implied by the reader)

This is the cause of the 1s time on the far right:

image


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

mergify bot and others added 2 commits May 25, 2024 11:42
…ometbft#3018) (cometbft#3023)

This PR is a slight optimization to save one allocation per packet. We
have much more worthwhile performance improvements to pursue, just
driveby noticed it as I was reading through the code. (Though I am
surprised this did add up to 1 second in total -- 4% of the processing
time)

This re-uses the byte reader's allocation across all ReadMsg's. There is
no concurrenct access possible under safe usage (also implied by the
reader)

This is the cause of the 1s time on the far right: 

![image](https://github.com/cometbft/cometbft/assets/6440154/d6c6bfaa-d287-4355-b094-9bdcbc6379c8)


---

#### PR checklist

- [x] Tests written/updated - covered by existing tests
- [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#3018 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 ValarDragon merged commit 68c9c3f into osmo/v0.37.4 May 25, 2024
@PaddyMc PaddyMc added the S:backport/v25 backport to the osmo-v25/v0.37.4 branch label May 28, 2024
PaddyMc added a commit that referenced this pull request May 28, 2024
…… (backport #75) (#80)

* perf: Micro optimization to save one allocation per packet (backport cometbft#3018) (cometbft#3023)

This PR is a slight optimization to save one allocation per packet. We
have much more worthwhile performance improvements to pursue, just
driveby noticed it as I was reading through the code. (Though I am
surprised this did add up to 1 second in total -- 4% of the processing
time)

This re-uses the byte reader's allocation across all ReadMsg's. There is
no concurrenct access possible under safe usage (also implied by the
reader)

This is the cause of the 1s time on the far right:

![image](https://github.com/cometbft/cometbft/assets/6440154/d6c6bfaa-d287-4355-b094-9bdcbc6379c8)

---

#### PR checklist

- [x] Tests written/updated - covered by existing tests
- [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#3018 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>
(cherry picked from commit ad8f851)

* changelog

(cherry picked from commit dd19cb0)

# Conflicts:
#	CHANGELOG.md

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <dojha@berkeley.edu>
Co-authored-by: PaddyMc <paddymchale@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants