From ad8f851f09c5948affa0864903dda7f59a52ef4f Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Tue, 7 May 2024 15:21:39 +0800
Subject: [PATCH 1/2] perf: Micro optimization to save one allocation per
 packet (backport #3018) (#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 #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>
---
 ...3019-reduce-allocations-in-packet-reads.md |  3 +++
 .golangci.yml                                 |  2 ++
 libs/protoio/io.go                            |  4 ++++
 libs/protoio/reader.go                        | 23 ++++++++++---------
 libs/pubsub/subscription.go                   |  2 --
 types/event_bus.go                            |  2 +-
 6 files changed, 22 insertions(+), 14 deletions(-)
 create mode 100644 .changelog/unreleased/improvements/3019-reduce-allocations-in-packet-reads.md

diff --git a/.changelog/unreleased/improvements/3019-reduce-allocations-in-packet-reads.md b/.changelog/unreleased/improvements/3019-reduce-allocations-in-packet-reads.md
new file mode 100644
index 00000000000..604002636a9
--- /dev/null
+++ b/.changelog/unreleased/improvements/3019-reduce-allocations-in-packet-reads.md
@@ -0,0 +1,3 @@
+- [`protoio`] Remove one allocation and new object call from `ReadMsg`,
+  leading to a 4% p2p message reading performance gain.
+  ([\#3018](https://github.com/cometbft/cometbft/issues/3018)
diff --git a/.golangci.yml b/.golangci.yml
index d7db8f833c0..c9a29c49046 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -43,6 +43,8 @@ linters-settings:
     suggest-new: true
   misspell:
     locale: US
+    ignore-words:
+      - Cancelled
   depguard:
     rules:
       main:
diff --git a/libs/protoio/io.go b/libs/protoio/io.go
index 6244afd97b6..d024f6a04f8 100644
--- a/libs/protoio/io.go
+++ b/libs/protoio/io.go
@@ -97,3 +97,7 @@ func (r *byteReader) ReadByte() (byte, error) {
 	}
 	return r.buf[0], nil
 }
+
+func (r *byteReader) resetBytesRead() {
+	r.bytesRead = 0
+}
diff --git a/libs/protoio/reader.go b/libs/protoio/reader.go
index 95b8d345585..054a114df8b 100644
--- a/libs/protoio/reader.go
+++ b/libs/protoio/reader.go
@@ -49,24 +49,25 @@ func NewDelimitedReader(r io.Reader, maxSize int) ReadCloser {
 	if c, ok := r.(io.Closer); ok {
 		closer = c
 	}
-	return &varintReader{r, nil, maxSize, closer}
+	return &varintReader{r, newByteReader(r), nil, maxSize, closer}
 }
 
 type varintReader struct {
-	r       io.Reader
-	buf     []byte
-	maxSize int
-	closer  io.Closer
-}
-
-func (r *varintReader) ReadMsg(msg proto.Message) (int, error) {
+	r io.Reader
 	// ReadUvarint needs an io.ByteReader, and we also need to keep track of the
 	// number of bytes read, so we use our own byteReader. This can't be
 	// buffered, so the caller should pass a buffered io.Reader to avoid poor
 	// performance.
-	byteReader := newByteReader(r.r)
-	l, err := binary.ReadUvarint(byteReader)
-	n := byteReader.bytesRead
+	byteReader *byteReader
+	buf        []byte
+	maxSize    int
+	closer     io.Closer
+}
+
+func (r *varintReader) ReadMsg(msg proto.Message) (int, error) {
+	r.byteReader.resetBytesRead()
+	l, err := binary.ReadUvarint(r.byteReader)
+	n := r.byteReader.bytesRead
 	if err != nil {
 		return n, err
 	}
diff --git a/libs/pubsub/subscription.go b/libs/pubsub/subscription.go
index 3c9046792f5..435a30e3de5 100644
--- a/libs/pubsub/subscription.go
+++ b/libs/pubsub/subscription.go
@@ -45,8 +45,6 @@ func (s *Subscription) Out() <-chan Message {
 
 // Cancelled returns a channel that's closed when the subscription is
 // terminated and supposed to be used in a select statement.
-//
-//nolint:misspell
 func (s *Subscription) Cancelled() <-chan struct{} {
 	return s.canceled
 }
diff --git a/types/event_bus.go b/types/event_bus.go
index b22df3b9dad..580254561c3 100644
--- a/types/event_bus.go
+++ b/types/event_bus.go
@@ -23,7 +23,7 @@ type EventBusSubscriber interface {
 
 type Subscription interface {
 	Out() <-chan cmtpubsub.Message
-	Cancelled() <-chan struct{} //nolint: misspell
+	Cancelled() <-chan struct{}
 	Err() error
 }
 

From dd19cb06a0705d128e788dd5dbed4cf056d33ba2 Mon Sep 17 00:00:00 2001
From: Dev Ojha <dojha@berkeley.edu>
Date: Sat, 25 May 2024 11:43:31 +0900
Subject: [PATCH 2/2] changelog

---
 CHANGELOG.md | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0d893481972..6322c68b7a7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,13 +2,15 @@
 
 ## Unreleased
 
+* [#73](https://github.com/osmosis-labs/cometbft/pull/73) perf(consensus/blockexec): Add simplistic block validation cache
+* [#74](https://github.com/osmosis-labs/cometbft/pull/74) perf(consensus): Minor speedup to mark late vote metrics
+* [#75](https://github.com/osmosis-labs/cometbft/pull/75) perf(p2p): 4% speedup to readMsg by removing one allocation
+
 ## v0.37.4-v25-osmo-3
 
 * [#61](https://github.com/osmosis-labs/cometbft/pull/61) refactor(p2p/connection): Slight refactor to sendManyPackets that helps highlight performance improvements (backport #2953) (#2978)
 * [#62](https://github.com/osmosis-labs/cometbft/pull/62) perf(consensus/blockstore): Remove validate basic call from LoadBlock
 * [#71](https://github.com/osmosis-labs/cometbft/pull/71) perf(consensus): Make mempool update async from Commit
-* [#73](https://github.com/osmosis-labs/cometbft/pull/73) perf(consensus/blockexec): Add simplistic block validation cache
-* [#74](https://github.com/osmosis-labs/cometbft/pull/74) perf(consensus): Minor speedup to mark late vote metrics
 * [#59](https://github.com/osmosis-labs/cometbft/pull/59) `[blockstore]` Remove a redundant `Header.ValidateBasic` call in `LoadBlockMeta`, 75% reducing this time. ([\#2964](https://github.com/cometbft/cometbft/pull/2964))
 * [#59](https://github.com/osmosis-labs/cometbft/pull/59) `[p2p/conn]` Speedup connection.WritePacketMsgTo, by reusing internal buffers rather than re-allocating. ([\#2986](https://github.com/cometbft/cometbft/pull/2986))