From 07b8821ef7d7ad317637427ec7aefc50de08a164 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 21 Feb 2018 22:37:08 +0800 Subject: [PATCH] use pointer to byte slices in the buffer pool https://staticcheck.io/docs/staticcheck#SA6002 suggests to use pointers to objects in the sync.Pool. --- buffer_pool.go | 13 +++++++------ buffer_pool_test.go | 17 +++-------------- client.go | 2 +- mint_utils.go | 4 ++-- packet_packer.go | 4 ++-- packet_unpacker.go | 5 +++-- server.go | 2 +- session.go | 4 ++-- 8 files changed, 21 insertions(+), 30 deletions(-) diff --git a/buffer_pool.go b/buffer_pool.go index 5032ca7f..6b233696 100644 --- a/buffer_pool.go +++ b/buffer_pool.go @@ -8,19 +8,20 @@ import ( var bufferPool sync.Pool -func getPacketBuffer() []byte { - return bufferPool.Get().([]byte) +func getPacketBuffer() *[]byte { + return bufferPool.Get().(*[]byte) } -func putPacketBuffer(buf []byte) { - if cap(buf) != int(protocol.MaxReceivePacketSize) { +func putPacketBuffer(buf *[]byte) { + if cap(*buf) != int(protocol.MaxReceivePacketSize) { panic("putPacketBuffer called with packet of wrong size!") } - bufferPool.Put(buf[:0]) + bufferPool.Put(buf) } func init() { bufferPool.New = func() interface{} { - return make([]byte, 0, protocol.MaxReceivePacketSize) + b := make([]byte, 0, protocol.MaxReceivePacketSize) + return &b } } diff --git a/buffer_pool_test.go b/buffer_pool_test.go index 49e37a68..7413ceaf 100644 --- a/buffer_pool_test.go +++ b/buffer_pool_test.go @@ -8,25 +8,14 @@ import ( ) var _ = Describe("Buffer Pool", func() { - It("returns buffers of correct len and cap", func() { - buf := getPacketBuffer() - Expect(buf).To(HaveLen(0)) + It("returns buffers of cap", func() { + buf := *getPacketBuffer() Expect(buf).To(HaveCap(int(protocol.MaxReceivePacketSize))) }) - It("zeroes put buffers' length", func() { - for i := 0; i < 1000; i++ { - buf := getPacketBuffer() - putPacketBuffer(buf[0:10]) - buf = getPacketBuffer() - Expect(buf).To(HaveLen(0)) - Expect(buf).To(HaveCap(int(protocol.MaxReceivePacketSize))) - } - }) - It("panics if wrong-sized buffers are passed", func() { Expect(func() { - putPacketBuffer([]byte{0}) + putPacketBuffer(&[]byte{0}) }).To(Panic()) }) }) diff --git a/client.go b/client.go index 766d03ab..fba3daee 100644 --- a/client.go +++ b/client.go @@ -245,7 +245,7 @@ func (c *client) listen() { for { var n int var addr net.Addr - data := getPacketBuffer() + data := *getPacketBuffer() data = data[:protocol.MaxReceivePacketSize] // The packet size should not exceed protocol.MaxReceivePacketSize bytes // If it does, we only read a truncated packet, which will then end up undecryptable diff --git a/mint_utils.go b/mint_utils.go index 578aecca..5156a652 100644 --- a/mint_utils.go +++ b/mint_utils.go @@ -139,8 +139,8 @@ func unpackInitialPacket(aead crypto.AEAD, hdr *wire.Header, data []byte, versio // packUnencryptedPacket provides a low-overhead way to pack a packet. // It is supposed to be used in the early stages of the handshake, before a session (which owns a packetPacker) is available. func packUnencryptedPacket(aead crypto.AEAD, hdr *wire.Header, f wire.Frame, pers protocol.Perspective) ([]byte, error) { - raw := getPacketBuffer() - buffer := bytes.NewBuffer(raw) + raw := *getPacketBuffer() + buffer := bytes.NewBuffer(raw[:0]) if err := hdr.Write(buffer, pers, hdr.Version); err != nil { return nil, err } diff --git a/packet_packer.go b/packet_packer.go index 71176923..bc71441f 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -342,8 +342,8 @@ func (p *packetPacker) writeAndSealPacket( payloadFrames []wire.Frame, sealer handshake.Sealer, ) ([]byte, error) { - raw := getPacketBuffer() - buffer := bytes.NewBuffer(raw) + raw := *getPacketBuffer() + buffer := bytes.NewBuffer(raw[:0]) if err := header.Write(buffer, p.perspective, p.version); err != nil { return nil, err diff --git a/packet_unpacker.go b/packet_unpacker.go index 45bdc0fa..a978675d 100644 --- a/packet_unpacker.go +++ b/packet_unpacker.go @@ -24,8 +24,9 @@ type packetUnpacker struct { } func (u *packetUnpacker) Unpack(headerBinary []byte, hdr *wire.Header, data []byte) (*unpackedPacket, error) { - buf := getPacketBuffer() - defer putPacketBuffer(buf) + buf := *getPacketBuffer() + buf = buf[:0] + defer putPacketBuffer(&buf) decrypted, encryptionLevel, err := u.aead.Open(buf, data, hdr.PacketNumber, headerBinary) if err != nil { // Wrap err in quicError so that public reset is sent by session diff --git a/server.go b/server.go index dc23b868..26e153d5 100644 --- a/server.go +++ b/server.go @@ -213,7 +213,7 @@ func populateServerConfig(config *Config) *Config { // serve listens on an existing PacketConn func (s *server) serve() { for { - data := getPacketBuffer() + data := *getPacketBuffer() data = data[:protocol.MaxReceivePacketSize] // The packet size should not exceed protocol.MaxReceivePacketSize bytes // If it does, we only read a truncated packet, which will then end up undecryptable diff --git a/session.go b/session.go index 20777796..c07d6299 100644 --- a/session.go +++ b/session.go @@ -393,7 +393,7 @@ runLoop: } // This is a bit unclean, but works properly, since the packet always // begins with the public header and we never copy it. - putPacketBuffer(p.header.Raw) + putPacketBuffer(&p.header.Raw) case p := <-s.paramsChan: s.processTransportParameters(&p) case _, ok := <-handshakeEvent: @@ -867,7 +867,7 @@ func (s *session) sendPacket() (bool, error) { } func (s *session) sendPackedPacket(packet *packedPacket) error { - defer putPacketBuffer(packet.raw) + defer putPacketBuffer(&packet.raw) err := s.sentPacketHandler.SentPacket(&ackhandler.Packet{ PacketNumber: packet.header.PacketNumber, Frames: packet.frames,