Merge pull request #3547 from lucas-clemente/pool-ack-frames

use a sync.Pool for ACK frames
This commit is contained in:
Marten Seemann
2022-09-09 11:56:15 +03:00
committed by GitHub
8 changed files with 106 additions and 27 deletions

View File

@@ -1334,6 +1334,7 @@ func (s *connection) handleFrame(f wire.Frame, encLevel protocol.EncryptionLevel
err = s.handleStreamFrame(frame)
case *wire.AckFrame:
err = s.handleAckFrame(frame, encLevel)
wire.PutAckFrame(frame)
case *wire.ConnectionCloseFrame:
s.handleConnectionCloseFrame(frame)
case *wire.ResetStreamFrame:
@@ -1952,7 +1953,11 @@ func (s *connection) logPacketContents(p *packetContents) {
for _, f := range p.frames {
frames = append(frames, logutils.ConvertFrame(f.Frame))
}
s.tracer.SentPacket(p.header, p.length, p.ack, frames)
var ack *logging.AckFrame
if p.ack != nil {
ack = logutils.ConvertAckFrame(p.ack)
}
s.tracer.SentPacket(p.header, p.length, ack, frames)
}
// quic-go logging

View File

@@ -107,17 +107,12 @@ func (h *receivedPacketHistory) DeleteBelow(p protocol.PacketNumber) {
}
}
// GetAckRanges gets a slice of all AckRanges that can be used in an AckFrame
func (h *receivedPacketHistory) GetAckRanges() []wire.AckRange {
if h.ranges.Len() == 0 {
return nil
}
ackRanges := make([]wire.AckRange, h.ranges.Len())
i := 0
for el := h.ranges.Back(); el != nil; el = el.Prev() {
ackRanges[i] = wire.AckRange{Smallest: el.Value.Start, Largest: el.Value.End}
i++
// AppendAckRanges appends to a slice of all AckRanges that can be used in an AckFrame
func (h *receivedPacketHistory) AppendAckRanges(ackRanges []wire.AckRange) []wire.AckRange {
if h.ranges.Len() > 0 {
for el := h.ranges.Back(); el != nil; el = el.Prev() {
ackRanges = append(ackRanges, wire.AckRange{Smallest: el.Value.Start, Largest: el.Value.End})
}
}
return ackRanges
}

View File

@@ -204,17 +204,26 @@ var _ = Describe("receivedPacketHistory", func() {
Context("ACK range export", func() {
It("returns nil if there are no ranges", func() {
Expect(hist.GetAckRanges()).To(BeNil())
Expect(hist.AppendAckRanges(nil)).To(BeEmpty())
})
It("gets a single ACK range", func() {
Expect(hist.ReceivedPacket(4)).To(BeTrue())
Expect(hist.ReceivedPacket(5)).To(BeTrue())
ackRanges := hist.GetAckRanges()
ackRanges := hist.AppendAckRanges(nil)
Expect(ackRanges).To(HaveLen(1))
Expect(ackRanges[0]).To(Equal(wire.AckRange{Smallest: 4, Largest: 5}))
})
It("appends ACK ranges", func() {
Expect(hist.ReceivedPacket(4)).To(BeTrue())
Expect(hist.ReceivedPacket(5)).To(BeTrue())
ackRanges := hist.AppendAckRanges([]wire.AckRange{{Smallest: 1, Largest: 2}})
Expect(ackRanges).To(HaveLen(2))
Expect(ackRanges[0]).To(Equal(wire.AckRange{Smallest: 1, Largest: 2}))
Expect(ackRanges[1]).To(Equal(wire.AckRange{Smallest: 4, Largest: 5}))
})
It("gets multiple ACK ranges", func() {
Expect(hist.ReceivedPacket(4)).To(BeTrue())
Expect(hist.ReceivedPacket(5)).To(BeTrue())
@@ -223,7 +232,7 @@ var _ = Describe("receivedPacketHistory", func() {
Expect(hist.ReceivedPacket(11)).To(BeTrue())
Expect(hist.ReceivedPacket(10)).To(BeTrue())
Expect(hist.ReceivedPacket(2)).To(BeTrue())
ackRanges := hist.GetAckRanges()
ackRanges := hist.AppendAckRanges(nil)
Expect(ackRanges).To(HaveLen(3))
Expect(ackRanges[0]).To(Equal(wire.AckRange{Smallest: 10, Largest: 11}))
Expect(ackRanges[1]).To(Equal(wire.AckRange{Smallest: 4, Largest: 6}))
@@ -339,7 +348,7 @@ var _ = Describe("receivedPacketHistory", func() {
}
}
var counter int
ackRanges := hist.GetAckRanges()
ackRanges := hist.AppendAckRanges(nil)
fmt.Fprintf(GinkgoWriter, "ACK ranges: %v\n", ackRanges)
Expect(len(ackRanges)).To(BeNumerically("<=", numLostPackets+1))
for _, ackRange := range ackRanges {

View File

@@ -171,16 +171,16 @@ func (h *receivedPacketTracker) GetAckFrame(onlyIfQueued bool) *wire.AckFrame {
}
}
ack := &wire.AckFrame{
AckRanges: h.packetHistory.GetAckRanges(),
// Make sure that the DelayTime is always positive.
// This is not guaranteed on systems that don't have a monotonic clock.
DelayTime: utils.Max(0, now.Sub(h.largestObservedReceivedTime)),
ECT0: h.ect0,
ECT1: h.ect1,
ECNCE: h.ecnce,
}
ack := wire.GetAckFrame()
ack.DelayTime = utils.Max(0, now.Sub(h.largestObservedReceivedTime))
ack.ECT0 = h.ect0
ack.ECT1 = h.ect1
ack.ECNCE = h.ecnce
ack.AckRanges = h.packetHistory.AppendAckRanges(ack.AckRanges)
if h.lastAck != nil {
wire.PutAckFrame(h.lastAck)
}
h.lastAck = ack
h.ackAlarm = time.Time{}
h.ackQueued = false

View File

@@ -11,6 +11,10 @@ import (
// Furthermore, it removes the data slices from CRYPTO and STREAM frames.
func ConvertFrame(frame wire.Frame) logging.Frame {
switch f := frame.(type) {
case *wire.AckFrame:
// We use a pool for ACK frames.
// Implementations of the tracer interface may hold on to frames, so we need to make a copy here.
return ConvertAckFrame(f)
case *wire.CryptoFrame:
return &logging.CryptoFrame{
Offset: f.Offset,
@@ -31,3 +35,16 @@ func ConvertFrame(frame wire.Frame) logging.Frame {
return logging.Frame(frame)
}
}
func ConvertAckFrame(f *wire.AckFrame) *logging.AckFrame {
ranges := make([]wire.AckRange, 0, len(f.AckRanges))
ranges = append(ranges, f.AckRanges...)
ack := &logging.AckFrame{
AckRanges: ranges,
DelayTime: f.DelayTime,
ECNCE: f.ECNCE,
ECT0: f.ECT0,
ECT1: f.ECT1,
}
return ack
}

View File

@@ -29,7 +29,7 @@ func parseAckFrame(r *bytes.Reader, ackDelayExponent uint8, _ protocol.VersionNu
}
ecn := typeByte&0x1 > 0
frame := &AckFrame{}
frame := GetAckFrame()
la, err := quicvarint.Read(r)
if err != nil {
@@ -106,7 +106,7 @@ func parseAckFrame(r *bytes.Reader, ackDelayExponent uint8, _ protocol.VersionNu
return frame, nil
}
// Write writes an ACK frame.
// Append appends an ACK frame.
func (f *AckFrame) Append(b []byte, _ protocol.VersionNumber) ([]byte, error) {
hasECN := f.ECT0 > 0 || f.ECT1 > 0 || f.ECNCE > 0
if hasECN {

View File

@@ -0,0 +1,24 @@
package wire
import "sync"
var ackFramePool = sync.Pool{New: func() any {
return &AckFrame{}
}}
func GetAckFrame() *AckFrame {
f := ackFramePool.Get().(*AckFrame)
f.AckRanges = f.AckRanges[:0]
f.ECNCE = 0
f.ECT0 = 0
f.ECT1 = 0
f.DelayTime = 0
return f
}
func PutAckFrame(f *AckFrame) {
if cap(f.AckRanges) > 4 {
return
}
ackFramePool.Put(f)
}

View File

@@ -0,0 +1,29 @@
package wire
import (
"math/rand"
"time"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)
var _ = Describe("ACK Frame (for IETF QUIC)", func() {
It("gets an ACK frame from the pool", func() {
for i := 0; i < 100; i++ {
ack := GetAckFrame()
Expect(ack.AckRanges).To(BeEmpty())
Expect(ack.ECNCE).To(BeZero())
Expect(ack.ECT0).To(BeZero())
Expect(ack.ECT1).To(BeZero())
Expect(ack.DelayTime).To(BeZero())
ack.AckRanges = make([]AckRange, rand.Intn(10))
ack.ECNCE = 1
ack.ECT0 = 2
ack.ECT1 = 3
ack.DelayTime = time.Hour
PutAckFrame(ack)
}
})
})