Skip to content

perf(p2p/conn): Remove p2p allocations for wrapping outbound packets (backport #3423)#3465

Merged
melekes merged 2 commits intov1.xfrom
mergify/bp/v1.x/pr-3423
Jul 10, 2024
Merged

perf(p2p/conn): Remove p2p allocations for wrapping outbound packets (backport #3423)#3465
melekes merged 2 commits intov1.xfrom
mergify/bp/v1.x/pr-3423

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Jul 10, 2024

Closes #2951

Remove p2p allocations for wrapping outbound packets. Every packet send would require 3 new heap objects due to how we used pointers / did this wrapping. This costs us: 13% of all system heap allocations, and 20% of the CPU time of writing packets (under "normal" osmosis load. I don't have big blocks benchmarks after the secret connection improvement got in).

This should be a 25% speedup to channel.writePacketMsgTo, making writing getting closer to the bound of just proto-marshalling a packet.

This solution is simple (just make a buffer in the struct) since these packet sends are all single threaded.


PR checklist

  • Tests written/updated - N/A, existing tests cover safety here
  • 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 #3423 done by [Mergify](https://mergify.com).

…3423)

Closes #2951

Remove p2p allocations for wrapping outbound packets. Every packet send
would require 3 new heap objects due to how we used pointers / did this
wrapping. This costs us: 13% of all system heap allocations, and 20% of
the CPU time of writing packets (under "normal" osmosis load. I don't
have big blocks benchmarks after the secret connection improvement got
in).

This should be a 25% speedup to `channel.writePacketMsgTo`, making
writing getting closer to the bound of just proto-marshalling a packet.

This solution is simple (just make a buffer in the struct) since these
packet sends are all single threaded.

---

#### PR checklist

- [X] Tests written/updated - N/A, existing tests cover safety here
- [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

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit 35894b2)
@mergify mergify bot requested a review from a team as a code owner July 10, 2024 06:11
@mergify mergify bot requested a review from a team July 10, 2024 06:11
@melekes melekes merged commit f80ef1c into v1.x Jul 10, 2024
@melekes melekes deleted the mergify/bp/v1.x/pr-3423 branch July 10, 2024 06:53
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