From 5dbb46dcc19dd60d680fad69d652173b408e2f12 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 22 Dec 2024 14:50:31 +0800 Subject: [PATCH] utils: remove unused now parameter from RTTStats.UpdateRTT (#4780) --- internal/ackhandler/sent_packet_handler.go | 4 +- .../ackhandler/sent_packet_handler_test.go | 12 ++-- internal/congestion/cubic_sender_test.go | 4 +- .../connection_flow_controller_test.go | 2 +- .../stream_flow_controller_test.go | 2 +- internal/handshake/crypto_setup_test.go | 2 +- internal/handshake/updatable_aead_test.go | 6 +- internal/utils/rtt_stats.go | 2 +- internal/utils/rtt_stats_test.go | 62 +++++++++---------- qlog/connection_tracer_test.go | 20 +++--- 10 files changed, 57 insertions(+), 59 deletions(-) diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go index b84f0dcbb..8f44d9883 100644 --- a/internal/ackhandler/sent_packet_handler.go +++ b/internal/ackhandler/sent_packet_handler.go @@ -338,7 +338,7 @@ func (h *sentPacketHandler) ReceivedAck(ack *wire.AckFrame, encLevel protocol.En if encLevel == protocol.Encryption1RTT { ackDelay = min(ack.DelayTime, h.rttStats.MaxAckDelay()) } - h.rttStats.UpdateRTT(rcvTime.Sub(p.SendTime), ackDelay, rcvTime) + h.rttStats.UpdateRTT(rcvTime.Sub(p.SendTime), ackDelay) if h.logger.Debug() { h.logger.Debugf("\tupdated RTT: %s (σ: %s)", h.rttStats.SmoothedRTT(), h.rttStats.MeanDeviation()) } @@ -890,7 +890,7 @@ func (h *sentPacketHandler) ResetForRetry(now time.Time) error { // Otherwise, we don't know which Initial the Retry was sent in response to. if h.ptoCount == 0 { // Don't set the RTT to a value lower than 5ms here. - h.rttStats.UpdateRTT(max(minRTTAfterRetry, now.Sub(firstPacketSendTime)), 0, now) + h.rttStats.UpdateRTT(max(minRTTAfterRetry, now.Sub(firstPacketSendTime)), 0) if h.logger.Debug() { h.logger.Debugf("\tupdated RTT: %s (σ: %s)", h.rttStats.SmoothedRTT(), h.rttStats.MeanDeviation()) } diff --git a/internal/ackhandler/sent_packet_handler_test.go b/internal/ackhandler/sent_packet_handler_test.go index ea07b5fdc..1b8472340 100644 --- a/internal/ackhandler/sent_packet_handler_test.go +++ b/internal/ackhandler/sent_packet_handler_test.go @@ -125,7 +125,7 @@ var _ = Describe("SentPacketHandler", func() { } updateRTT := func(rtt time.Duration) { - handler.rttStats.UpdateRTT(rtt, 0, time.Now()) + handler.rttStats.UpdateRTT(rtt, 0) ExpectWithOffset(1, handler.rttStats.SmoothedRTT()).To(Equal(rtt)) } @@ -416,7 +416,7 @@ var _ = Describe("SentPacketHandler", func() { sentPacket(initialPacket(&packet{PacketNumber: 1})) handler.rttStats.SetMaxAckDelay(time.Hour) // make sure the rttStats have a min RTT, so that the delay is used - handler.rttStats.UpdateRTT(5*time.Minute, 0, time.Now()) + handler.rttStats.UpdateRTT(5*time.Minute, 0) getPacket(1, protocol.EncryptionInitial).SendTime = time.Now().Add(-10 * time.Minute) ack := &wire.AckFrame{ AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}, @@ -430,7 +430,7 @@ var _ = Describe("SentPacketHandler", func() { It("uses the DelayTime in the ACK frame", func() { handler.rttStats.SetMaxAckDelay(time.Hour) // make sure the rttStats have a min RTT, so that the delay is used - handler.rttStats.UpdateRTT(5*time.Minute, 0, time.Now()) + handler.rttStats.UpdateRTT(5*time.Minute, 0) getPacket(1, protocol.Encryption1RTT).SendTime = time.Now().Add(-10 * time.Minute) ack := &wire.AckFrame{ AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}, @@ -444,7 +444,7 @@ var _ = Describe("SentPacketHandler", func() { It("limits the DelayTime in the ACK frame to max_ack_delay", func() { handler.rttStats.SetMaxAckDelay(time.Minute) // make sure the rttStats have a min RTT, so that the delay is used - handler.rttStats.UpdateRTT(5*time.Minute, 0, time.Now()) + handler.rttStats.UpdateRTT(5*time.Minute, 0) getPacket(1, protocol.Encryption1RTT).SendTime = time.Now().Add(-10 * time.Minute) ack := &wire.AckFrame{ AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}, @@ -752,7 +752,7 @@ var _ = Describe("SentPacketHandler", func() { handler.ReceivedPacket(protocol.EncryptionHandshake) now := time.Now() - handler.rttStats.UpdateRTT(time.Second/2, 0, now) + handler.rttStats.UpdateRTT(time.Second/2, 0) Expect(handler.rttStats.SmoothedRTT()).To(Equal(time.Second / 2)) Expect(handler.rttStats.PTO(true)).To(And( BeNumerically(">", time.Second), @@ -1442,7 +1442,7 @@ var _ = Describe("SentPacketHandler", func() { ecnHandler = NewMockECNHandler(mockCtrl) lostPackets = nil var rttStats utils.RTTStats - rttStats.UpdateRTT(time.Hour, 0, time.Now()) + rttStats.UpdateRTT(time.Hour, 0) handler = newSentPacketHandler(42, protocol.InitialPacketSize, &rttStats, false, false, perspective, nil, utils.DefaultLogger) handler.ecnTracker = ecnHandler handler.congestion = cong diff --git a/internal/congestion/cubic_sender_test.go b/internal/congestion/cubic_sender_test.go index d72940312..08655fbf3 100644 --- a/internal/congestion/cubic_sender_test.go +++ b/internal/congestion/cubic_sender_test.go @@ -67,7 +67,7 @@ var _ = Describe("Cubic Sender", func() { // Normal is that TCP acks every other segment. AckNPackets := func(n int) { - rttStats.UpdateRTT(60*time.Millisecond, 0, clock.Now()) + rttStats.UpdateRTT(60*time.Millisecond, 0) sender.MaybeExitSlowStart() for i := 0; i < n; i++ { ackedPacketNumber++ @@ -109,7 +109,7 @@ var _ = Describe("Cubic Sender", func() { }) It("paces", func() { - rttStats.UpdateRTT(10*time.Millisecond, 0, time.Now()) + rttStats.UpdateRTT(10*time.Millisecond, 0) clock.Advance(time.Hour) // Fill the send window with data, then verify that we can't send. SendAvailableSendWindow() diff --git a/internal/flowcontrol/connection_flow_controller_test.go b/internal/flowcontrol/connection_flow_controller_test.go index 1e1288b38..de54ca282 100644 --- a/internal/flowcontrol/connection_flow_controller_test.go +++ b/internal/flowcontrol/connection_flow_controller_test.go @@ -27,7 +27,7 @@ func TestConnectionFlowControlWindowUpdate(t *testing.T) { func TestConnectionWindowAutoTuningNotAllowed(t *testing.T) { // the RTT is 1 second rttStats := &utils.RTTStats{} - rttStats.UpdateRTT(time.Second, 0, time.Now()) + rttStats.UpdateRTT(time.Second, 0) require.Equal(t, time.Second, rttStats.SmoothedRTT()) callbackCalledWith := protocol.InvalidByteCount diff --git a/internal/flowcontrol/stream_flow_controller_test.go b/internal/flowcontrol/stream_flow_controller_test.go index da3e78df0..34bc80842 100644 --- a/internal/flowcontrol/stream_flow_controller_test.go +++ b/internal/flowcontrol/stream_flow_controller_test.go @@ -214,7 +214,7 @@ func TestStreamWindowUpdate(t *testing.T) { func TestStreamWindowAutoTuning(t *testing.T) { // the RTT is 1 second rttStats := &utils.RTTStats{} - rttStats.UpdateRTT(time.Second, 0, time.Now()) + rttStats.UpdateRTT(time.Second, 0) require.Equal(t, time.Second, rttStats.SmoothedRTT()) connFC := NewConnectionFlowController( diff --git a/internal/handshake/crypto_setup_test.go b/internal/handshake/crypto_setup_test.go index 64525cb46..391392a17 100644 --- a/internal/handshake/crypto_setup_test.go +++ b/internal/handshake/crypto_setup_test.go @@ -110,7 +110,7 @@ func TestMessageReceivedAtWrongEncryptionLevel(t *testing.T) { func newRTTStatsWithRTT(t *testing.T, rtt time.Duration) *utils.RTTStats { t.Helper() rttStats := &utils.RTTStats{} - rttStats.UpdateRTT(rtt, 0, time.Now()) + rttStats.UpdateRTT(rtt, 0) require.Equal(t, rtt, rttStats.SmoothedRTT()) return rttStats } diff --git a/internal/handshake/updatable_aead_test.go b/internal/handshake/updatable_aead_test.go index b95ae5a36..2b4cefa9d 100644 --- a/internal/handshake/updatable_aead_test.go +++ b/internal/handshake/updatable_aead_test.go @@ -274,7 +274,7 @@ func TestDropsKeys3PTOsAfterKeyUpdate(t *testing.T) { client, server, serverTracer := setupEndpoints(t, &rttStats) now := time.Now() - rttStats.UpdateRTT(10*time.Millisecond, 0, now) + rttStats.UpdateRTT(10*time.Millisecond, 0) pto := rttStats.PTO(true) encrypted01 := client.Seal(nil, []byte(msg), 0x42, []byte(ad)) encrypted02 := client.Seal(nil, []byte(msg), 0x43, []byte(ad)) @@ -466,7 +466,7 @@ func TestKeyUpdateKeyPhaseSkipping(t *testing.T) { setKeyUpdateIntervals(t, firstKeyUpdateInterval, keyUpdateInterval) var rttStats utils.RTTStats - rttStats.UpdateRTT(10*time.Millisecond, 0, time.Now()) + rttStats.UpdateRTT(10*time.Millisecond, 0) client, server, serverTracer := setupEndpoints(t, &rttStats) server.SetHandshakeConfirmed() @@ -538,7 +538,7 @@ func TestFastKeyUpdateByUs(t *testing.T) { setKeyUpdateIntervals(t, firstKeyUpdateInterval, keyUpdateInterval) var rttStats utils.RTTStats - rttStats.UpdateRTT(10*time.Millisecond, 0, time.Now()) + rttStats.UpdateRTT(10*time.Millisecond, 0) client, server, serverTracer := setupEndpoints(t, &rttStats) server.SetHandshakeConfirmed() diff --git a/internal/utils/rtt_stats.go b/internal/utils/rtt_stats.go index dcfac67d5..92fec2e28 100644 --- a/internal/utils/rtt_stats.go +++ b/internal/utils/rtt_stats.go @@ -58,7 +58,7 @@ func (r *RTTStats) PTO(includeMaxAckDelay bool) time.Duration { } // UpdateRTT updates the RTT based on a new sample. -func (r *RTTStats) UpdateRTT(sendDelta, ackDelay time.Duration, now time.Time) { +func (r *RTTStats) UpdateRTT(sendDelta, ackDelay time.Duration) { if sendDelta <= 0 { return } diff --git a/internal/utils/rtt_stats_test.go b/internal/utils/rtt_stats_test.go index e5d77c7b2..b0babc93f 100644 --- a/internal/utils/rtt_stats_test.go +++ b/internal/utils/rtt_stats_test.go @@ -16,35 +16,35 @@ func TestRTTStatsDefaults(t *testing.T) { func TestRTTStatsSmoothedRTT(t *testing.T) { var rttStats RTTStats - // Verify that ack_delay is ignored in the first measurement. - rttStats.UpdateRTT((300 * time.Millisecond), (100 * time.Millisecond), time.Time{}) - require.Equal(t, (300 * time.Millisecond), rttStats.LatestRTT()) - require.Equal(t, (300 * time.Millisecond), rttStats.SmoothedRTT()) - // Verify that Smoothed RTT includes max ack delay if it's reasonable. - rttStats.UpdateRTT((350 * time.Millisecond), (50 * time.Millisecond), time.Time{}) - require.Equal(t, (300 * time.Millisecond), rttStats.LatestRTT()) - require.Equal(t, (300 * time.Millisecond), rttStats.SmoothedRTT()) - // Verify that large erroneous ack_delay does not change Smoothed RTT. - rttStats.UpdateRTT((200 * time.Millisecond), (300 * time.Millisecond), time.Time{}) - require.Equal(t, (200 * time.Millisecond), rttStats.LatestRTT()) - require.Equal(t, (287500 * time.Microsecond), rttStats.SmoothedRTT()) + // verify that ack_delay is ignored in the first measurement + rttStats.UpdateRTT(300*time.Millisecond, 100*time.Millisecond) + require.Equal(t, 300*time.Millisecond, rttStats.LatestRTT()) + require.Equal(t, 300*time.Millisecond, rttStats.SmoothedRTT()) + // verify that smoothed RTT includes max ack delay if it's reasonable + rttStats.UpdateRTT(350*time.Millisecond, 50*time.Millisecond) + require.Equal(t, 300*time.Millisecond, rttStats.LatestRTT()) + require.Equal(t, 300*time.Millisecond, rttStats.SmoothedRTT()) + // verify that large erroneous ack_delay does not change smoothed RTT + rttStats.UpdateRTT(200*time.Millisecond, 300*time.Millisecond) + require.Equal(t, 200*time.Millisecond, rttStats.LatestRTT()) + require.Equal(t, 287500*time.Microsecond, rttStats.SmoothedRTT()) } func TestRTTStatsMinRTT(t *testing.T) { var rttStats RTTStats - rttStats.UpdateRTT((200 * time.Millisecond), 0, time.Time{}) - require.Equal(t, (200 * time.Millisecond), rttStats.MinRTT()) - rttStats.UpdateRTT((10 * time.Millisecond), 0, time.Time{}.Add((10 * time.Millisecond))) - require.Equal(t, (10 * time.Millisecond), rttStats.MinRTT()) - rttStats.UpdateRTT((50 * time.Millisecond), 0, time.Time{}.Add((20 * time.Millisecond))) - require.Equal(t, (10 * time.Millisecond), rttStats.MinRTT()) - rttStats.UpdateRTT((50 * time.Millisecond), 0, time.Time{}.Add((30 * time.Millisecond))) - require.Equal(t, (10 * time.Millisecond), rttStats.MinRTT()) - rttStats.UpdateRTT((50 * time.Millisecond), 0, time.Time{}.Add((40 * time.Millisecond))) - require.Equal(t, (10 * time.Millisecond), rttStats.MinRTT()) - // Verify that ack_delay does not go into recording of MinRTT_. - rttStats.UpdateRTT((7 * time.Millisecond), (2 * time.Millisecond), time.Time{}.Add((50 * time.Millisecond))) - require.Equal(t, (7 * time.Millisecond), rttStats.MinRTT()) + rttStats.UpdateRTT(200*time.Millisecond, 0) + require.Equal(t, 200*time.Millisecond, rttStats.MinRTT()) + rttStats.UpdateRTT(10*time.Millisecond, 0) + require.Equal(t, 10*time.Millisecond, rttStats.MinRTT()) + rttStats.UpdateRTT(50*time.Millisecond, 0) + require.Equal(t, 10*time.Millisecond, rttStats.MinRTT()) + rttStats.UpdateRTT(50*time.Millisecond, 0) + require.Equal(t, 10*time.Millisecond, rttStats.MinRTT()) + rttStats.UpdateRTT(50*time.Millisecond, 0) + require.Equal(t, 10*time.Millisecond, rttStats.MinRTT()) + // verify that ack_delay does not go into recording of MinRTT + rttStats.UpdateRTT(7*time.Millisecond, 2*time.Millisecond) + require.Equal(t, 7*time.Millisecond, rttStats.MinRTT()) } func TestRTTStatsMaxAckDelay(t *testing.T) { @@ -60,7 +60,7 @@ func TestRTTStatsComputePTO(t *testing.T) { ) var rttStats RTTStats rttStats.SetMaxAckDelay(maxAckDelay) - rttStats.UpdateRTT(rtt, 0, time.Time{}) + rttStats.UpdateRTT(rtt, 0) require.Equal(t, rtt, rttStats.SmoothedRTT()) require.Equal(t, rtt/2, rttStats.MeanDeviation()) require.Equal(t, rtt+4*(rtt/2), rttStats.PTO(false)) @@ -70,14 +70,14 @@ func TestRTTStatsComputePTO(t *testing.T) { func TestRTTStatsPTOWithShortRTT(t *testing.T) { const rtt = time.Microsecond var rttStats RTTStats - rttStats.UpdateRTT(rtt, 0, time.Time{}) + rttStats.UpdateRTT(rtt, 0) require.Equal(t, rtt+protocol.TimerGranularity, rttStats.PTO(true)) } func TestRTTStatsUpdateWithBadSendDeltas(t *testing.T) { var rttStats RTTStats const initialRtt = 10 * time.Millisecond - rttStats.UpdateRTT(initialRtt, 0, time.Time{}) + rttStats.UpdateRTT(initialRtt, 0) require.Equal(t, initialRtt, rttStats.MinRTT()) require.Equal(t, initialRtt, rttStats.SmoothedRTT()) @@ -87,7 +87,7 @@ func TestRTTStatsUpdateWithBadSendDeltas(t *testing.T) { } for _, badSendDelta := range badSendDeltas { - rttStats.UpdateRTT(badSendDelta, 0, time.Time{}) + rttStats.UpdateRTT(badSendDelta, 0) require.Equal(t, initialRtt, rttStats.MinRTT()) require.Equal(t, initialRtt, rttStats.SmoothedRTT()) } @@ -100,7 +100,7 @@ func TestRTTStatsRestore(t *testing.T) { require.Equal(t, 10*time.Second, rttStats.SmoothedRTT()) require.Zero(t, rttStats.MeanDeviation()) // update the RTT and make sure that the initial value is immediately forgotten - rttStats.UpdateRTT(200*time.Millisecond, 0, time.Time{}) + rttStats.UpdateRTT(200*time.Millisecond, 0) require.Equal(t, 200*time.Millisecond, rttStats.LatestRTT()) require.Equal(t, 200*time.Millisecond, rttStats.SmoothedRTT()) require.Equal(t, 100*time.Millisecond, rttStats.MeanDeviation()) @@ -109,7 +109,7 @@ func TestRTTStatsRestore(t *testing.T) { func TestRTTMeasurementAfterRestore(t *testing.T) { var rttStats RTTStats const rtt = 10 * time.Millisecond - rttStats.UpdateRTT(rtt, 0, time.Now()) + rttStats.UpdateRTT(rtt, 0) require.Equal(t, rtt, rttStats.LatestRTT()) require.Equal(t, rtt, rttStats.SmoothedRTT()) rttStats.SetInitialRTT(time.Minute) diff --git a/qlog/connection_tracer_test.go b/qlog/connection_tracer_test.go index 9869430fb..5b7adf603 100644 --- a/qlog/connection_tracer_test.go +++ b/qlog/connection_tracer_test.go @@ -602,11 +602,10 @@ func TestDroppedPacketWithPacketNumber(t *testing.T) { } func TestUpdatedMetrics(t *testing.T) { - now := time.Now() var rttStats utils.RTTStats - rttStats.UpdateRTT(15*time.Millisecond, 0, now) - rttStats.UpdateRTT(20*time.Millisecond, 0, now) - rttStats.UpdateRTT(25*time.Millisecond, 0, now) + rttStats.UpdateRTT(15*time.Millisecond, 0) + rttStats.UpdateRTT(20*time.Millisecond, 0) + rttStats.UpdateRTT(25*time.Millisecond, 0) tracer, buf := newConnectionTracer() tracer.UpdatedMetrics(&rttStats, 4321, 1234, 42) tracer.Close() @@ -626,16 +625,15 @@ func TestUpdatedMetrics(t *testing.T) { } func TestDiffForOnlyChangedMetrics(t *testing.T) { - now := time.Now() var rttStats utils.RTTStats - rttStats.UpdateRTT(15*time.Millisecond, 0, now) - rttStats.UpdateRTT(20*time.Millisecond, 0, now) - rttStats.UpdateRTT(25*time.Millisecond, 0, now) + rttStats.UpdateRTT(15*time.Millisecond, 0) + rttStats.UpdateRTT(20*time.Millisecond, 0) + rttStats.UpdateRTT(25*time.Millisecond, 0) var rttStats2 utils.RTTStats - rttStats2.UpdateRTT(15*time.Millisecond, 0, now) - rttStats2.UpdateRTT(15*time.Millisecond, 0, now) - rttStats2.UpdateRTT(15*time.Millisecond, 0, now) + rttStats2.UpdateRTT(15*time.Millisecond, 0) + rttStats2.UpdateRTT(15*time.Millisecond, 0) + rttStats2.UpdateRTT(15*time.Millisecond, 0) tracer, buf := newConnectionTracer() tracer.UpdatedMetrics(&rttStats, 4321, 1234, 42)