Skip to content

Commit

Permalink
perf(p2p/conn): Remove unneeded global pool buffers in secret connect…
Browse files Browse the repository at this point in the history
…ion (backport #3403) (#3418)

Closes #3197 

Remove unneeded global pool buffers in secret connection, instead just
use a byte slice per secret connection, and just once for all packet
sends. This is a 15% improvement to Secret connection's CPU time in
Write, and 8% to read

We have much bigger system bottlenecks to fix (in the same RecvRoutine
and SendRoutine calls), but this felt very low-lift, hence making the
PR.

---

#### PR checklist

- [X] Tests written/updated  - N/A
- [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 #3403 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>
Co-authored-by: Hernán Vanzetto <15466498+hvanz@users.noreply.github.com>
  • Loading branch information
4 people committed Jul 5, 2024
1 parent 0aeebd5 commit 9ca0b49
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 32 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[p2p/conn]` Remove the usage of a synchronous pool of buffers in secret connection, storing instead the buffer in the connection struct. This reduces the synchronization primitive usage, speeding up the code.
([\#3403](https://github.com/cometbft/cometbft/issues/3403))
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ require (
github.com/gorilla/websocket v1.5.0
github.com/informalsystems/tm-load-test v1.3.0
github.com/lib/pq v1.10.7
github.com/libp2p/go-buffer-pool v0.1.0
github.com/minio/highwayhash v1.0.2
github.com/ory/dockertest v3.3.5+incompatible
github.com/pkg/errors v0.9.1
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,6 @@ github.com/leonklingele/grouper v1.1.1 h1:suWXRU57D4/Enn6pXR0QVqqWWrnJ9Osrz+5rjt
github.com/leonklingele/grouper v1.1.1/go.mod h1:uk3I3uDfi9B6PeUjsCKi6ndcf63Uy7snXgR4yDYQVDY=
github.com/lib/pq v1.10.7 h1:p7ZhMD+KsSRozJr34udlUrhboJwWAgCg34+/ZZNvZZw=
github.com/lib/pq v1.10.7/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/libp2p/go-buffer-pool v0.1.0 h1:oK4mSFcQz7cTQIfqbe4MIj9gLW+mnanjyFtc6cdF0Y8=
github.com/libp2p/go-buffer-pool v0.1.0/go.mod h1:N+vh8gMqimBzdKkSMVuydVDq+UV5QTWy5HSiZacSbPg=
github.com/lufeee/execinquery v1.2.1 h1:hf0Ems4SHcUGBxpGN7Jz78z1ppVkP/837ZlETPCEtOM=
github.com/lufeee/execinquery v1.2.1/go.mod h1:EC7DrEKView09ocscGHC+apXMIaorh4xqSxS/dy8SbM=
github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
Expand Down
16 changes: 10 additions & 6 deletions p2p/conn/evil_secret_connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,16 @@ func (c *evilConn) signChallenge() []byte {

b := &buffer{}
c.secretConn = &SecretConnection{
conn: b,
recvBuffer: nil,
recvNonce: new([aeadNonceSize]byte),
sendNonce: new([aeadNonceSize]byte),
recvAead: recvAead,
sendAead: sendAead,
conn: b,
recvBuffer: nil,
recvNonce: new([aeadNonceSize]byte),
sendNonce: new([aeadNonceSize]byte),
recvAead: recvAead,
sendAead: sendAead,
recvFrame: make([]byte, totalFrameSize),
recvSealedFrame: make([]byte, totalFrameSize+aeadSizeOverhead),
sendFrame: make([]byte, totalFrameSize),
sendSealedFrame: make([]byte, totalFrameSize+aeadSizeOverhead),
}
c.buffer = b

Expand Down
46 changes: 23 additions & 23 deletions p2p/conn/secret_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"time"

gogotypes "github.com/cosmos/gogoproto/types"
pool "github.com/libp2p/go-buffer-pool"
"github.com/oasisprotocol/curve25519-voi/primitives/merlin"
"golang.org/x/crypto/chacha20poly1305"
"golang.org/x/crypto/curve25519"
Expand Down Expand Up @@ -76,12 +75,16 @@ type SecretConnection struct {
// are independent, so we can use two mtxs.
// All .Read are covered by recvMtx,
// all .Write are covered by sendMtx.
recvMtx cmtsync.Mutex
recvBuffer []byte
recvNonce *[aeadNonceSize]byte

sendMtx cmtsync.Mutex
sendNonce *[aeadNonceSize]byte
recvMtx cmtsync.Mutex
recvBuffer []byte
recvNonce *[aeadNonceSize]byte
recvFrame []byte
recvSealedFrame []byte

sendMtx cmtsync.Mutex
sendNonce *[aeadNonceSize]byte
sendFrame []byte
sendSealedFrame []byte
}

// MakeSecretConnection performs handshake and returns a new authenticated
Expand Down Expand Up @@ -144,12 +147,16 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*
}

sc := &SecretConnection{
conn: conn,
recvBuffer: nil,
recvNonce: new([aeadNonceSize]byte),
sendNonce: new([aeadNonceSize]byte),
recvAead: recvAead,
sendAead: sendAead,
conn: conn,
recvBuffer: nil,
recvNonce: new([aeadNonceSize]byte),
sendNonce: new([aeadNonceSize]byte),
recvAead: recvAead,
sendAead: sendAead,
recvFrame: make([]byte, totalFrameSize),
recvSealedFrame: make([]byte, aeadSizeOverhead+totalFrameSize),
sendFrame: make([]byte, totalFrameSize),
sendSealedFrame: make([]byte, aeadSizeOverhead+totalFrameSize),
}

// Sign the challenge bytes for authentication.
Expand Down Expand Up @@ -187,15 +194,10 @@ func (sc *SecretConnection) RemotePubKey() crypto.PubKey {
func (sc *SecretConnection) Write(data []byte) (n int, err error) {
sc.sendMtx.Lock()
defer sc.sendMtx.Unlock()
sealedFrame, frame := sc.sendSealedFrame, sc.sendFrame

for 0 < len(data) {
if err := func() error {
var sealedFrame = pool.Get(aeadSizeOverhead + totalFrameSize)
var frame = pool.Get(totalFrameSize)
defer func() {
pool.Put(sealedFrame)
pool.Put(frame)
}()
var chunk []byte
if dataMaxSize < len(data) {
chunk = data[:dataMaxSize]
Expand Down Expand Up @@ -239,17 +241,15 @@ func (sc *SecretConnection) Read(data []byte) (n int, err error) {
}

// read off the conn
var sealedFrame = pool.Get(aeadSizeOverhead + totalFrameSize)
defer pool.Put(sealedFrame)
sealedFrame := sc.recvSealedFrame
_, err = io.ReadFull(sc.conn, sealedFrame)
if err != nil {
return n, err
}

// decrypt the frame.
// reads and updates the sc.recvNonce
var frame = pool.Get(totalFrameSize)
defer pool.Put(frame)
frame := sc.recvFrame
_, err = sc.recvAead.Open(frame[:0], sc.recvNonce[:], sealedFrame, nil)
if err != nil {
return n, fmt.Errorf("failed to decrypt SecretConnection: %w", err)
Expand Down

0 comments on commit 9ca0b49

Please sign in to comment.