From 2e3040715b915a022099122c2d9f01cc3e4b7ce5 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 30 Mar 2016 14:59:46 -0700 Subject: [PATCH 1/6] Delay congestion epoch's first decrease --- .../networking/src/udt/CongestionControl.cpp | 42 ++++++++++++------- .../networking/src/udt/CongestionControl.h | 12 ++++-- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/libraries/networking/src/udt/CongestionControl.cpp b/libraries/networking/src/udt/CongestionControl.cpp index d30be2c139..bb95fef05b 100644 --- a/libraries/networking/src/udt/CongestionControl.cpp +++ b/libraries/networking/src/udt/CongestionControl.cpp @@ -11,8 +11,6 @@ #include "CongestionControl.h" -#include - #include "Packet.h" using namespace udt; @@ -40,7 +38,10 @@ void CongestionControl::setPacketSendPeriod(double newSendPeriod) { } DefaultCC::DefaultCC() : - _lastDecreaseMaxSeq(SequenceNumber {SequenceNumber::MAX }) + _lastDecreaseMaxSeq(SequenceNumber {SequenceNumber::MAX }), + _rd(), + _generator(_rd()), + _distribution(1, 1) { _mss = udt::MAX_PACKET_SIZE_WITH_UDP_HEADER; @@ -152,15 +153,18 @@ void DefaultCC::onLoss(SequenceNumber rangeStart, SequenceNumber rangeEnd) { // check if this NAK starts a new congestion period - this will be the case if the // NAK received occured for a packet sent after the last decrease if (rangeStart > _lastDecreaseMaxSeq) { + _delayedDecrease = (rangeStart == rangeEnd); _lastDecreasePeriod = _packetSendPeriod; - - setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); // use EWMA to update the average number of NAKs per congestion static const double NAK_EWMA_ALPHA = 0.125; _avgNAKNum = (int)ceil(_avgNAKNum * (1 - NAK_EWMA_ALPHA) + _nakCount * NAK_EWMA_ALPHA); - + + if (!_delayedDecrease) { + setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); + } + // update the count of NAKs and count of decreases in this interval _nakCount = 1; _decreaseCount = 1; @@ -171,18 +175,24 @@ void DefaultCC::onLoss(SequenceNumber rangeStart, SequenceNumber rangeEnd) { _randomDecreaseThreshold = 1; } else { // avoid synchronous rate decrease across connections using randomization - std::random_device rd; - std::mt19937 generator(rd()); - std::uniform_int_distribution<> distribution(1, std::max(1, _avgNAKNum)); + if (_distribution.b() != _avgNAKNum) { + _distribution = std::uniform_int_distribution<>(1, std::max(1, _avgNAKNum)); + } - _randomDecreaseThreshold = distribution(generator); + _randomDecreaseThreshold = _distribution(_generator); + } + } else { + ++_nakCount; + + if (_delayedDecrease && _nakCount == 2) { + setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); + } else if ((_decreaseCount++ < MAX_DECREASES_PER_CONGESTION_EPOCH) && ((_nakCount % _randomDecreaseThreshold) == 0)) { + // there have been fewer than MAX_DECREASES_PER_CONGESTION_EPOCH AND this NAK matches the random count at which we + // decided we would decrease the packet send period + + setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); + _lastDecreaseMaxSeq = _sendCurrSeqNum; } - } else if ((_decreaseCount++ < MAX_DECREASES_PER_CONGESTION_EPOCH) && ((++_nakCount % _randomDecreaseThreshold) == 0)) { - // there have been fewer than MAX_DECREASES_PER_CONGESTION_EPOCH AND this NAK matches the random count at which we - // decided we would decrease the packet send period - - setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); - _lastDecreaseMaxSeq = _sendCurrSeqNum; } } diff --git a/libraries/networking/src/udt/CongestionControl.h b/libraries/networking/src/udt/CongestionControl.h index 8297b5f6bd..9599df7465 100644 --- a/libraries/networking/src/udt/CongestionControl.h +++ b/libraries/networking/src/udt/CongestionControl.h @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include @@ -81,8 +81,8 @@ private: bool _userDefinedRTO { false }; // if the RTO value is defined by users int _rto { -1 }; // RTO value, microseconds }; - - + + class CongestionControlVirtualFactory { public: virtual ~CongestionControlVirtualFactory() {} @@ -124,6 +124,12 @@ private: int _randomDecreaseThreshold { 1 }; // random threshold on decrease by number of loss events int _avgNAKNum { 0 }; // average number of NAKs per congestion int _decreaseCount { 0 }; // number of decreases in a congestion epoch + + + bool _delayedDecrease { false }; + std::random_device _rd; + std::mt19937 _generator; + std::uniform_int_distribution<> _distribution; }; } From c95bc1c563f48423d9fc3b3426915943ae34eb4c Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 30 Mar 2016 15:58:31 -0700 Subject: [PATCH 2/6] Don't set _loss on delayed decrease --- libraries/networking/src/udt/CongestionControl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/networking/src/udt/CongestionControl.cpp b/libraries/networking/src/udt/CongestionControl.cpp index bb95fef05b..a62ec072bc 100644 --- a/libraries/networking/src/udt/CongestionControl.cpp +++ b/libraries/networking/src/udt/CongestionControl.cpp @@ -163,6 +163,8 @@ void DefaultCC::onLoss(SequenceNumber rangeStart, SequenceNumber rangeEnd) { if (!_delayedDecrease) { setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); + } else { + _loss = false; } // update the count of NAKs and count of decreases in this interval From a3b771c4dc0f0cda64b511f4f5e02897c52eeb08 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 31 Mar 2016 10:46:09 -0700 Subject: [PATCH 3/6] UdtTest uses Mbps --- tools/udt-test/src/UDTTest.cpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tools/udt-test/src/UDTTest.cpp b/tools/udt-test/src/UDTTest.cpp index 2b5e306b09..03c98799c1 100644 --- a/tools/udt-test/src/UDTTest.cpp +++ b/tools/udt-test/src/UDTTest.cpp @@ -57,13 +57,13 @@ const QCommandLineOption STATS_INTERVAL { }; const QStringList CLIENT_STATS_TABLE_HEADERS { - "Send (P/s)", "Est. Max (P/s)", "RTT (ms)", "CW (P)", "Period (us)", + "Send (Mb/s)", "Est. Max (Mb/s)", "RTT (ms)", "CW (P)", "Period (us)", "Recv ACK", "Procd ACK", "Recv LACK", "Recv NAK", "Recv TNAK", "Sent ACK2", "Sent Packets", "Re-sent Packets" }; const QStringList SERVER_STATS_TABLE_HEADERS { - " Mb/s ", "Recv P/s", "Est. Max (P/s)", "RTT (ms)", "CW (P)", + " Mb/s ", "Recv Mb/s", "Est. Max (Mb/s)", "RTT (ms)", "CW (P)", "Sent ACK", "Sent LACK", "Sent NAK", "Sent TNAK", "Recv ACK2", "Duplicates (P)" }; @@ -364,7 +364,11 @@ void UDTTest::handleMessage(std::unique_ptr message) { void UDTTest::sampleStats() { static bool first = true; static const double USECS_PER_MSEC = 1000.0; - + static const double MEGABITS_PER_BYTE = 8.0 / 1000000.0; + static const double MS_PER_SECOND = 1000.0; + static const double PPS_TO_MBPS = 1500.0 * 8.0 / 1000000.0; + + if (!_target.isNull()) { if (first) { // output the headers for stats for our table @@ -378,8 +382,8 @@ void UDTTest::sampleStats() { // setup a list of left justified values QStringList values { - QString::number(stats.sendRate).rightJustified(CLIENT_STATS_TABLE_HEADERS[++headerIndex].size()), - QString::number(stats.estimatedBandwith).rightJustified(CLIENT_STATS_TABLE_HEADERS[++headerIndex].size()), + QString::number(stats.sendRate * PPS_TO_MBPS).rightJustified(CLIENT_STATS_TABLE_HEADERS[++headerIndex].size()), + QString::number(stats.estimatedBandwith * PPS_TO_MBPS).rightJustified(CLIENT_STATS_TABLE_HEADERS[++headerIndex].size()), QString::number(stats.rtt / USECS_PER_MSEC, 'f', 2).rightJustified(CLIENT_STATS_TABLE_HEADERS[++headerIndex].size()), QString::number(stats.congestionWindowSize).rightJustified(CLIENT_STATS_TABLE_HEADERS[++headerIndex].size()), QString::number(stats.packetSendPeriod).rightJustified(CLIENT_STATS_TABLE_HEADERS[++headerIndex].size()), @@ -408,16 +412,13 @@ void UDTTest::sampleStats() { int headerIndex = -1; - static const double MEGABITS_PER_BYTE = 8.0 / 1000000.0; - static const double MS_PER_SECOND = 1000.0; - double megabitsPerSecond = (stats.receivedBytes * MEGABITS_PER_BYTE * MS_PER_SECOND) / _statsInterval; // setup a list of left justified values QStringList values { QString::number(megabitsPerSecond, 'f', 2).rightJustified(SERVER_STATS_TABLE_HEADERS[++headerIndex].size()), - QString::number(stats.receiveRate).rightJustified(SERVER_STATS_TABLE_HEADERS[++headerIndex].size()), - QString::number(stats.estimatedBandwith).rightJustified(SERVER_STATS_TABLE_HEADERS[++headerIndex].size()), + QString::number(stats.receiveRate * PPS_TO_MBPS).rightJustified(SERVER_STATS_TABLE_HEADERS[++headerIndex].size()), + QString::number(stats.estimatedBandwith * PPS_TO_MBPS).rightJustified(SERVER_STATS_TABLE_HEADERS[++headerIndex].size()), QString::number(stats.rtt / USECS_PER_MSEC, 'f', 2).rightJustified(SERVER_STATS_TABLE_HEADERS[++headerIndex].size()), QString::number(stats.congestionWindowSize).rightJustified(SERVER_STATS_TABLE_HEADERS[++headerIndex].size()), QString::number(stats.events[udt::ConnectionStats::Stats::SentACK]).rightJustified(SERVER_STATS_TABLE_HEADERS[++headerIndex].size()), From 86b425143b4de5ad619b731b310b41ff405298a6 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 31 Mar 2016 11:03:06 -0700 Subject: [PATCH 4/6] Use Mbps for max bandwith --- assignment-client/src/assets/AssetServer.cpp | 6 +++--- libraries/networking/src/udt/CongestionControl.cpp | 2 +- libraries/networking/src/udt/Socket.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index fad8ece7bf..0bf59eb8ad 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -82,11 +82,11 @@ void AssetServer::completeSetup() { auto maxBandwidthFloat = maxBandwidthValue.toDouble(-1); if (maxBandwidthFloat > 0.0) { - const int BYTES_PER_MEGABITS = (1024 * 1024) / 8; - int maxBandwidth = maxBandwidthFloat * BYTES_PER_MEGABITS; + const int BITS_PER_MEGABITS = 1000 * 1000; + int maxBandwidth = maxBandwidthFloat * BITS_PER_MEGABITS; nodeList->setConnectionMaxBandwidth(maxBandwidth); qInfo() << "Set maximum bandwith per connection to" << maxBandwidthFloat << "Mb/s." - " (" << maxBandwidth << "bytes/sec)"; + " (" << maxBandwidth << "bits/s)"; } // get the path to the asset folder from the domain server settings diff --git a/libraries/networking/src/udt/CongestionControl.cpp b/libraries/networking/src/udt/CongestionControl.cpp index a62ec072bc..e583ff7e3b 100644 --- a/libraries/networking/src/udt/CongestionControl.cpp +++ b/libraries/networking/src/udt/CongestionControl.cpp @@ -19,7 +19,7 @@ using namespace std::chrono; static const double USECS_PER_SECOND = 1000000.0; void CongestionControl::setMaxBandwidth(int maxBandwidth) { - _maxBandwidth = maxBandwidth; + _maxBandwidth = maxBandwidth / 8; setPacketSendPeriod(_packetSendPeriod); } diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index e9af1577fb..0695a65e1f 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -354,7 +354,7 @@ void Socket::setCongestionControlFactory(std::unique_ptr Date: Thu, 31 Mar 2016 15:56:42 -0700 Subject: [PATCH 5/6] Code cleanup --- .../networking/src/udt/CongestionControl.cpp | 53 +++++++++---------- .../networking/src/udt/CongestionControl.h | 14 ++--- 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/libraries/networking/src/udt/CongestionControl.cpp b/libraries/networking/src/udt/CongestionControl.cpp index e583ff7e3b..5826bfa11c 100644 --- a/libraries/networking/src/udt/CongestionControl.cpp +++ b/libraries/networking/src/udt/CongestionControl.cpp @@ -11,26 +11,29 @@ #include "CongestionControl.h" +#include + #include "Packet.h" using namespace udt; using namespace std::chrono; static const double USECS_PER_SECOND = 1000000.0; +static const int BITS_PER_BYTE = 8; void CongestionControl::setMaxBandwidth(int maxBandwidth) { - _maxBandwidth = maxBandwidth / 8; + _maxBandwidth = maxBandwidth; setPacketSendPeriod(_packetSendPeriod); } void CongestionControl::setPacketSendPeriod(double newSendPeriod) { Q_ASSERT_X(newSendPeriod >= 0, "CongestionControl::setPacketPeriod", "Can not set a negative packet send period"); - auto maxBandwidth = _maxBandwidth.load(); - if (maxBandwidth > 0) { + auto packetsPerSecond = (double)_maxBandwidth / (BITS_PER_BYTE * _mss); + if (packetsPerSecond > 0.0) { // anytime the packet send period is about to be increased, make sure it stays below the minimum period, // calculated based on the maximum desired bandwidth - double minPacketSendPeriod = USECS_PER_SECOND / (((double) maxBandwidth) / _mss); + double minPacketSendPeriod = USECS_PER_SECOND / packetsPerSecond; _packetSendPeriod = std::max(newSendPeriod, minPacketSendPeriod); } else { _packetSendPeriod = newSendPeriod; @@ -38,10 +41,7 @@ void CongestionControl::setPacketSendPeriod(double newSendPeriod) { } DefaultCC::DefaultCC() : - _lastDecreaseMaxSeq(SequenceNumber {SequenceNumber::MAX }), - _rd(), - _generator(_rd()), - _distribution(1, 1) + _lastDecreaseMaxSeq(SequenceNumber {SequenceNumber::MAX }) { _mss = udt::MAX_PACKET_SIZE_WITH_UDP_HEADER; @@ -146,7 +146,8 @@ void DefaultCC::onLoss(SequenceNumber rangeStart, SequenceNumber rangeEnd) { } _loss = true; - + ++_nakCount; + static const double INTER_PACKET_ARRIVAL_INCREASE = 1.125; static const int MAX_DECREASES_PER_CONGESTION_EPOCH = 5; @@ -157,16 +158,16 @@ void DefaultCC::onLoss(SequenceNumber rangeStart, SequenceNumber rangeEnd) { _lastDecreasePeriod = _packetSendPeriod; - // use EWMA to update the average number of NAKs per congestion - static const double NAK_EWMA_ALPHA = 0.125; - _avgNAKNum = (int)ceil(_avgNAKNum * (1 - NAK_EWMA_ALPHA) + _nakCount * NAK_EWMA_ALPHA); - if (!_delayedDecrease) { setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); } else { _loss = false; } + // use EWMA to update the average number of NAKs per congestion + static const double NAK_EWMA_ALPHA = 0.125; + _avgNAKNum = (int)ceil(_avgNAKNum * (1 - NAK_EWMA_ALPHA) + _nakCount * NAK_EWMA_ALPHA); + // update the count of NAKs and count of decreases in this interval _nakCount = 1; _decreaseCount = 1; @@ -177,24 +178,20 @@ void DefaultCC::onLoss(SequenceNumber rangeStart, SequenceNumber rangeEnd) { _randomDecreaseThreshold = 1; } else { // avoid synchronous rate decrease across connections using randomization - if (_distribution.b() != _avgNAKNum) { - _distribution = std::uniform_int_distribution<>(1, std::max(1, _avgNAKNum)); - } + std::random_device rd; + std::mt19937 generator(rd()); + std::uniform_int_distribution<> distribution(1, std::max(1, _avgNAKNum)); - _randomDecreaseThreshold = _distribution(_generator); + _randomDecreaseThreshold = distribution(generator); } - } else { - ++_nakCount; + } else if (_delayedDecrease && _nakCount == 2) { + setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); + } else if ((_decreaseCount++ < MAX_DECREASES_PER_CONGESTION_EPOCH) && ((_nakCount % _randomDecreaseThreshold) == 0)) { + // there have been fewer than MAX_DECREASES_PER_CONGESTION_EPOCH AND this NAK matches the random count at which we + // decided we would decrease the packet send period - if (_delayedDecrease && _nakCount == 2) { - setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); - } else if ((_decreaseCount++ < MAX_DECREASES_PER_CONGESTION_EPOCH) && ((_nakCount % _randomDecreaseThreshold) == 0)) { - // there have been fewer than MAX_DECREASES_PER_CONGESTION_EPOCH AND this NAK matches the random count at which we - // decided we would decrease the packet send period - - setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); - _lastDecreaseMaxSeq = _sendCurrSeqNum; - } + setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); + _lastDecreaseMaxSeq = _sendCurrSeqNum; } } diff --git a/libraries/networking/src/udt/CongestionControl.h b/libraries/networking/src/udt/CongestionControl.h index 9599df7465..cb99e6c8d4 100644 --- a/libraries/networking/src/udt/CongestionControl.h +++ b/libraries/networking/src/udt/CongestionControl.h @@ -15,7 +15,6 @@ #include #include #include -#include #include @@ -61,7 +60,7 @@ protected: double _congestionWindowSize { 16.0 }; // Congestion window size, in packets int _bandwidth { 0 }; // estimated bandwidth, packets per second - std::atomic _maxBandwidth { -1 }; // Maximum desired bandwidth, bytes per second + std::atomic _maxBandwidth { -1 }; // Maximum desired bandwidth, bits per second double _maxCongestionWindowSize { 0.0 }; // maximum cwnd size, in packets int _mss { 0 }; // Maximum Packet Size, including all packet headers @@ -81,8 +80,8 @@ private: bool _userDefinedRTO { false }; // if the RTO value is defined by users int _rto { -1 }; // RTO value, microseconds }; - - + + class CongestionControlVirtualFactory { public: virtual ~CongestionControlVirtualFactory() {} @@ -124,12 +123,7 @@ private: int _randomDecreaseThreshold { 1 }; // random threshold on decrease by number of loss events int _avgNAKNum { 0 }; // average number of NAKs per congestion int _decreaseCount { 0 }; // number of decreases in a congestion epoch - - - bool _delayedDecrease { false }; - std::random_device _rd; - std::mt19937 _generator; - std::uniform_int_distribution<> _distribution; + bool _delayedDecrease { false }; }; } From 0409479a12f3e4835dddf15bc01615267bd12005 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 31 Mar 2016 16:36:34 -0700 Subject: [PATCH 6/6] CR --- tools/udt-test/src/UDTTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/udt-test/src/UDTTest.cpp b/tools/udt-test/src/UDTTest.cpp index 03c98799c1..2f2e896d87 100644 --- a/tools/udt-test/src/UDTTest.cpp +++ b/tools/udt-test/src/UDTTest.cpp @@ -366,7 +366,7 @@ void UDTTest::sampleStats() { static const double USECS_PER_MSEC = 1000.0; static const double MEGABITS_PER_BYTE = 8.0 / 1000000.0; static const double MS_PER_SECOND = 1000.0; - static const double PPS_TO_MBPS = 1500.0 * 8.0 / 1000000.0; + static const double PPS_TO_MBPS = 1500.0 * MEGABITS_PER_BYTE; if (!_target.isNull()) {