From 7a0e52acd65e7097977c924f26b387ec3da9baac Mon Sep 17 00:00:00 2001 From: ANAND Date: Sat, 16 Feb 2019 05:09:22 +0530 Subject: [PATCH] Revert RTT fixes (#8187) The reverted commit 968ce9af598024ec71e9ffb2d15c3997a13ad754 is suspected (through the use of bisection) of causing network slowdowns. Revert for now as we are close to release. --- src/client/client.cpp | 12 ++--------- src/network/connection.cpp | 35 ++++++++++++++++++++----------- src/network/connection.h | 6 +++--- src/network/connectionthreads.cpp | 18 +++++++++------- src/network/peerhandler.h | 3 +-- 5 files changed, 39 insertions(+), 35 deletions(-) diff --git a/src/client/client.cpp b/src/client/client.cpp index 41893fcba..a4a379a73 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -459,7 +459,7 @@ void Client::step(float dtime) counter = 0.0; // connectedAndInitialized() is true, peer exists. float avg_rtt = getRTT(); - infostream << "Client: average rtt: " << avg_rtt << std::endl; + infostream << "Client: avg_rtt=" << avg_rtt << std::endl; } /* @@ -1716,17 +1716,9 @@ 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() { - 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; + return m_con->getPeerStat(PEER_ID_SERVER,con::AVG_RTT); } float Client::getCurRate() diff --git a/src/network/connection.cpp b/src/network/connection.cpp index 495b15426..28084c921 100644 --- a/src/network/connection.cpp +++ b/src/network/connection.cpp @@ -809,9 +809,8 @@ void Peer::DecUseCount() delete this; } -void Peer::RTTStatistics(float rtt, const std::string &profiler_id) -{ - static const float avg_factor = 100.0f / MAX_RELIABLE_WINDOW_SIZE; +void Peer::RTTStatistics(float rtt, const std::string &profiler_id, + unsigned int num_samples) { if (m_last_rtt > 0) { /* set min max values */ @@ -822,14 +821,21 @@ 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 += (rtt - m_rtt.avg_rtt) * avg_factor; + m_rtt.avg_rtt = m_rtt.avg_rtt * (num_samples/(num_samples-1)) + + rtt * (1/num_samples); /* do jitter calculation */ //just use some neutral value at beginning - float jitter = std::fabs(rtt - m_last_rtt); + 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; if (jitter < m_rtt.jitter_min) m_rtt.jitter_min = jitter; @@ -837,9 +843,10 @@ 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 += (jitter - m_rtt.jitter_avg) * avg_factor; + m_rtt.jitter_avg = m_rtt.jitter_avg * (num_samples/(num_samples-1)) + + jitter * (1/num_samples); if (!profiler_id.empty()) { g_profiler->graphAdd(profiler_id + "_rtt", rtt); @@ -904,12 +911,16 @@ bool UDPPeer::getAddress(MTProtocols type,Address& toset) void UDPPeer::reportRTT(float rtt) { - assert(rtt >= 0.0f); - - RTTStatistics(rtt, "rudp"); + if (rtt < 0.0) { + return; + } + RTTStatistics(rtt,"rudp",MAX_RELIABLE_WINDOW_SIZE*10); float timeout = getStat(AVG_RTT) * RESEND_TIMEOUT_FACTOR; - timeout = rangelim(timeout, RESEND_TIMEOUT_MIN, RESEND_TIMEOUT_MAX); + if (timeout < RESEND_TIMEOUT_MIN) + timeout = RESEND_TIMEOUT_MIN; + if (timeout > RESEND_TIMEOUT_MAX) + timeout = 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 f9cd3e6ca..346c7f886 100644 --- a/src/network/connection.h +++ b/src/network/connection.h @@ -581,15 +581,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 = ""); + void RTTStatistics(float rtt, + const std::string &profiler_id = "", + unsigned int num_samples = 1000); bool IncUseCount(); void DecUseCount(); diff --git a/src/network/connectionthreads.cpp b/src/network/connectionthreads.cpp index 5e4397d90..63a9064e7 100644 --- a/src/network/connectionthreads.cpp +++ b/src/network/connectionthreads.cpp @@ -1155,17 +1155,19 @@ 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 = 0.0f; if (current_time > p.absolute_send_time) { - rtt = (current_time - p.absolute_send_time) / 1000.0f; - } else if (p.totaltime > 0) { - rtt = p.totaltime; - } + float rtt = (current_time - p.absolute_send_time) / 1000.0; - // Let peer calculate stuff according to it - // (avg_rtt and resend_timeout) - if (rtt != 0.0f) + // Let peer calculate stuff according to it + // (avg_rtt and resend_timeout) dynamic_cast(peer)->reportRTT(rtt); + } 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); + } } // 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 60b7243bc..208ab801e 100644 --- a/src/network/peerhandler.h +++ b/src/network/peerhandler.h @@ -30,8 +30,7 @@ typedef enum { AVG_RTT, MIN_JITTER, MAX_JITTER, - AVG_JITTER, - TIMEOUT_COUNTER + AVG_JITTER } rtt_stat_type; class Peer;