Skip to content

Commit 7a0e52a

Browse files
ClobberXDparamat
ClobberXD
authored and
paramat
committedFeb 15, 2019
Revert RTT fixes (#8187)
The reverted commit 968ce9a is suspected (through the use of bisection) of causing network slowdowns. Revert for now as we are close to release.
·
5.11.05.0.0
1 parent 2153163 commit 7a0e52a

File tree

5 files changed

+38
-34
lines changed

5 files changed

+38
-34
lines changed
 

Diff for: ‎src/client/client.cpp

+2-10
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ void Client::step(float dtime)
459459
counter = 0.0;
460460
// connectedAndInitialized() is true, peer exists.
461461
float avg_rtt = getRTT();
462-
infostream << "Client: average rtt: " << avg_rtt << std::endl;
462+
infostream << "Client: avg_rtt=" << avg_rtt << std::endl;
463463
}
464464

465465
/*
@@ -1716,17 +1716,9 @@ void Client::afterContentReceived()
17161716
delete[] text;
17171717
}
17181718

1719-
// returns the Round Trip Time
1720-
// if the RTT did not become updated within 2 seconds, e.g. before timing out,
1721-
// it returns the expired time instead
17221719
float Client::getRTT()
17231720
{
1724-
float avg_rtt = m_con->getPeerStat(PEER_ID_SERVER, con::AVG_RTT);
1725-
float time_from_last_rtt =
1726-
m_con->getPeerStat(PEER_ID_SERVER, con::TIMEOUT_COUNTER);
1727-
if (avg_rtt + 2.0f > time_from_last_rtt)
1728-
return avg_rtt;
1729-
return time_from_last_rtt;
1721+
return m_con->getPeerStat(PEER_ID_SERVER,con::AVG_RTT);
17301722
}
17311723

17321724
float Client::getCurRate()

Diff for: ‎src/network/connection.cpp

+23-12
Original file line numberDiff line numberDiff line change
@@ -809,9 +809,8 @@ void Peer::DecUseCount()
809809
delete this;
810810
}
811811

812-
void Peer::RTTStatistics(float rtt, const std::string &profiler_id)
813-
{
814-
static const float avg_factor = 100.0f / MAX_RELIABLE_WINDOW_SIZE;
812+
void Peer::RTTStatistics(float rtt, const std::string &profiler_id,
813+
unsigned int num_samples) {
815814

816815
if (m_last_rtt > 0) {
817816
/* set min max values */
@@ -822,24 +821,32 @@ void Peer::RTTStatistics(float rtt, const std::string &profiler_id)
822821

823822
/* do average calculation */
824823
if (m_rtt.avg_rtt < 0.0)
825-
m_rtt.avg_rtt = rtt;
824+
m_rtt.avg_rtt = rtt;
826825
else
827-
m_rtt.avg_rtt += (rtt - m_rtt.avg_rtt) * avg_factor;
826+
m_rtt.avg_rtt = m_rtt.avg_rtt * (num_samples/(num_samples-1)) +
827+
rtt * (1/num_samples);
828828

829829
/* do jitter calculation */
830830

831831
//just use some neutral value at beginning
832-
float jitter = std::fabs(rtt - m_last_rtt);
832+
float jitter = m_rtt.jitter_min;
833+
834+
if (rtt > m_last_rtt)
835+
jitter = rtt-m_last_rtt;
836+
837+
if (rtt <= m_last_rtt)
838+
jitter = m_last_rtt - rtt;
833839

834840
if (jitter < m_rtt.jitter_min)
835841
m_rtt.jitter_min = jitter;
836842
if (jitter >= m_rtt.jitter_max)
837843
m_rtt.jitter_max = jitter;
838844

839845
if (m_rtt.jitter_avg < 0.0)
840-
m_rtt.jitter_avg = jitter;
846+
m_rtt.jitter_avg = jitter;
841847
else
842-
m_rtt.jitter_avg += (jitter - m_rtt.jitter_avg) * avg_factor;
848+
m_rtt.jitter_avg = m_rtt.jitter_avg * (num_samples/(num_samples-1)) +
849+
jitter * (1/num_samples);
843850

