append ACK ranges instead of allocating a new slice

quic-go · Sep 5, 2022 · c328918 · c328918
1 parent 62b8278
commit c328918
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 16 deletions.
diff --git a/internal/ackhandler/received_packet_history.go b/internal/ackhandler/received_packet_history.go
@@ -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
 }

diff --git a/internal/ackhandler/received_packet_history_test.go b/internal/ackhandler/received_packet_history_test.go
@@ -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 {

diff --git a/internal/ackhandler/received_packet_tracker.go b/internal/ackhandler/received_packet_tracker.go
@@ -172,7 +172,7 @@ func (h *receivedPacketTracker) GetAckFrame(onlyIfQueued bool) *wire.AckFrame {
 	}
 
 	ack := &wire.AckFrame{
-		AckRanges: h.packetHistory.GetAckRanges(),
+		AckRanges: h.packetHistory.AppendAckRanges(nil),
 		// 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)),