remove ACK decimation

The benefits of this are unclear when using Reno / Cubic.

integrationtests/self/timeout_test.go

@@ -214,24 +214,38 @@ var _ = Describe("Timeout tests", func() {
 			Expect(err).ToNot(HaveOccurred())
 			defer server.Close()
 
+			drop := utils.AtomicBool{}
+			proxy, err := quicproxy.NewQuicProxy("localhost:0", &quicproxy.Opts{
+				RemoteAddr: fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port),
+				DropPacket: func(dir quicproxy.Direction, _ []byte) bool {
+					if dir == quicproxy.DirectionOutgoing {
+						return drop.Get()
+					}
+					return false
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+			defer proxy.Close()
+
 			serverSessionClosed := make(chan struct{})
 			go func() {
 				defer GinkgoRecover()
 				sess, err := server.Accept(context.Background())
 				Expect(err).ToNot(HaveOccurred())
-				sess.AcceptStream(context.Background()) // blocks until the session is closed
+				<-sess.Context().Done() // block until the session is closed
 				close(serverSessionClosed)
 			}()
 
 			sess, err := quic.DialAddr(
-				fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port),
+				fmt.Sprintf("localhost:%d", proxy.LocalPort()),
 				getTLSClientConfig(),
 				getQuicConfig(&quic.Config{MaxIdleTimeout: idleTimeout}),
 			)
 			Expect(err).ToNot(HaveOccurred())
 
 			// wait half the idle timeout, then send a packet
 			time.Sleep(idleTimeout / 2)
+			drop.Set(true)
 			str, err := sess.OpenUniStream()
 			Expect(err).ToNot(HaveOccurred())
 			_, err = str.Write([]byte("foobar"))
@@ -281,7 +295,6 @@ var _ = Describe("Timeout tests", func() {
 		}()
 
 		drop := utils.AtomicBool{}
-
 		proxy, err := quicproxy.NewQuicProxy("localhost:0", &quicproxy.Opts{
 			RemoteAddr: fmt.Sprintf("localhost:%d", server.Addr().(*net.UDPAddr).Port),
 			DropPacket: func(quicproxy.Direction, []byte) bool {

internal/ackhandler/received_packet_handler.go

@@ -9,27 +9,6 @@ import (
 	"github.com/lucas-clemente/quic-go/internal/wire"
 )
 
-const (
-	// initial maximum number of ack-eliciting packets received before sending an ack.
-	initialAckElicitingPacketsBeforeAck = 2
-	// number of ack-eliciting that an ACK is sent for
-	ackElicitingPacketsBeforeAck = 10
-	// 1/5 RTT delay when doing ack decimation
-	ackDecimationDelay = 1.0 / 4
-	// 1/8 RTT delay when doing ack decimation
-	shortAckDecimationDelay = 1.0 / 8
-	// Minimum number of packets received before ack decimation is enabled.
-	// This intends to avoid the beginning of slow start, when CWNDs may be
-	// rapidly increasing.
-	minReceivedBeforeAckDecimation = 100
-	// Maximum number of packets to ack immediately after a missing packet for
-	// fast retransmission to kick in at the sender.  This limit is created to
-	// reduce the number of acks sent that have no benefit for fast retransmission.
-	// Set to the number of nacks needed for fast retransmit plus one for protection
-	// against an ack loss
-	maxPacketsAfterNewMissing = 4
-)
-
 type receivedPacketHandler struct {
 	sentPackets sentPacketTracker
 

internal/ackhandler/received_packet_tracker.go

@@ -8,6 +8,9 @@ import (
 	"github.com/lucas-clemente/quic-go/internal/wire"
 )
 
+// number of ack-eliciting packets received before sending an ack.
+const packetsBeforeAck = 2
+
 type receivedPacketTracker struct {
 	largestObserved             protocol.PacketNumber
 	ignoreBelow                 protocol.PacketNumber
@@ -61,7 +64,7 @@ func (h *receivedPacketTracker) ReceivedPacket(packetNumber protocol.PacketNumbe
 	h.maybeQueueAck(packetNumber, rcvTime, shouldInstigateAck, isMissing)
 }
 
-// IgnoreBelow sets a lower limit for acking packets.
+// IgnoreBelow sets a lower limit for acknowledging packets.
 // Packets with packet numbers smaller than p will not be acked.
 func (h *receivedPacketTracker) IgnoreBelow(p protocol.PacketNumber) {
 	if p <= h.ignoreBelow {
@@ -87,12 +90,10 @@ func (h *receivedPacketTracker) hasNewMissingPackets() bool {
 		return false
 	}
 	highestRange := h.packetHistory.GetHighestAckRange()
-	return highestRange.Smallest >= h.lastAck.LargestAcked() && highestRange.Len() <= maxPacketsAfterNewMissing
+	return highestRange.Smallest >= h.lastAck.LargestAcked() && highestRange.Len() == 1
 }
 
 // maybeQueueAck queues an ACK, if necessary.
-// It is implemented analogously to Chrome's QuicConnection::MaybeQueueAck()
-// in ACK_DECIMATION_WITH_REORDERING mode.
 func (h *receivedPacketTracker) maybeQueueAck(packetNumber protocol.PacketNumber, rcvTime time.Time, shouldInstigateAck, wasMissing bool) {
 	// always ack the first packet
 	if h.lastAck == nil {
@@ -116,46 +117,22 @@ func (h *receivedPacketTracker) maybeQueueAck(packetNumber protocol.PacketNumber
 	if !h.ackQueued && shouldInstigateAck {
 		h.ackElicitingPacketsReceivedSinceLastAck++
 
-		if packetNumber > minReceivedBeforeAckDecimation {
-			// ack up to 10 packets at once
-			if h.ackElicitingPacketsReceivedSinceLastAck >= ackElicitingPacketsBeforeAck {
-				h.ackQueued = true
-				if h.logger.Debug() {
-					h.logger.Debugf("\tQueueing ACK because packet %d packets were received after the last ACK (using threshold: %d).", h.ackElicitingPacketsReceivedSinceLastAck, ackElicitingPacketsBeforeAck)
-				}
-			} else if h.ackAlarm.IsZero() {
-				// wait for the minimum of the ack decimation delay or the delayed ack time before sending an ack
-				ackDelay := utils.MinDuration(h.maxAckDelay, time.Duration(float64(h.rttStats.MinRTT())*float64(ackDecimationDelay)))
-				h.ackAlarm = rcvTime.Add(ackDelay)
-				if h.logger.Debug() {
-					h.logger.Debugf("\tSetting ACK timer to min(1/4 min-RTT, max ack delay): %s (%s from now)", ackDelay, time.Until(h.ackAlarm))
-				}
+		// send an ACK every 2 ack-eliciting packets
+		if h.ackElicitingPacketsReceivedSinceLastAck >= packetsBeforeAck {
+			if h.logger.Debug() {
+				h.logger.Debugf("\tQueueing ACK because packet %d packets were received after the last ACK (using initial threshold: %d).", h.ackElicitingPacketsReceivedSinceLastAck, packetsBeforeAck)
 			}
-		} else {
-			// send an ACK every 2 ack-eliciting packets
-			if h.ackElicitingPacketsReceivedSinceLastAck >= initialAckElicitingPacketsBeforeAck {
-				if h.logger.Debug() {
-					h.logger.Debugf("\tQueueing ACK because packet %d packets were received after the last ACK (using initial threshold: %d).", h.ackElicitingPacketsReceivedSinceLastAck, initialAckElicitingPacketsBeforeAck)
-				}
-				h.ackQueued = true
-			} else if h.ackAlarm.IsZero() {
-				if h.logger.Debug() {
-					h.logger.Debugf("\tSetting ACK timer to max ack delay: %s", h.maxAckDelay)
-				}
-				h.ackAlarm = rcvTime.Add(h.maxAckDelay)
+			h.ackQueued = true
+		} else if h.ackAlarm.IsZero() {
+			if h.logger.Debug() {
+				h.logger.Debugf("\tSetting ACK timer to max ack delay: %s", h.maxAckDelay)
 			}
+			h.ackAlarm = rcvTime.Add(h.maxAckDelay)
 		}
-		// If there are new missing packets to report, set a short timer to send an ACK.
+
+		// Queue an ACK if there are new missing packets to report.
 		if h.hasNewMissingPackets() {
-			// wait the minimum of 1/8 min RTT and the existing ack time
-			ackDelay := time.Duration(float64(h.rttStats.MinRTT()) * float64(shortAckDecimationDelay))
-			ackTime := rcvTime.Add(ackDelay)
-			if h.ackAlarm.IsZero() || h.ackAlarm.After(ackTime) {
-				h.ackAlarm = ackTime
-				if h.logger.Debug() {
-					h.logger.Debugf("\tSetting ACK timer to 1/8 min-RTT: %s (%s from now)", ackDelay, time.Until(h.ackAlarm))
-				}
-			}
+			h.ackQueued = true
 		}
 	}