ackhandler: use a slice to keep track of sent packets (#3841)

* ackhandler: simplify deletion of old packets in packet history

* ackhandler: use a slice to keep track of sent packets

This is a vastly simpler data structure than the combination of map
and linked list used before. It avoids using a linked list (bad cache
locality) and a sync.Pool (for list elements), as well as having to do
hash table lookups.

In the future, this can be easily replaces with a ring buffer, avoiding
all allocations.

* ackhandler: don't store packets that were declared lost

internal/ackhandler/sent_packet_handler.go

@@ -38,7 +38,7 @@ type packetNumberSpace struct {
 	largestSent  protocol.PacketNumber
 }
 
-func newPacketNumberSpace(initialPN protocol.PacketNumber, skipPNs bool, rttStats *utils.RTTStats) *packetNumberSpace {
+func newPacketNumberSpace(initialPN protocol.PacketNumber, skipPNs bool) *packetNumberSpace {
 	var pns packetNumberGenerator
 	if skipPNs {
 		pns = newSkippingPacketNumberGenerator(initialPN, protocol.SkipPacketInitialPeriod, protocol.SkipPacketMaxPeriod)
@@ -46,7 +46,7 @@ func newPacketNumberSpace(initialPN protocol.PacketNumber, skipPNs bool, rttStat
 		pns = newSequentialPacketNumberGenerator(initialPN)
 	}
 	return &packetNumberSpace{
-		history:      newSentPacketHistory(rttStats),
+		history:      newSentPacketHistory(),
 		pns:          pns,
 		largestSent:  protocol.InvalidPacketNumber,
 		largestAcked: protocol.InvalidPacketNumber,
@@ -125,9 +125,9 @@ func newSentPacketHandler(
 	return &sentPacketHandler{
 		peerCompletedAddressValidation: pers == protocol.PerspectiveServer,
 		peerAddressValidated:           pers == protocol.PerspectiveClient || clientAddressValidated,
-		initialPackets:                 newPacketNumberSpace(initialPN, false, rttStats),
+		initialPackets:                 newPacketNumberSpace(initialPN, false),
-		handshakePackets:               newPacketNumberSpace(0, false, rttStats),
+		handshakePackets:               newPacketNumberSpace(0, false),
-		appDataPackets:                 newPacketNumberSpace(0, true, rttStats),
+		appDataPackets:                 newPacketNumberSpace(0, true),
 		rttStats:                       rttStats,
 		congestion:                     congestion,
 		perspective:                    pers,
@@ -356,7 +356,6 @@ func (h *sentPacketHandler) ReceivedAck(ack *wire.AckFrame, encLevel protocol.En
 		h.tracer.UpdatedMetrics(h.rttStats, h.congestion.GetCongestionWindow(), h.bytesInFlight, h.packetsInFlight())
 	}
 
-	pnSpace.history.DeleteOldPackets(rcvTime)
 	h.setLossDetectionTimer()
 	return acked1RTTPacket, nil
 }
@@ -589,9 +588,6 @@ func (h *sentPacketHandler) detectLostPackets(now time.Time, encLevel protocol.E
 		if p.PacketNumber > pnSpace.largestAcked {
 			return false, nil
 		}
-		if p.declaredLost || p.skippedPacket {
-			return true, nil
-		}
 
 		var packetLost bool
 		if p.SendTime.Before(lostSendTime) {
@@ -619,12 +615,14 @@ func (h *sentPacketHandler) detectLostPackets(now time.Time, encLevel protocol.E
 			pnSpace.lossTime = lossTime
 		}
 		if packetLost {
-			p = pnSpace.history.DeclareLost(p)
+			pnSpace.history.DeclareLost(p.PacketNumber)
-			// the bytes in flight need to be reduced no matter if the frames in this packet will be retransmitted
+			if !p.skippedPacket {
-			h.removeFromBytesInFlight(p)
+				// the bytes in flight need to be reduced no matter if the frames in this packet will be retransmitted
-			h.queueFramesForRetransmission(p)
+				h.removeFromBytesInFlight(p)
-			if !p.IsPathMTUProbePacket {
+				h.queueFramesForRetransmission(p)
-				h.congestion.OnPacketLost(p.PacketNumber, p.Length, priorInFlight)
+				if !p.IsPathMTUProbePacket {
+					h.congestion.OnPacketLost(p.PacketNumber, p.Length, priorInFlight)
+				}
 			}
 		}
 		return true, nil
@@ -780,7 +778,7 @@ func (h *sentPacketHandler) QueueProbePacket(encLevel protocol.EncryptionLevel)
 	// TODO: don't declare the packet lost here.
 	// Keep track of acknowledged frames instead.
 	h.removeFromBytesInFlight(p)
-	pnSpace.history.DeclareLost(p)
+	pnSpace.history.DeclareLost(p.PacketNumber)
 	return true
 }
 
@@ -833,8 +831,8 @@ func (h *sentPacketHandler) ResetForRetry() error {
 			h.tracer.UpdatedMetrics(h.rttStats, h.congestion.GetCongestionWindow(), h.bytesInFlight, h.packetsInFlight())
 		}
 	}
-	h.initialPackets = newPacketNumberSpace(h.initialPackets.pns.Pop(), false, h.rttStats)
+	h.initialPackets = newPacketNumberSpace(h.initialPackets.pns.Pop(), false)
-	h.appDataPackets = newPacketNumberSpace(h.appDataPackets.pns.Pop(), true, h.rttStats)
+	h.appDataPackets = newPacketNumberSpace(h.appDataPackets.pns.Pop(), true)
 	oldAlarm := h.alarm
 	h.alarm = time.Time{}
 	if h.tracer != nil {

internal/ackhandler/sent_packet_handler_test.go

@@ -37,8 +37,10 @@ var _ = Describe("SentPacketHandler", func() {
 	})
 
 	getPacket := func(pn protocol.PacketNumber, encLevel protocol.EncryptionLevel) *Packet {
-		if el, ok := handler.getPacketNumberSpace(encLevel).history.packetMap[pn]; ok {
+		for _, p := range handler.getPacketNumberSpace(encLevel).history.packets {
-			return el.Value
+			if p != nil && p.PacketNumber == pn {
+				return p
+			}
 		}
 		return nil
 	}
@@ -97,7 +99,7 @@ var _ = Describe("SentPacketHandler", func() {
 		})
 		ExpectWithOffset(1, length).To(Equal(len(expected)))
 		for _, p := range expected {
-			ExpectWithOffset(2, pnSpace.history.packetMap).To(HaveKey(p))
+			ExpectWithOffset(2, getPacket(p, encLevel)).ToNot(BeNil())
 		}
 	}
 
@@ -242,7 +244,7 @@ var _ = Describe("SentPacketHandler", func() {
 				ExpectWithOffset(1, length+len(lostPackets)).To(Equal(len(expected)))
 			expectedLoop:
 				for _, p := range expected {
-					if _, ok := pnSpace.history.packetMap[p]; ok {
+					if getPacket(p, encLevel) != nil {
 						continue
 					}
 					for _, lostP := range lostPackets {

internal/ackhandler/sent_packet_history.go

@@ -2,162 +2,178 @@ package ackhandler
 
 import (
 	"fmt"
-	"sync"
 	"time"
 
 	"github.com/quic-go/quic-go/internal/protocol"
-	"github.com/quic-go/quic-go/internal/utils"
-	list "github.com/quic-go/quic-go/internal/utils/linkedlist"
 )
 
 type sentPacketHistory struct {
-	rttStats              *utils.RTTStats
+	packets []*Packet
-	outstandingPacketList *list.List[*Packet]
+
-	etcPacketList         *list.List[*Packet]
+	numOutstanding int
-	packetMap             map[protocol.PacketNumber]*list.Element[*Packet]
+
-	highestSent           protocol.PacketNumber
+	highestPacketNumber protocol.PacketNumber
 }
 
-var packetElementPool sync.Pool
+func newSentPacketHistory() *sentPacketHistory {
-
-func init() {
-	packetElementPool = *list.NewPool[*Packet]()
-}
-
-func newSentPacketHistory(rttStats *utils.RTTStats) *sentPacketHistory {
 	return &sentPacketHistory{
-		rttStats:              rttStats,
+		packets:             make([]*Packet, 0, 32),
-		outstandingPacketList: list.NewWithPool[*Packet](&packetElementPool),
+		highestPacketNumber: protocol.InvalidPacketNumber,
-		etcPacketList:         list.NewWithPool[*Packet](&packetElementPool),
-		packetMap:             make(map[protocol.PacketNumber]*list.Element[*Packet]),
-		highestSent:           protocol.InvalidPacketNumber,
 	}
 }
 
 func (h *sentPacketHistory) SentNonAckElicitingPacket(pn protocol.PacketNumber, encLevel protocol.EncryptionLevel, t time.Time) {
-	h.registerSentPacket(pn, encLevel, t)
+	h.maybeAddSkippedPacketsBefore(pn, encLevel, t)
+	h.highestPacketNumber = pn
+	if len(h.packets) > 0 {
+		h.packets = append(h.packets, nil)
+	}
 }
 
 func (h *sentPacketHistory) SentAckElicitingPacket(p *Packet) {
-	h.registerSentPacket(p.PacketNumber, p.EncryptionLevel, p.SendTime)
+	h.maybeAddSkippedPacketsBefore(p.PacketNumber, p.EncryptionLevel, p.SendTime)
-
+	h.packets = append(h.packets, p)
-	var el *list.Element[*Packet]
 	if p.outstanding() {
-		el = h.outstandingPacketList.PushBack(p)
+		h.numOutstanding++
-	} else {
-		el = h.etcPacketList.PushBack(p)
 	}
-	h.packetMap[p.PacketNumber] = el
+	h.highestPacketNumber = p.PacketNumber
 }
 
-func (h *sentPacketHistory) registerSentPacket(pn protocol.PacketNumber, encLevel protocol.EncryptionLevel, t time.Time) {
+func (h *sentPacketHistory) maybeAddSkippedPacketsBefore(pn protocol.PacketNumber, encLevel protocol.EncryptionLevel, t time.Time) {
-	if pn <= h.highestSent {
+	if pn <= h.highestPacketNumber {
 		panic("non-sequential packet number use")
 	}
-	// Skipped packet numbers.
+	var start protocol.PacketNumber
-	for p := h.highestSent + 1; p < pn; p++ {
+	if h.highestPacketNumber != protocol.InvalidPacketNumber {
-		el := h.etcPacketList.PushBack(&Packet{
+		start = h.highestPacketNumber + 1
+	}
+	for p := start; p < pn; p++ {
+		h.packets = append(h.packets, &Packet{
 			PacketNumber:    p,
 			EncryptionLevel: encLevel,
 			SendTime:        t,
 			skippedPacket:   true,
 		})
-		h.packetMap[p] = el
 	}
-	h.highestSent = pn
 }
 
 // Iterate iterates through all packets.
 func (h *sentPacketHistory) Iterate(cb func(*Packet) (cont bool, err error)) error {
-	cont := true
+	for _, p := range h.packets {
-	outstandingEl := h.outstandingPacketList.Front()
+		if p == nil {
-	etcEl := h.etcPacketList.Front()
+			continue
-	var el *list.Element[*Packet]
-	// whichever has the next packet number is returned first
-	for cont {
-		if outstandingEl == nil || (etcEl != nil && etcEl.Value.PacketNumber < outstandingEl.Value.PacketNumber) {
-			el = etcEl
-		} else {
-			el = outstandingEl
 		}
-		if el == nil {
+		cont, err := cb(p)
-			return nil
-		}
-		if el == outstandingEl {
-			outstandingEl = outstandingEl.Next()
-		} else {
-			etcEl = etcEl.Next()
-		}
-		var err error
-		cont, err = cb(el.Value)
 		if err != nil {
 			return err
 		}
+		if !cont {
+			return nil
+		}
 	}
 	return nil
 }
 
 // FirstOutstanding returns the first outstanding packet.
 func (h *sentPacketHistory) FirstOutstanding() *Packet {
-	el := h.outstandingPacketList.Front()
+	if !h.HasOutstandingPackets() {
-	if el == nil {
 		return nil
 	}
-	return el.Value
+	for _, p := range h.packets {
-}
+		if p != nil && p.outstanding() {
-
+			return p
-func (h *sentPacketHistory) Len() int {
+		}
-	return len(h.packetMap)
-}
-
-func (h *sentPacketHistory) Remove(p protocol.PacketNumber) error {
-	el, ok := h.packetMap[p]
-	if !ok {
-		return fmt.Errorf("packet %d not found in sent packet history", p)
 	}
-	el.List().Remove(el)
-	delete(h.packetMap, p)
 	return nil
 }
 
-func (h *sentPacketHistory) HasOutstandingPackets() bool {
+func (h *sentPacketHistory) Len() int {
-	return h.outstandingPacketList.Len() > 0
+	return len(h.packets)
 }
 
-func (h *sentPacketHistory) DeleteOldPackets(now time.Time) {
+func (h *sentPacketHistory) Remove(pn protocol.PacketNumber) error {
-	maxAge := 3 * h.rttStats.PTO(false)
+	idx, ok := h.getIndex(pn)
-	var nextEl *list.Element[*Packet]
-	// we don't iterate outstandingPacketList, as we should not delete outstanding packets.
-	// being outstanding for more than 3*PTO should only happen in the case of drastic RTT changes.
-	for el := h.etcPacketList.Front(); el != nil; el = nextEl {
-		nextEl = el.Next()
-		p := el.Value
-		if p.SendTime.After(now.Add(-maxAge)) {
-			break
-		}
-		delete(h.packetMap, p.PacketNumber)
-		h.etcPacketList.Remove(el)
-	}
-}
-
-func (h *sentPacketHistory) DeclareLost(p *Packet) *Packet {
-	el, ok := h.packetMap[p.PacketNumber]
 	if !ok {
-		return nil
+		return fmt.Errorf("packet %d not found in sent packet history", pn)
 	}
-	el.List().Remove(el)
+	p := h.packets[idx]
-	p.declaredLost = true
+	if p.outstanding() {
-	// move it to the correct position in the etc list (based on the packet number)
+		h.numOutstanding--
-	for el = h.etcPacketList.Back(); el != nil; el = el.Prev() {
+		if h.numOutstanding < 0 {
-		if el.Value.PacketNumber < p.PacketNumber {
+			panic("negative number of outstanding packets")
-			break
 		}
 	}
-	if el == nil {
+	h.packets[idx] = nil
-		el = h.etcPacketList.PushFront(p)
+	// clean up all skipped packets directly before this packet number
-	} else {
+	for idx > 0 {
-		el = h.etcPacketList.InsertAfter(p, el)
+		idx--
+		p := h.packets[idx]
+		if p == nil || !p.skippedPacket {
+			break
+		}
+		h.packets[idx] = nil
+	}
+	if idx == 0 {
+		h.cleanupStart()
+	}
+	if len(h.packets) > 0 && h.packets[0] == nil {
+		panic("remove failed")
+	}
+	return nil
+}
+
+// getIndex gets the index of packet p in the packets slice.
+func (h *sentPacketHistory) getIndex(p protocol.PacketNumber) (int, bool) {
+	if len(h.packets) == 0 {
+		return 0, false
+	}
+	first := h.packets[0].PacketNumber
+	if p < first {
+		return 0, false
+	}
+	index := int(p - first)
+	if index > len(h.packets)-1 {
+		return 0, false
+	}
+	return index, true
+}
+
+func (h *sentPacketHistory) HasOutstandingPackets() bool {
+	return h.numOutstanding > 0
+}
+
+// delete all nil entries at the beginning of the packets slice
+func (h *sentPacketHistory) cleanupStart() {
+	for i, p := range h.packets {
+		if p != nil {
+			h.packets = h.packets[i:]
+			return
+		}
+	}
+	h.packets = h.packets[:0]
+}
+
+func (h *sentPacketHistory) LowestPacketNumber() protocol.PacketNumber {
+	if len(h.packets) == 0 {
+		return protocol.InvalidPacketNumber
+	}
+	return h.packets[0].PacketNumber
+}
+
+func (h *sentPacketHistory) DeclareLost(pn protocol.PacketNumber) {
+	idx, ok := h.getIndex(pn)
+	if !ok {
+		return
+	}
+	p := h.packets[idx]
+	if p.outstanding() {
+		h.numOutstanding--
+		if h.numOutstanding < 0 {
+			panic("negative number of outstanding packets")
+		}
+	}
+	h.packets[idx] = nil
+	if idx == 0 {
+		h.cleanupStart()
 	}
-	h.packetMap[p.PacketNumber] = el
-	return el.Value
 }

internal/ackhandler/sent_packet_history_test.go

@@ -5,58 +5,69 @@ import (
 	"time"
 
 	"github.com/quic-go/quic-go/internal/protocol"
-	"github.com/quic-go/quic-go/internal/utils"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("SentPacketHistory", func() {
-	var (
+	var hist *sentPacketHistory
-		hist     *sentPacketHistory
-		rttStats *utils.RTTStats
-	)
 
-	expectInHistory := func(packetNumbers []protocol.PacketNumber) {
+	expectInHistory := func(expected []protocol.PacketNumber) {
-		var mapLen int
+		pns := make([]protocol.PacketNumber, 0, len(expected))
-		for _, el := range hist.packetMap {
+		for _, p := range hist.packets {
-			if !el.Value.skippedPacket {
+			if p != nil && !p.skippedPacket {
-				mapLen++
+				pns = append(pns, p.PacketNumber)
 			}
 		}
-		var listLen int
+		if len(expected) == 0 {
-		hist.Iterate(func(p *Packet) (bool, error) {
+			Expect(pns).To(BeEmpty())
-			if !p.skippedPacket {
+			return
-				listLen++
+		}
+		Expect(pns).To(Equal(expected))
+	}
+
+	expectSkippedInHistory := func(expected []protocol.PacketNumber) {
+		pns := make([]protocol.PacketNumber, 0, len(expected))
+		for _, p := range hist.packets {
+			if p != nil && p.skippedPacket {
+				pns = append(pns, p.PacketNumber)
 			}
-			return true, nil
+		}
-		})
+		if len(expected) == 0 {
-		ExpectWithOffset(1, mapLen).To(Equal(len(packetNumbers)))
+			Expect(pns).To(BeEmpty())
-		ExpectWithOffset(1, listLen).To(Equal(len(packetNumbers)))
+			return
-		i := 0
+		}
-		err := hist.Iterate(func(p *Packet) (bool, error) {
+		Expect(pns).To(Equal(expected))
-			if p.skippedPacket {
-				return true, nil
-			}
-			pn := packetNumbers[i]
-			ExpectWithOffset(1, p.PacketNumber).To(Equal(pn))
-			ExpectWithOffset(1, hist.packetMap[pn].Value.PacketNumber).To(Equal(pn))
-			i++
-			return true, nil
-		})
-		Expect(err).ToNot(HaveOccurred())
 	}
 
 	BeforeEach(func() {
-		rttStats = utils.NewRTTStats()
+		hist = newSentPacketHistory()
-		hist = newSentPacketHistory(rttStats)
 	})
 
 	It("saves sent packets", func() {
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 0})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 2})
+		expectInHistory([]protocol.PacketNumber{0, 1, 2})
+		expectSkippedInHistory(nil)
+	})
+
+	It("saves non-ack-eliciting packets", func() {
+		now := time.Now()
+		hist.SentNonAckElicitingPacket(0, protocol.Encryption1RTT, now)
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 1, SendTime: now})
+		hist.SentNonAckElicitingPacket(2, protocol.Encryption1RTT, now)
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 3, SendTime: now})
+		expectInHistory([]protocol.PacketNumber{1, 3})
+	})
+
+	It("saves sent packets, with skipped packet number", func() {
 		hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
 		hist.SentAckElicitingPacket(&Packet{PacketNumber: 3})
 		hist.SentAckElicitingPacket(&Packet{PacketNumber: 4})
 		expectInHistory([]protocol.PacketNumber{1, 3, 4})
+		expectSkippedInHistory([]protocol.PacketNumber{0, 2})
 	})
 
 	It("doesn't save non-ack-eliciting packets", func() {
@@ -64,10 +75,6 @@ var _ = Describe("SentPacketHistory", func() {
 		hist.SentNonAckElicitingPacket(3, protocol.EncryptionLevel(0), time.Time{})
 		hist.SentAckElicitingPacket(&Packet{PacketNumber: 4})
 		expectInHistory([]protocol.PacketNumber{1, 4})
-		hist.Iterate(func(p *Packet) (bool, error) {
-			Expect(p.PacketNumber).ToNot(Equal(protocol.PacketNumber(3)))
-			return true, nil
-		})
 	})
 
 	It("gets the length", func() {
@@ -88,6 +95,10 @@ var _ = Describe("SentPacketHistory", func() {
 			front := hist.FirstOutstanding()
 			Expect(front).ToNot(BeNil())
 			Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(2)))
+			hist.Remove(2)
+			front = hist.FirstOutstanding()
+			Expect(front).ToNot(BeNil())
+			Expect(front.PacketNumber).To(Equal(protocol.PacketNumber(3)))
 		})
 
 		It("doesn't regard path MTU packets as outstanding", func() {
@@ -100,18 +111,52 @@ var _ = Describe("SentPacketHistory", func() {
 	})
 
 	It("removes packets", func() {
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 0})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 2})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 3})
+		Expect(hist.Remove(2)).To(Succeed())
+		expectInHistory([]protocol.PacketNumber{0, 1, 3})
+	})
+
+	It("also remove skipped packets before the removed packet", func() {
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 4})
+		expectSkippedInHistory([]protocol.PacketNumber{0, 2, 3})
+		Expect(hist.Remove(4)).To(Succeed())
+		expectSkippedInHistory([]protocol.PacketNumber{0})
+		expectInHistory([]protocol.PacketNumber{1})
+		Expect(hist.Remove(1)).To(Succeed())
+		expectInHistory(nil)
+		expectSkippedInHistory(nil)
+	})
+
+	It("removes and adds packets", func() {
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 0})
 		hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
 		hist.SentAckElicitingPacket(&Packet{PacketNumber: 4})
 		hist.SentAckElicitingPacket(&Packet{PacketNumber: 8})
-		err := hist.Remove(4)
+		Expect(hist.Remove(0)).To(Succeed())
-		Expect(err).ToNot(HaveOccurred())
+		Expect(hist.Remove(1)).To(Succeed())
-		expectInHistory([]protocol.PacketNumber{1, 8})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 9})
+		expectInHistory([]protocol.PacketNumber{4, 8, 9})
+	})
+
+	It("removes the last packet, then adds more", func() {
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 0})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
+		Expect(hist.Remove(0)).To(Succeed())
+		Expect(hist.Remove(1)).To(Succeed())
+		expectInHistory([]protocol.PacketNumber{})
+		hist.SentAckElicitingPacket(&Packet{PacketNumber: 2})
+		expectInHistory([]protocol.PacketNumber{2})
+		Expect(hist.Remove(2)).To(Succeed())
+		expectInHistory(nil)
 	})
 
 	It("errors when trying to remove a non existing packet", func() {
 		hist.SentAckElicitingPacket(&Packet{PacketNumber: 1})
-		err := hist.Remove(2)
+		Expect(hist.Remove(2)).To(MatchError("packet 2 not found in sent packet history"))
-		Expect(err).To(MatchError("packet 2 not found in sent packet history"))
 	})
 
 	Context("iterating", func() {
@@ -177,6 +222,22 @@ var _ = Describe("SentPacketHistory", func() {
 			Expect(iterations).To(Equal([]protocol.PacketNumber{1, 4}))
 		})
 
+		It("doesn't iterate over deleted packets", func() {
+			hist.Remove(4)
+			var iterations []protocol.PacketNumber
+			Expect(hist.Iterate(func(p *Packet) (bool, error) {
+				if p.skippedPacket {
+					return true, nil
+				}
+				iterations = append(iterations, p.PacketNumber)
+				if p.PacketNumber == 4 {
+					Expect(hist.Remove(4)).To(Succeed())
+				}
+				return true, nil
+			})).To(Succeed())
+			Expect(iterations).To(Equal([]protocol.PacketNumber{1, 8}))
+		})
+
 		It("allows deletions", func() {
 			var iterations []protocol.PacketNumber
 			Expect(hist.Iterate(func(p *Packet) (bool, error) {
@@ -197,7 +258,7 @@ var _ = Describe("SentPacketHistory", func() {
 	Context("outstanding packets", func() {
 		It("says if it has outstanding packets", func() {
 			Expect(hist.HasOutstandingPackets()).To(BeFalse())
-			hist.SentAckElicitingPacket(&Packet{EncryptionLevel: protocol.Encryption1RTT})
+			hist.SentAckElicitingPacket(&Packet{EncryptionLevel: protocol.Encryption1RTT, PacketNumber: 0})
 			Expect(hist.HasOutstandingPackets()).To(BeTrue())
 		})
 
@@ -226,42 +287,4 @@ var _ = Describe("SentPacketHistory", func() {
 			Expect(hist.HasOutstandingPackets()).To(BeFalse())
 		})
 	})
-
-	Context("deleting old packets", func() {
-		const pto = 3 * time.Second
-
-		BeforeEach(func() {
-			rttStats.UpdateRTT(time.Second, 0, time.Time{})
-			Expect(rttStats.PTO(false)).To(Equal(pto))
-		})
-
-		It("deletes old packets after 3 PTOs", func() {
-			now := time.Now()
-			hist.SentAckElicitingPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto), declaredLost: true})
-			expectInHistory([]protocol.PacketNumber{10})
-			hist.DeleteOldPackets(now.Add(-time.Nanosecond))
-			expectInHistory([]protocol.PacketNumber{10})
-			hist.DeleteOldPackets(now)
-			expectInHistory([]protocol.PacketNumber{})
-		})
-
-		It("doesn't delete a packet if it hasn't been declared lost yet", func() {
-			now := time.Now()
-			hist.SentAckElicitingPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto), declaredLost: true})
-			hist.SentAckElicitingPacket(&Packet{PacketNumber: 11, SendTime: now.Add(-3 * pto), declaredLost: false})
-			expectInHistory([]protocol.PacketNumber{10, 11})
-			hist.DeleteOldPackets(now)
-			expectInHistory([]protocol.PacketNumber{11})
-		})
-
-		It("deletes skipped packets", func() {
-			now := time.Now()
-			hist.SentAckElicitingPacket(&Packet{PacketNumber: 10, SendTime: now.Add(-3 * pto)})
-			expectInHistory([]protocol.PacketNumber{10})
-			Expect(hist.Len()).To(Equal(11))
-			hist.DeleteOldPackets(now)
-			expectInHistory([]protocol.PacketNumber{10}) // the packet was not declared lost
-			Expect(hist.Len()).To(Equal(1))
-		})
-	})
 })