QUIC: wrong calculation of smoothed rtt
Reported by
psali...@akamai.com,
Aug 25 2016
|
|||||||
Issue descriptionChrome Version : 54.0.2839.3 The code in net/quic/core/congestion_control/rtt_stats.cc calculates SRTT by smoothed_rtt_ = kOneMinusAlpha * smoothed_rtt_ + kAlpha * rtt_sample; But the actual evaluation is due to operator*(QuicTime::Delta lhs, double rhs): smoothed_rtt_ = trunc(kOneMinusAlpha * smoothed_rtt_) + trunc(kAlpha * rtt_sample); instead of desirable smoothed_rtt_ = round(kOneMinusAlpha * smoothed_rtt_ + kAlpha * rtt_sample); What is the expected behaviour? Repeated sampling of RTT QuicTime::Delta::FromMicroseconds(12345) should generate SRTT = 12345 microseconds. The attached patch corrects the calculation and switches it to integer arithmetic.
,
Aug 25 2016
Victor, can you take a look at this?
,
Aug 25 2016
So, I've looked at the current behavior of SRTT calculation, and by feeding it repeatedly the same value the SRTT can be off by 0 to 7 us. That's acceptable, but indeed not optimal. I've added llround() call to operator* for both QuicBandwidth and QuicTime::Delta (in internal version, the change is likely to show up in Chromium some time next week), and it now has up to 1 us error for feeding-the-same-value case, so the equation currently looks like smoothed_rtt_ = round(kOneMinusAlpha * smoothed_rtt_) + round(kAlpha * rtt_sample); The reason why QUIC code in Chromium uses doubles for constants and operator* for QuicTime::Delta/QuicBandwidth is that this makes code more easy to understand and compare against the code present in research papers, and I do not believe being 1us error is too much to pay for that (especially given that random delays in the system are probably much larger than that). So I am hesitant to switch it to integers, especially given that we only recently got rid of switching back-and-forth between microseconds and QuicTime::Delta.
,
Sep 5 2016
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by juliatut...@chromium.org
, Aug 25 2016