844851
if (!profiler_id.empty()) {
845852
g_profiler->graphAdd(profiler_id + "_rtt", rtt);
@@ -904,12 +911,16 @@ bool UDPPeer::getAddress(MTProtocols type,Address& toset)
904911

905912
void UDPPeer::reportRTT(float rtt)
906913
{
907-
assert(rtt >= 0.0f);
908-
909-
RTTStatistics(rtt, "rudp");
914+
if (rtt < 0.0) {
915+
return;
916+
}
917+
RTTStatistics(rtt,"rudp",MAX_RELIABLE_WINDOW_SIZE*10);
910918

911919
float timeout = getStat(AVG_RTT) * RESEND_TIMEOUT_FACTOR;
912-
timeout = rangelim(timeout, RESEND_TIMEOUT_MIN, RESEND_TIMEOUT_MAX);
920+
if (timeout < RESEND_TIMEOUT_MIN)
921+
timeout = RESEND_TIMEOUT_MIN;
922+
if (timeout > RESEND_TIMEOUT_MAX)
923+
timeout = RESEND_TIMEOUT_MAX;
913924

914925
MutexAutoLock usage_lock(m_exclusive_access_mutex);
915926
resend_timeout = timeout;

Diff for: ‎src/network/connection.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -581,15 +581,15 @@ class Peer {
581581
return m_rtt.jitter_max;
582582
case AVG_JITTER:
583583
return m_rtt.jitter_avg;
584-
case TIMEOUT_COUNTER:
585-
return m_timeout_counter;
586584
}
587585
return -1;
588586
}
589587
protected:
590588
virtual void reportRTT(float rtt) {};
591589

592-
void RTTStatistics(float rtt, const std::string &profiler_id = "");
590+
void RTTStatistics(float rtt,
591+
const std::string &profiler_id = "",
592+
unsigned int num_samples = 1000);
593593

594594
bool IncUseCount();
595595
void DecUseCount();

Diff for: ‎src/network/connectionthreads.cpp

+9-7
Original file line numberDiff line numberDiff line change
@@ -1155,17 +1155,19 @@ SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Control(Channel *chan
11551155

11561156
// a overflow is quite unlikely but as it'd result in major
11571157
// rtt miscalculation we handle it here
1158-
float rtt = 0.0f;
11591158
if (current_time > p.absolute_send_time) {
1160-
rtt = (current_time - p.absolute_send_time) / 1000.0f;
1159+
float rtt = (current_time - p.absolute_send_time) / 1000.0;
1160+
1161+
// Let peer calculate stuff according to it
1162+
// (avg_rtt and resend_timeout)
1163+
dynamic_cast<UDPPeer *>(peer)->reportRTT(rtt);
11611164
} else if (p.totaltime > 0) {
1162-
rtt = p.totaltime;
1163-
}
1165+
float rtt = p.totaltime;
11641166

1165-
// Let peer calculate stuff according to it
1166-
// (avg_rtt and resend_timeout)
1167-
if (rtt != 0.0f)
1167+
// Let peer calculate stuff according to it
1168+
// (avg_rtt and resend_timeout)
11681169
dynamic_cast<UDPPeer *>(peer)->reportRTT(rtt);
1170+
}
11691171
}
11701172
// put bytes for max bandwidth calculation
11711173
channel->UpdateBytesSent(p.data.getSize(), 1);

Diff for: ‎src/network/peerhandler.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ typedef enum {
3030
AVG_RTT,
3131
MIN_JITTER,
3232
MAX_JITTER,
33-
AVG_JITTER,
34-
TIMEOUT_COUNTER
33+
AVG_JITTER
3534
} rtt_stat_type;
3635

3736
class Peer;

0 commit comments

Comments
 (0)
Please sign in to comment.