handle ACKs separately in the sent packet handler packet struct

internal/ackhandler/ack_eliciting.go

@@ -2,17 +2,6 @@ package ackhandler
 
 import "github.com/lucas-clemente/quic-go/internal/wire"
 
-// Returns a new slice with all non-ack-eliciting frames deleted.
-func stripNonAckElicitingFrames(fs []wire.Frame) []wire.Frame {
-	res := make([]wire.Frame, 0, len(fs))
-	for _, f := range fs {
-		if IsFrameAckEliciting(f) {
-			res = append(res, f)
-		}
-	}
-	return res
-}
-
 // IsFrameAckEliciting returns true if the frame is ack-eliciting.
 func IsFrameAckEliciting(f wire.Frame) bool {
 	switch f.(type) {

internal/ackhandler/ack_eliciting_test.go

@@ -27,15 +27,6 @@ var _ = Describe("ack-eliciting frames", func() {
 			Expect(IsFrameAckEliciting(f)).To(Equal(e))
 		})
 
-		It("stripping non-ack-elicinting frames works for "+fName, func() {
-			s := []wire.Frame{f}
-			if e {
-				Expect(stripNonAckElicitingFrames(s)).To(Equal([]wire.Frame{f}))
-			} else {
-				Expect(stripNonAckElicitingFrames(s)).To(BeEmpty())
-			}
-		})
-
 		It("HasAckElicitingFrames works for "+fName, func() {
 			Expect(HasAckElicitingFrames([]wire.Frame{f})).To(Equal(e))
 		})

internal/ackhandler/packet.go

@@ -11,6 +11,7 @@ import (
 type Packet struct {
 	PacketNumber    protocol.PacketNumber
 	PacketType      protocol.PacketType
+	Ack             *wire.AckFrame
 	Frames          []wire.Frame
 	Length          protocol.ByteCount
 	EncryptionLevel protocol.EncryptionLevel

internal/ackhandler/sent_packet_handler.go

@@ -166,14 +166,12 @@ func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* is ack-elicit
 
 	pnSpace.largestSent = packet.PacketNumber
 
-	if len(packet.Frames) > 0 {
-		if ackFrame, ok := packet.Frames[0].(*wire.AckFrame); ok {
-			packet.largestAcked = ackFrame.LargestAcked()
-		}
+	if packet.Ack != nil {
+		packet.largestAcked = packet.Ack.LargestAcked()
 	}
+	packet.Ack = nil // no need to save the ACK
 
-	packet.Frames = stripNonAckElicitingFrames(packet.Frames)
-	isAckEliciting := len(packet.Frames) != 0
+	isAckEliciting := len(packet.Frames) > 0
 
 	if isAckEliciting {
 		if packet.EncryptionLevel != protocol.Encryption1RTT {

internal/ackhandler/sent_packet_handler_test.go

@@ -29,9 +29,8 @@ func ackElicitingPacket(p *Packet) *Packet {
 
 func nonAckElicitingPacket(p *Packet) *Packet {
 	p = ackElicitingPacket(p)
-	p.Frames = []wire.Frame{
-		&wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}},
-	}
+	p.Frames = nil
+	p.Ack = &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}}
 	return p
 }
 
@@ -313,8 +312,8 @@ var _ = Describe("SentPacketHandler", func() {
 				ack1 := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 80, Largest: 100}}}
 				ack2 := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 50, Largest: 200}}}
 				morePackets := []*Packet{
-					{PacketNumber: 13, Frames: []wire.Frame{ack1, &streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
-					{PacketNumber: 14, Frames: []wire.Frame{ack2, &streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
+					{PacketNumber: 13, Ack: ack1, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
+					{PacketNumber: 14, Ack: ack2, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
 					{PacketNumber: 15, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
 				}
 				for _, packet := range morePackets {

packet_packer.go

@@ -52,10 +52,24 @@ func (p *packedPacket) IsAckEliciting() bool {
 }
 
 func (p *packedPacket) ToAckHandlerPacket() *ackhandler.Packet {
+	var frames []wire.Frame
+	var ack *wire.AckFrame
+	if len(p.frames) > 0 {
+		var ok bool
+		ack, ok = p.frames[0].(*wire.AckFrame)
+		if ok {
+			// make a copy, so that the ACK can be garbage collected
+			frames = make([]wire.Frame, len(p.frames)-1)
+			copy(frames, p.frames[1:])
+		} else {
+			frames = p.frames
+		}
+	}
 	return &ackhandler.Packet{
 		PacketNumber:    p.header.PacketNumber,
 		PacketType:      p.header.Type,
-		Frames:          p.frames,
+		Ack:             ack,
+		Frames:          frames,
 		Length:          protocol.ByteCount(len(p.raw)),
 		EncryptionLevel: p.EncryptionLevel(),
 		SendTime:        time.Now(),
@@ -330,7 +344,7 @@ func (p *packetPacker) composeNextPacket(maxFrameSize protocol.ByteCount) ([]wir
 	var length protocol.ByteCount
 	var frames []wire.Frame
 
-	// ACKs need to go first, so that the sentPacketHandler will recognize them
+	// ACKs need to go first, so we recognize them in packedPacket.ToAckHandlerPacket()
 	if ack := p.acks.GetAckFrame(protocol.Encryption1RTT); ack != nil {
 		frames = append(frames, ack)
 		length += ack.Length(p.version)