RTT fixes (#7428)

HybridDog · nerzhul · commit 968ce9af5980 · 2018-06-23T09:16:01.000+02:00
* 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 -&gt; 100.0f for stronger average updates
diff --git 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
@@ -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,32 +838,24 @@ 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;
 		if (jitter >= m_rtt.jitter_max)
 			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
@@ -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
@@ -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
@@ -30,7 +30,8 @@ typedef enum {
 	AVG_RTT,
 	MIN_JITTER,
 	MAX_JITTER,
-	AVG_JITTER
+	AVG_JITTER,
+	TIMEOUT_COUNTER
 } rtt_stat_type;
 
 class Peer;

Original file line number	Diff line number	Diff line change
`@@ -437,7 +437,7 @@ void Client::step(float dtime)`
`437`	`437`	`counter = 0.0;`
`438`	`438`	`// connectedAndInitialized() is true, peer exists.`
`439`	`439`	`float avg_rtt = getRTT();`
`440`		`- infostream << "Client: avg_rtt=" << avg_rtt << std::endl;`
	`440`	`+ infostream << "Client: average rtt: " << avg_rtt << std::endl;`
`441`	`441`	`}`
`442`	`442`
`443`	`443`	`/*`
`@@ -1706,9 +1706,17 @@ void Client::afterContentReceived()`
`1706`	`1706`	`delete[] text;`
`1707`	`1707`	`}`
`1708`	`1708`
	`1709`	`+// returns the Round Trip Time`
	`1710`	`+// if the RTT did not become updated within 2 seconds, e.g. before timing out,`
	`1711`	`+// it returns the expired time instead`
`1709`	`1712`	`float Client::getRTT()`
`1710`	`1713`	`{`
`1711`		`- return m_con->getPeerStat(PEER_ID_SERVER,con::AVG_RTT);`
	`1714`	`+ float avg_rtt = m_con->getPeerStat(PEER_ID_SERVER, con::AVG_RTT);`
	`1715`	`+ float time_from_last_rtt =`
	`1716`	`+ m_con->getPeerStat(PEER_ID_SERVER, con::TIMEOUT_COUNTER);`
	`1717`	`+ if (avg_rtt + 2.0f > time_from_last_rtt)`
	`1718`	`+ return avg_rtt;`
	`1719`	`+ return time_from_last_rtt;`
`1712`	`1720`	`}`
`1713`	`1721`
`1714`	`1722`	`float Client::getCurRate()`