ackhandler: use an iterator to process received packet ranges (#5309)

* ackhandler: use an interator to track received packet ranges

* use slices.Collect

* naming convention

diff --git a/internal/ackhandler/received_packet_history.go b/internal/ackhandler/received_packet_history.go
index 091e7fc11fb17074b44e626e878147515eafa7c7..d065b6e65996644608919042c7a0655cc32cce63 100644 (file)
--- a/internal/ackhandler/received_packet_history.go
+++ b/internal/ackhandler/received_packet_history.go
@@ -1,10 +1,10 @@
 package ackhandler
 
 import (
+       "iter"
        "slices"
 
        "github.com/quic-go/quic-go/internal/protocol"
-       "github.com/quic-go/quic-go/internal/wire"
 )
 
 // interval is an interval from one PacketNumber to the other
@@ -109,12 +109,15 @@ func (h *receivedPacketHistory) DeleteBelow(p protocol.PacketNumber) {
        }
 }
 
-// AppendAckRanges appends to a slice of all AckRanges that can be used in an AckFrame
-func (h *receivedPacketHistory) AppendAckRanges(ackRanges []wire.AckRange) []wire.AckRange {
-       for i := len(h.ranges) - 1; i >= 0; i-- {
-               ackRanges = append(ackRanges, wire.AckRange{Smallest: h.ranges[i].Start, Largest: h.ranges[i].End})
+// Backward returns an iterator over the ranges in reverse order
+func (h *receivedPacketHistory) Backward() iter.Seq[interval] {
+       return func(yield func(interval) bool) {
+               for i := len(h.ranges) - 1; i >= 0; i-- {
+                       if !yield(h.ranges[i]) {
+                               return
+                       }
+               }
        }
-       return ackRanges
 }
 
 func (h *receivedPacketHistory) HighestMissingUpTo(p protocol.PacketNumber) protocol.PacketNumber {

diff --git a/internal/ackhandler/received_packet_history_test.go b/internal/ackhandler/received_packet_history_test.go
index 67a9f3458c88481a4cd3bcf70dee83eda049cb6a..9e0d883ca5398545ef73a7cda3952982075acd64 100644 (file)
--- a/internal/ackhandler/received_packet_history_test.go
+++ b/internal/ackhandler/received_packet_history_test.go
@@ -6,7 +6,6 @@ import (
        "testing"
 
        "github.com/quic-go/quic-go/internal/protocol"
-       "github.com/quic-go/quic-go/internal/wire"
 
        "github.com/stretchr/testify/require"
 )
@@ -15,24 +14,24 @@ func TestReceivedPacketHistorySingleRange(t *testing.T) {
        hist := newReceivedPacketHistory()
 
        require.True(t, hist.ReceivedPacket(4))
-       require.Equal(t, []wire.AckRange{{Smallest: 4, Largest: 4}}, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{{Start: 4, End: 4}}, slices.Collect(hist.Backward()))
 
        // add a duplicate packet
        require.False(t, hist.ReceivedPacket(4))
-       require.Equal(t, []wire.AckRange{{Smallest: 4, Largest: 4}}, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{{Start: 4, End: 4}}, slices.Collect(hist.Backward()))
 
        // add a few more packets to extend the range
        require.True(t, hist.ReceivedPacket(5))
        require.True(t, hist.ReceivedPacket(6))
-       require.Equal(t, []wire.AckRange{{Smallest: 4, Largest: 6}}, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{{Start: 4, End: 6}}, slices.Collect(hist.Backward()))
 
        // add a duplicate within this range
        require.False(t, hist.ReceivedPacket(5))
-       require.Equal(t, []wire.AckRange{{Smallest: 4, Largest: 6}}, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{{Start: 4, End: 6}}, slices.Collect(hist.Backward()))
 
        // extend the range at the front
        require.True(t, hist.ReceivedPacket(3))
-       require.Equal(t, []wire.AckRange{{Smallest: 3, Largest: 6}}, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{{Start: 3, End: 6}}, slices.Collect(hist.Backward()))
 }
 
 func TestReceivedPacketHistoryRanges(t *testing.T) {
@@ -46,53 +45,53 @@ func TestReceivedPacketHistoryRanges(t *testing.T) {
        require.Equal(t, protocol.PacketNumber(2), hist.HighestMissingUpTo(2))
        require.True(t, hist.ReceivedPacket(10))
        require.Equal(t, protocol.PacketNumber(9), hist.HighestMissingUpTo(1000))
-       require.Equal(t, []wire.AckRange{
-               {Smallest: 10, Largest: 10},
-               {Smallest: 4, Largest: 4},
-       }, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{
+               {Start: 10, End: 10},
+               {Start: 4, End: 4},
+       }, slices.Collect(hist.Backward()))
 
        // create a new range in the middle
        require.True(t, hist.ReceivedPacket(7))
-       require.Equal(t, []wire.AckRange{
-               {Smallest: 10, Largest: 10},
-               {Smallest: 7, Largest: 7},
-               {Smallest: 4, Largest: 4},
-       }, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{
+               {Start: 10, End: 10},
+               {Start: 7, End: 7},
+               {Start: 4, End: 4},
+       }, slices.Collect(hist.Backward()))
 
        // create a new range at the front
        require.True(t, hist.ReceivedPacket(1))
-       require.Equal(t, []wire.AckRange{
-               {Smallest: 10, Largest: 10},
-               {Smallest: 7, Largest: 7},
-               {Smallest: 4, Largest: 4},
-               {Smallest: 1, Largest: 1},
-       }, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{
+               {Start: 10, End: 10},
+               {Start: 7, End: 7},
+               {Start: 4, End: 4},
+               {Start: 1, End: 1},
+       }, slices.Collect(hist.Backward()))
 
        // extend an existing range at the end
        require.True(t, hist.ReceivedPacket(8))
-       require.Equal(t, []wire.AckRange{
-               {Smallest: 10, Largest: 10},
-               {Smallest: 7, Largest: 8},
-               {Smallest: 4, Largest: 4},
-               {Smallest: 1, Largest: 1},
-       }, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{
+               {Start: 10, End: 10},
+               {Start: 7, End: 8},
+               {Start: 4, End: 4},
+               {Start: 1, End: 1},
+       }, slices.Collect(hist.Backward()))
 
        // extend an existing range at the front
        require.True(t, hist.ReceivedPacket(6))
-       require.Equal(t, []wire.AckRange{
-               {Smallest: 10, Largest: 10},
-               {Smallest: 6, Largest: 8},
-               {Smallest: 4, Largest: 4},
-               {Smallest: 1, Largest: 1},
-       }, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{
+               {Start: 10, End: 10},
+               {Start: 6, End: 8},
+               {Start: 4, End: 4},
+               {Start: 1, End: 1},
+       }, slices.Collect(hist.Backward()))
 
        // close a range
        require.True(t, hist.ReceivedPacket(9))
-       require.Equal(t, []wire.AckRange{
-               {Smallest: 6, Largest: 10},
-               {Smallest: 4, Largest: 4},
-               {Smallest: 1, Largest: 1},
-       }, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{
+               {Start: 6, End: 10},
+               {Start: 4, End: 4},
+               {Start: 1, End: 1},
+       }, slices.Collect(hist.Backward()))
 }
 
 func TestReceivedPacketHistoryMaxNumAckRanges(t *testing.T) {
@@ -114,7 +113,7 @@ func TestReceivedPacketHistoryDeleteBelow(t *testing.T) {
        hist := newReceivedPacketHistory()
 
        hist.DeleteBelow(2)
-       require.Empty(t, hist.AppendAckRanges(nil))
+       require.Empty(t, slices.Collect(hist.Backward()))
 
        require.True(t, hist.ReceivedPacket(2))
        require.True(t, hist.ReceivedPacket(4))
@@ -126,27 +125,27 @@ func TestReceivedPacketHistoryDeleteBelow(t *testing.T) {
        hist.DeleteBelow(6)
        require.Equal(t, protocol.InvalidPacketNumber, hist.HighestMissingUpTo(6))
        require.Equal(t, protocol.PacketNumber(9), hist.HighestMissingUpTo(10))
-       require.Equal(t, []wire.AckRange{
-               {Smallest: 10, Largest: 10},
-               {Smallest: 6, Largest: 6},
-       }, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{
+               {Start: 10, End: 10},
+               {Start: 6, End: 6},
+       }, slices.Collect(hist.Backward()))
 
        // deleting from an existing range
        require.True(t, hist.ReceivedPacket(7))
        require.True(t, hist.ReceivedPacket(8))
        hist.DeleteBelow(7)
-       require.Equal(t, []wire.AckRange{
-               {Smallest: 10, Largest: 10},
-               {Smallest: 7, Largest: 8},
-       }, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{
+               {Start: 10, End: 10},
+               {Start: 7, End: 8},
+       }, slices.Collect(hist.Backward()))
 
        // keep a one-packet range
        hist.DeleteBelow(10)
-       require.Equal(t, []wire.AckRange{{Smallest: 10, Largest: 10}}, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{{Start: 10, End: 10}}, slices.Collect(hist.Backward()))
 
        // delayed packets below deleted ranges are ignored
        require.False(t, hist.ReceivedPacket(5))
-       require.Equal(t, []wire.AckRange{{Smallest: 10, Largest: 10}}, hist.AppendAckRanges(nil))
+       require.Equal(t, []interval{{Start: 10, End: 10}}, slices.Collect(hist.Backward()))
 }
 
 func TestReceivedPacketHistoryDuplicateDetection(t *testing.T) {
@@ -213,11 +212,11 @@ func TestReceivedPacketHistoryRandomized(t *testing.T) {
                }
        }
        var counter int
-       ackRanges := hist.AppendAckRanges(nil)
+       ackRanges := slices.Collect(hist.Backward())
        t.Logf("ACK ranges: %v", ackRanges)
        require.LessOrEqual(t, len(ackRanges), numLostPackets+1)
        for _, ackRange := range ackRanges {
-               for p := ackRange.Smallest; p <= ackRange.Largest; p++ {
+               for p := ackRange.Start; p <= ackRange.End; p++ {
                        counter++
                        require.Contains(t, packets, p)
                }

diff --git a/internal/ackhandler/received_packet_tracker.go b/internal/ackhandler/received_packet_tracker.go
index 32ec8a73c7112f884a6933ee576b4cdb5dc19804..c16400063e0fe21a90e343d52657423c4c8b6916 100644 (file)
--- a/internal/ackhandler/received_packet_tracker.go
+++ b/internal/ackhandler/received_packet_tracker.go
@@ -61,7 +61,9 @@ func (h *receivedPacketTracker) GetAckFrame() *wire.AckFrame {
        ack.ECT0 = h.ect0
        ack.ECT1 = h.ect1
        ack.ECNCE = h.ecnce
-       ack.AckRanges = h.packetHistory.AppendAckRanges(ack.AckRanges)
+       for r := range h.packetHistory.Backward() {
+               ack.AckRanges = append(ack.AckRanges, wire.AckRange{Smallest: r.Start, Largest: r.End})
+       }
 
        h.lastAck = ack
        h.hasNewAck = false