From d9665c632ec8bfbed579fb2885ca1049c92eb3fd Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 30 Dec 2022 18:02:19 +1300 Subject: [PATCH] use a sync.Pool to reduce allocation of linked list elements Especially the sentPacketHistory linked list shows up in allocation profiles, since a new list element is allocated for every single packet we send. Using a pool for the receiving path, as well as for the frame sorter, is less critical, since we're tracking ranges there instead of individual packets / frames, but it doesn't hurt either. The other occurrences where we use a linked list (connection ID tracking and the token store) are used so rarely (a few times over the lifetime of the connection) that using a pool doesn't make any sense there. --- frame_sorter.go | 9 +++++- .../ackhandler/received_packet_history.go | 10 ++++++- internal/ackhandler/sent_packet_history.go | 11 +++++-- internal/utils/linkedlist/README.md | 4 ++- internal/utils/linkedlist/linkedlist.go | 29 +++++++++++++++++-- 5 files changed, 56 insertions(+), 7 deletions(-) diff --git a/frame_sorter.go b/frame_sorter.go index 0573ade9..58dcfe55 100644 --- a/frame_sorter.go +++ b/frame_sorter.go @@ -2,6 +2,7 @@ package quic import ( "errors" + "sync" "github.com/lucas-clemente/quic-go/internal/protocol" list "github.com/lucas-clemente/quic-go/internal/utils/linkedlist" @@ -13,6 +14,12 @@ type byteInterval struct { End protocol.ByteCount } +var byteIntervalElementPool sync.Pool + +func init() { + byteIntervalElementPool = *list.NewPool[byteInterval]() +} + type frameSorterEntry struct { Data []byte DoneCb func() @@ -28,7 +35,7 @@ var errDuplicateStreamData = errors.New("duplicate stream data") func newFrameSorter() *frameSorter { s := frameSorter{ - gaps: list.New[byteInterval](), + gaps: list.NewWithPool[byteInterval](&byteIntervalElementPool), queue: make(map[protocol.ByteCount]frameSorterEntry), } s.gaps.PushFront(byteInterval{Start: 0, End: protocol.MaxByteCount}) diff --git a/internal/ackhandler/received_packet_history.go b/internal/ackhandler/received_packet_history.go index d4d28cc4..a8804f81 100644 --- a/internal/ackhandler/received_packet_history.go +++ b/internal/ackhandler/received_packet_history.go @@ -1,6 +1,8 @@ package ackhandler import ( + "sync" + "github.com/lucas-clemente/quic-go/internal/protocol" list "github.com/lucas-clemente/quic-go/internal/utils/linkedlist" "github.com/lucas-clemente/quic-go/internal/wire" @@ -12,6 +14,12 @@ type interval struct { End protocol.PacketNumber } +var intervalElementPool sync.Pool + +func init() { + intervalElementPool = *list.NewPool[interval]() +} + // The receivedPacketHistory stores if a packet number has already been received. // It generates ACK ranges which can be used to assemble an ACK frame. // It does not store packet contents. @@ -23,7 +31,7 @@ type receivedPacketHistory struct { func newReceivedPacketHistory() *receivedPacketHistory { return &receivedPacketHistory{ - ranges: list.New[interval](), + ranges: list.NewWithPool[interval](&intervalElementPool), } } diff --git a/internal/ackhandler/sent_packet_history.go b/internal/ackhandler/sent_packet_history.go index e9c1de1d..323401fd 100644 --- a/internal/ackhandler/sent_packet_history.go +++ b/internal/ackhandler/sent_packet_history.go @@ -2,6 +2,7 @@ package ackhandler import ( "fmt" + "sync" "time" "github.com/lucas-clemente/quic-go/internal/protocol" @@ -17,11 +18,17 @@ type sentPacketHistory struct { highestSent protocol.PacketNumber } +var packetElementPool sync.Pool + +func init() { + packetElementPool = *list.NewPool[*Packet]() +} + func newSentPacketHistory(rttStats *utils.RTTStats) *sentPacketHistory { return &sentPacketHistory{ rttStats: rttStats, - outstandingPacketList: list.New[*Packet](), - etcPacketList: list.New[*Packet](), + outstandingPacketList: list.NewWithPool[*Packet](&packetElementPool), + etcPacketList: list.NewWithPool[*Packet](&packetElementPool), packetMap: make(map[protocol.PacketNumber]*list.Element[*Packet]), highestSent: protocol.InvalidPacketNumber, } diff --git a/internal/utils/linkedlist/README.md b/internal/utils/linkedlist/README.md index 2915d601..66482f4f 100644 --- a/internal/utils/linkedlist/README.md +++ b/internal/utils/linkedlist/README.md @@ -1,4 +1,6 @@ # Usage This is the Go standard library implementation of a linked list -(https://golang.org/src/container/list/list.go), modified to use Go generics. +(https://golang.org/src/container/list/list.go), with the following modifications: +* it uses Go generics +* it allows passing in a `sync.Pool` (via the `NewWithPool` constructor) to reduce allocations of `Element` structs diff --git a/internal/utils/linkedlist/linkedlist.go b/internal/utils/linkedlist/linkedlist.go index c140cfcb..804a3444 100644 --- a/internal/utils/linkedlist/linkedlist.go +++ b/internal/utils/linkedlist/linkedlist.go @@ -11,6 +11,12 @@ // } package list +import "sync" + +func NewPool[T any]() *sync.Pool { + return &sync.Pool{New: func() any { return &Element[T]{} }} +} + // Element is an element of a linked list. type Element[T any] struct { // Next and previous pointers in the doubly-linked list of elements. @@ -52,6 +58,8 @@ func (e *Element[T]) List() *List[T] { type List[T any] struct { root Element[T] // sentinel list element, only &root, root.prev, and root.next are used len int // current list length excluding (this) sentinel element + + pool *sync.Pool } // Init initializes or clears list l. @@ -65,6 +73,12 @@ func (l *List[T]) Init() *List[T] { // New returns an initialized list. func New[T any]() *List[T] { return new(List[T]).Init() } +// NewWithPool returns an initialized list, using a sync.Pool for list elements. +func NewWithPool[T any](pool *sync.Pool) *List[T] { + l := &List[T]{pool: pool} + return l.Init() +} + // Len returns the number of elements of list l. // The complexity is O(1). func (l *List[T]) Len() int { return l.len } @@ -105,7 +119,14 @@ func (l *List[T]) insert(e, at *Element[T]) *Element[T] { // insertValue is a convenience wrapper for insert(&Element{Value: v}, at). func (l *List[T]) insertValue(v T, at *Element[T]) *Element[T] { - return l.insert(&Element[T]{Value: v}, at) + var e *Element[T] + if l.pool != nil { + e = l.pool.Get().(*Element[T]) + } else { + e = &Element[T]{} + } + e.Value = v + return l.insert(e, at) } // remove removes e from its list, decrements l.len @@ -115,6 +136,9 @@ func (l *List[T]) remove(e *Element[T]) { e.next = nil // avoid memory leaks e.prev = nil // avoid memory leaks e.list = nil + if l.pool != nil { + l.pool.Put(e) + } l.len-- } @@ -136,12 +160,13 @@ func (l *List[T]) move(e, at *Element[T]) { // It returns the element value e.Value. // The element must not be nil. func (l *List[T]) Remove(e *Element[T]) T { + v := e.Value if e.list == l { // if e.list == l, l must have been initialized when e was inserted // in l or l == nil (e is a zero Element) and l.remove will crash l.remove(e) } - return e.Value + return v } // PushFront inserts a new element e with value v at the front of list l and returns e.