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

diff --git a/src/client.cpp b/src/client.cpp
index 18e43b1b6c651c648f74dcb69147e43744dde1ce..09c67c2684a068d209f9a2c4e59263572c47bef4 100644 (file)
--- 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 69dc0cdffaefb5415f1cc01670bfde810d9a557f..a9f8ee082e065958dd3820cc9f1249651e17a665 100644 (file)
--- 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 2fc34416988bc962c0e63334501e80640078a6fd..f65540c8ccfacc19a8a7ad80b8b5532ce217304f 100644 (file)
--- 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 37a435ad85b95434ee8913d75d06208daaa071d6..9d948c59a2790868b61b9e5ac755d554eb5dfc5c 100644 (file)
--- a/src/network/connectionthreads.cpp
+++ b/src/network/connectionthreads.cpp
@@ -1167,19 +1167,16 @@ SharedBuffer<u8> 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<UDPPeer *>(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<UDPPeer *>(peer)->reportRTT(rtt);
+                                       rtt = p.totaltime;
                                }
+
+                               // Let peer calculate stuff according to it
+                               // (avg_rtt and resend_timeout)
+                               dynamic_cast<UDPPeer *>(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 208ab801e66805b42d6354d02a4d672fdb08401f..60b7243bc9ec1130b6b693049751f24fced25a68 100644 (file)
--- 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;