use a sync.Pool for ackhandler.Frames (#3656)

This commit is contained in:
Marten Seemann
2023-01-17 23:15:02 -08:00
committed by GitHub
parent b77d8570df
commit 2aa71ff76b
13 changed files with 129 additions and 90 deletions

View File

@@ -10,7 +10,7 @@ func IsFrameAckEliciting(f wire.Frame) bool {
}
// HasAckElicitingFrames returns true if at least one frame is ack-eliciting.
func HasAckElicitingFrames(fs []Frame) bool {
func HasAckElicitingFrames(fs []*Frame) bool {
for _, f := range fs {
if IsFrameAckEliciting(f.Frame) {
return true

View File

@@ -28,7 +28,7 @@ var _ = Describe("ack-eliciting frames", func() {
})
It("HasAckElicitingFrames works for "+fName, func() {
Expect(HasAckElicitingFrames([]Frame{{Frame: f}})).To(Equal(e))
Expect(HasAckElicitingFrames([]*Frame{{Frame: f}})).To(Equal(e))
})
}
})

View File

@@ -1,9 +1,29 @@
package ackhandler
import "github.com/lucas-clemente/quic-go/internal/wire"
import (
"sync"
"github.com/lucas-clemente/quic-go/internal/wire"
)
type Frame struct {
wire.Frame // nil if the frame has already been acknowledged in another packet
OnLost func(wire.Frame)
OnAcked func(wire.Frame)
}
var framePool = sync.Pool{New: func() any { return &Frame{} }}
func GetFrame() *Frame {
f := framePool.Get().(*Frame)
f.OnLost = nil
f.OnAcked = nil
return f
}
func putFrame(f *Frame) {
f.Frame = nil
f.OnLost = nil
f.OnAcked = nil
framePool.Put(f)
}

View File

@@ -10,7 +10,7 @@ import (
// A Packet is a packet
type Packet struct {
PacketNumber protocol.PacketNumber
Frames []Frame
Frames []*Frame
LargestAcked protocol.PacketNumber // InvalidPacketNumber if the packet doesn't contain an ACK
Length protocol.ByteCount
EncryptionLevel protocol.EncryptionLevel
@@ -46,4 +46,10 @@ func GetPacket() *Packet {
// We currently only return Packets back into the pool when they're acknowledged (not when they're lost).
// This simplifies the code, and gives the vast majority of the performance benefit we can gain from using the pool.
func putPacket(p *Packet) { packetPool.Put(p) }
func putPacket(p *Packet) {
for _, f := range p.Frames {
putFrame(f)
}
p.Frames = nil
packetPool.Put(p)
}

View File

@@ -54,7 +54,7 @@ var _ = Describe("SentPacketHandler", func() {
p.SendTime = time.Now()
}
if len(p.Frames) == 0 {
p.Frames = []Frame{
p.Frames = []*Frame{
{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) { lostPackets = append(lostPackets, p.PacketNumber) }},
}
}
@@ -276,7 +276,7 @@ var _ = Describe("SentPacketHandler", func() {
ping := &wire.PingFrame{}
handler.SentPacket(ackElicitingPacket(&Packet{
PacketNumber: 13,
Frames: []Frame{{
Frames: []*Frame{{
Frame: ping, OnAcked: func(f wire.Frame) {
Expect(f).To(Equal(ping))
acked = true
@@ -428,20 +428,20 @@ var _ = Describe("SentPacketHandler", func() {
{
PacketNumber: 13,
LargestAcked: 100,
Frames: []Frame{{Frame: &streamFrame, OnLost: func(wire.Frame) {}}},
Frames: []*Frame{{Frame: &streamFrame, OnLost: func(wire.Frame) {}}},
Length: 1,
EncryptionLevel: protocol.Encryption1RTT,
},
{
PacketNumber: 14,
LargestAcked: 200,
Frames: []Frame{{Frame: &streamFrame, OnLost: func(wire.Frame) {}}},
Frames: []*Frame{{Frame: &streamFrame, OnLost: func(wire.Frame) {}}},
Length: 1,
EncryptionLevel: protocol.Encryption1RTT,
},
{
PacketNumber: 15,
Frames: []Frame{{Frame: &streamFrame, OnLost: func(wire.Frame) {}}},
Frames: []*Frame{{Frame: &streamFrame, OnLost: func(wire.Frame) {}}},
Length: 1,
EncryptionLevel: protocol.Encryption1RTT,
},
@@ -501,7 +501,7 @@ var _ = Describe("SentPacketHandler", func() {
handler.SentPacket(&Packet{
PacketNumber: 1,
Length: 42,
Frames: []Frame{{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) {}}},
Frames: []*Frame{{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) {}}},
EncryptionLevel: protocol.Encryption1RTT,
})
})
@@ -548,7 +548,7 @@ var _ = Describe("SentPacketHandler", func() {
PacketNumber: 1,
SendTime: time.Now().Add(-time.Hour),
IsPathMTUProbePacket: true,
Frames: []Frame{{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) { mtuPacketDeclaredLost = true }}},
Frames: []*Frame{{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) { mtuPacketDeclaredLost = true }}},
}))
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2}))
// lose packet 1, but don't EXPECT any calls to OnPacketLost()
@@ -595,7 +595,7 @@ var _ = Describe("SentPacketHandler", func() {
handler.SentPacket(&Packet{
Length: 42,
EncryptionLevel: protocol.EncryptionInitial,
Frames: []Frame{{Frame: &wire.PingFrame{}}},
Frames: []*Frame{{Frame: &wire.PingFrame{}}},
SendTime: time.Now(),
})
cong.EXPECT().CanSend(protocol.ByteCount(42)).Return(true)
@@ -755,7 +755,7 @@ var _ = Describe("SentPacketHandler", func() {
handler.SentPacket(ackElicitingPacket(&Packet{
PacketNumber: 1,
SendTime: time.Now().Add(-time.Hour),
Frames: []Frame{
Frames: []*Frame{
{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) { lostPackets = append(lostPackets, 1) }},
},
}))
@@ -775,7 +775,7 @@ var _ = Describe("SentPacketHandler", func() {
handler.SentPacket(ackElicitingPacket(&Packet{
PacketNumber: pn,
SendTime: time.Now().Add(-time.Hour),
Frames: []Frame{
Frames: []*Frame{
{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) { lostPackets = append(lostPackets, 1) }},
},
}))
@@ -895,7 +895,7 @@ var _ = Describe("SentPacketHandler", func() {
PacketNumber: 1,
Length: 599,
EncryptionLevel: protocol.EncryptionInitial,
Frames: []Frame{{Frame: &wire.PingFrame{}}},
Frames: []*Frame{{Frame: &wire.PingFrame{}}},
SendTime: time.Now(),
})
Expect(handler.SendMode()).To(Equal(SendAny))
@@ -903,7 +903,7 @@ var _ = Describe("SentPacketHandler", func() {
PacketNumber: 2,
Length: 1,
EncryptionLevel: protocol.EncryptionInitial,
Frames: []Frame{{Frame: &wire.PingFrame{}}},
Frames: []*Frame{{Frame: &wire.PingFrame{}}},
SendTime: time.Now(),
})
Expect(handler.SendMode()).To(Equal(SendNone))
@@ -915,7 +915,7 @@ var _ = Describe("SentPacketHandler", func() {
PacketNumber: 1,
Length: 900,
EncryptionLevel: protocol.EncryptionInitial,
Frames: []Frame{{Frame: &wire.PingFrame{}}},
Frames: []*Frame{{Frame: &wire.PingFrame{}}},
SendTime: time.Now(),
})
// Amplification limited. We don't need to set a timer now.
@@ -931,7 +931,7 @@ var _ = Describe("SentPacketHandler", func() {
PacketNumber: 1,
Length: 900,
EncryptionLevel: protocol.EncryptionHandshake,
Frames: []Frame{{Frame: &wire.PingFrame{}}},
Frames: []*Frame{{Frame: &wire.PingFrame{}}},
SendTime: time.Now(),
})
// Amplification limited. We don't need to set a timer now.
@@ -965,7 +965,7 @@ var _ = Describe("SentPacketHandler", func() {
PacketNumber: 1,
Length: 900,
EncryptionLevel: protocol.EncryptionInitial,
Frames: []Frame{{Frame: &wire.PingFrame{}}},
Frames: []*Frame{{Frame: &wire.PingFrame{}}},
SendTime: time.Now(),
})
Expect(handler.SendMode()).To(Equal(SendAny))
@@ -1158,7 +1158,7 @@ var _ = Describe("SentPacketHandler", func() {
PacketNumber: 1,
SendTime: now.Add(-3 * time.Second),
IsPathMTUProbePacket: true,
Frames: []Frame{{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) { mtuPacketDeclaredLost = true }}},
Frames: []*Frame{{Frame: &wire.PingFrame{}, OnLost: func(wire.Frame) { mtuPacketDeclaredLost = true }}},
}))
handler.SentPacket(ackElicitingPacket(&Packet{PacketNumber: 2, SendTime: now.Add(-3 * time.Second)}))
ack := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 2, Largest: 2}}}
@@ -1341,7 +1341,7 @@ var _ = Describe("SentPacketHandler", func() {
handler.SentPacket(&Packet{
PacketNumber: 13,
EncryptionLevel: protocol.EncryptionInitial,
Frames: []Frame{
Frames: []*Frame{
{Frame: &wire.CryptoFrame{Data: []byte("foobar")}, OnLost: func(wire.Frame) { lostInitial = true }},
},
Length: 100,
@@ -1350,7 +1350,7 @@ var _ = Describe("SentPacketHandler", func() {
handler.SentPacket(&Packet{
PacketNumber: pn,
EncryptionLevel: protocol.Encryption0RTT,
Frames: []Frame{
Frames: []*Frame{
{Frame: &wire.StreamFrame{Data: []byte("foobar")}, OnLost: func(wire.Frame) { lost0RTT = true }},
},
Length: 999,