Secret Connection: Remove sync.Pool, and put those buffers in the connection struct

[ValarDragon]

Feature Request

Summary

In secret connection we use sync.Pool's for some 1024 byte buffers. However these buffers are only ever needed locally within the same thread, so this is a wasted synchronization primitive that does cost us. All reads and writes in secret connection are under a mutex, so we should just make a new buffer in each secret conn struct.

Impact is ~8% of the current secret connection read time (Get and Put in following pprof):

Problem Definition

Slight speedup to secret conn

[cason]

The problem is that Read() and Write() are public methods. So while we know that the p2p connection only uses them in a single thread, the implementation is more generic.

Actually, we have other mutexes in those methods that are based on the same assumption. And this is because it implements generic Go interfaces that assume that Read() and Write() can be called in parallel.

[ValarDragon]

Thats not what I mean, the Read method itself already has the mutex! So these buffers would already be mutex protected

[cason]

They are protected by different mutexes:

cometbft/p2p/conn/secret_connection.go

Lines 232 to 234 in 67b2987

	func (sc *SecretConnection) Read(data []byte) (n int, err error) {
	sc.recvMtx.Lock()
	defer sc.recvMtx.Unlock()

cometbft/p2p/conn/secret_connection.go

Lines 188 to 190 in 67b2987

	func (sc *SecretConnection) Write(data []byte) (n int, err error) {
	sc.sendMtx.Lock()
	defer sc.sendMtx.Unlock()

We use the pool to share buffers between readers and writers...

[cason]

What we can have, to reduce contention, are two unsynchronized pools, one for reads, another for writes.

By the way, we import libp2p for this buffer pool :D