From 968ce9af598024ec71e9ffb2d15c3997a13ad754 Mon Sep 17 00:00:00 2001 From: you Date: Sat, 23 Jun 2018 09:16:01 +0200 Subject: [PATCH] RTT fixes (#7428) * Few code updates * Do not show average RTT before timing out * Fix unwanted integer division in RTTStatistics * Fix float format, prettier jitter calculation * Use +=, 0.1f -> 100.0f for stronger average updates --- src/client.cpp | 12 +++++++++-- src/network/connection.cpp | 35 +++++++++++-------------------- src/network/connection.h | 6 +++--- src/network/connectionthreads.cpp | 17 +++++++-------- src/network/peerhandler.h | 3 ++- 5 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 18e43b1b6..09c67c268 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -437,7 +437,7 @@ void Client::step(float dtime) counter = 0.0; // connectedAndInitialized() is true, peer exists. float avg_rtt = getRTT(); - infostream << "Client: avg_rtt=" << avg_rtt << std::endl; + infostream << "Client: average rtt: " << avg_rtt << std::endl; } /* @@ -1706,9 +1706,17 @@ void Client::afterContentReceived() delete[] text; } +// returns the Round Trip Time +// if the RTT did not become updated within 2 seconds, e.g. before timing out, +// it returns the expired time instead float Client::getRTT() { - return m_con->getPeerStat(PEER_ID_SERVER,con::AVG_RTT); + float avg_rtt = m_con->getPeerStat(PEER_ID_SERVER, con::AVG_RTT); + float time_from_last_rtt = + m_con->getPeerStat(PEER_ID_SERVER, con::TIMEOUT_COUNTER); + if (avg_rtt + 2.0f > time_from_last_rtt) + return avg_rtt; + return time_from_last_rtt; } float Client::getCurRate() diff --git a/src/network/connection.cpp b/src/network/connection.cpp index 69dc0cdff..a9f8ee082 100644 --- a/src/network/connection.cpp +++ b/src/network/connection.cpp @@ -825,8 +825,9 @@ void Peer::DecUseCount() delete this; } -void Peer::RTTStatistics(float rtt, const std::string &profiler_id, - unsigned int num_samples) { +void Peer::RTTStatistics(float rtt, const std::string &profiler_id) +{ + static const float avg_factor = 100.0f / MAX_RELIABLE_WINDOW_SIZE; if (m_last_rtt > 0) { /* set min max values */ @@ -837,21 +838,14 @@ void Peer::RTTStatistics(float rtt, const std::string &profiler_id, /* do average calculation */ if (m_rtt.avg_rtt < 0.0) - m_rtt.avg_rtt = rtt; + m_rtt.avg_rtt = rtt; else - m_rtt.avg_rtt = m_rtt.avg_rtt * (num_samples/(num_samples-1)) + - rtt * (1/num_samples); + m_rtt.avg_rtt += (rtt - m_rtt.avg_rtt) * avg_factor; /* do jitter calculation */ //just use some neutral value at beginning - float jitter = m_rtt.jitter_min; - - if (rtt > m_last_rtt) - jitter = rtt-m_last_rtt; - - if (rtt <= m_last_rtt) - jitter = m_last_rtt - rtt; + float jitter = std::fabs(rtt - m_last_rtt); if (jitter < m_rtt.jitter_min) m_rtt.jitter_min = jitter; @@ -859,10 +853,9 @@ void Peer::RTTStatistics(float rtt, const std::string &profiler_id, m_rtt.jitter_max = jitter; if (m_rtt.jitter_avg < 0.0) - m_rtt.jitter_avg = jitter; + m_rtt.jitter_avg = jitter; else - m_rtt.jitter_avg = m_rtt.jitter_avg * (num_samples/(num_samples-1)) + - jitter * (1/num_samples); + m_rtt.jitter_avg += (jitter - m_rtt.jitter_avg) * avg_factor; if (!profiler_id.empty()) { g_profiler->graphAdd(profiler_id + "_rtt", rtt); @@ -934,16 +927,12 @@ void UDPPeer::setNonLegacyPeer() void UDPPeer::reportRTT(float rtt) { - if (rtt < 0.0) { - return; - } - RTTStatistics(rtt,"rudp",MAX_RELIABLE_WINDOW_SIZE*10); + assert(rtt >= 0.0f); + + RTTStatistics(rtt, "rudp"); float timeout = getStat(AVG_RTT) * RESEND_TIMEOUT_FACTOR; - if (timeout < RESEND_TIMEOUT_MIN) - timeout = RESEND_TIMEOUT_MIN; - if (timeout > RESEND_TIMEOUT_MAX) - timeout = RESEND_TIMEOUT_MAX; + timeout = rangelim(timeout, RESEND_TIMEOUT_MIN, RESEND_TIMEOUT_MAX); MutexAutoLock usage_lock(m_exclusive_access_mutex); resend_timeout = timeout; diff --git a/src/network/connection.h b/src/network/connection.h index 2fc344169..f65540c8c 100644 --- a/src/network/connection.h +++ b/src/network/connection.h @@ -593,15 +593,15 @@ class Peer { return m_rtt.jitter_max; case AVG_JITTER: return m_rtt.jitter_avg; + case TIMEOUT_COUNTER: + return m_timeout_counter; } return -1; } protected: virtual void reportRTT(float rtt) {}; - void RTTStatistics(float rtt, - const std::string &profiler_id = "", - unsigned int num_samples = 1000); + void RTTStatistics(float rtt, const std::string &profiler_id = ""); bool IncUseCount(); void DecUseCount(); diff --git a/src/network/connectionthreads.cpp b/src/network/connectionthreads.cpp index 37a435ad8..9d948c59a 100644 --- a/src/network/connectionthreads.cpp +++ b/src/network/connectionthreads.cpp @@ -1167,19 +1167,16 @@ SharedBuffer ConnectionReceiveThread::handlePacketType_Control(Channel *chan // a overflow is quite unlikely but as it'd result in major // rtt miscalculation we handle it here + float rtt; if (current_time > p.absolute_send_time) { - float rtt = (current_time - p.absolute_send_time) / 1000.0; - - // Let peer calculate stuff according to it - // (avg_rtt and resend_timeout) - dynamic_cast(peer)->reportRTT(rtt); + rtt = (current_time - p.absolute_send_time) / 1000.0f; } else if (p.totaltime > 0) { - float rtt = p.totaltime; - - // Let peer calculate stuff according to it - // (avg_rtt and resend_timeout) - dynamic_cast(peer)->reportRTT(rtt); + rtt = p.totaltime; } + + // Let peer calculate stuff according to it + // (avg_rtt and resend_timeout) + dynamic_cast(peer)->reportRTT(rtt); } // put bytes for max bandwidth calculation channel->UpdateBytesSent(p.data.getSize(), 1); diff --git a/src/network/peerhandler.h b/src/network/peerhandler.h index 208ab801e..60b7243bc 100644 --- a/src/network/peerhandler.h +++ b/src/network/peerhandler.h @@ -30,7 +30,8 @@ typedef enum { AVG_RTT, MIN_JITTER, MAX_JITTER, - AVG_JITTER + AVG_JITTER, + TIMEOUT_COUNTER } rtt_stat_type; class Peer;