From 84ab4fd5856474e630bca9758f13f0fd8bade0b6 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 14 Sep 2015 17:13:43 -0700 Subject: [PATCH 1/5] force minimum for p_hrc time_point values --- libraries/networking/src/udt/CongestionControl.h | 3 ++- libraries/networking/src/udt/Connection.cpp | 6 +++--- libraries/networking/src/udt/Connection.h | 6 +++--- libraries/networking/src/udt/PacketTimeWindow.h | 4 ++-- libraries/networking/src/udt/SendQueue.cpp | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/libraries/networking/src/udt/CongestionControl.h b/libraries/networking/src/udt/CongestionControl.h index adbf1f0e85..5f9b64c1ad 100644 --- a/libraries/networking/src/udt/CongestionControl.h +++ b/libraries/networking/src/udt/CongestionControl.h @@ -108,7 +108,8 @@ public: private: void stopSlowStart(); // stops the slow start on loss or timeout - p_high_resolution_clock::time_point _lastRCTime; // last rate increase time + p_high_resolution_clock::time_point _lastRCTime = p_high_resolution_clock::time_point().min(); // last rate increase time + bool _slowStart { true }; // if in slow start phase SequenceNumber _slowStartLastAck; // last ACKed seq num bool _loss { false }; // if loss happened since last rate increase diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 21dce2831c..4aa785fa8c 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -222,7 +222,7 @@ void Connection::recordRetransmission() { } void Connection::sendACK(bool wasCausedBySyncTimeout) { - static p_high_resolution_clock::time_point lastACKSendTime; + static p_high_resolution_clock::time_point lastACKSendTime = p_high_resolution_clock::time_point().min(); auto currentTime = p_high_resolution_clock::now(); SequenceNumber nextACKNumber = nextACK(); @@ -534,7 +534,7 @@ void Connection::processACK(std::unique_ptr controlPacket) { // This will be the case if it has been longer than the sync interval OR // it looks like they haven't received our ACK2 for this ACK auto currentTime = p_high_resolution_clock::now(); - static p_high_resolution_clock::time_point lastACK2SendTime; + static p_high_resolution_clock::time_point lastACK2SendTime = p_high_resolution_clock::time_point().min(); microseconds sinceLastACK2 = duration_cast(currentTime - lastACK2SendTime); @@ -779,7 +779,7 @@ void Connection::resetReceiveState() { // clear the loss list and _lastNAKTime _lossList.clear(); - _lastNAKTime = p_high_resolution_clock::time_point(); + _lastNAKTime = p_high_resolution_clock::time_point().min(); // the _nakInterval need not be reset, that will happen on loss diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index 31ef664ce5..346f559165 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -114,13 +114,13 @@ private: int _nakInterval { -1 }; // NAK timeout interval, in microseconds, set on loss int _minNAKInterval { 100000 }; // NAK timeout interval lower bound, default of 100ms - p_high_resolution_clock::time_point _lastNAKTime; + p_high_resolution_clock::time_point _lastNAKTime = p_high_resolution_clock::time_point().min(); bool _hasReceivedHandshake { false }; // flag for receipt of handshake from server bool _hasReceivedHandshakeACK { false }; // flag for receipt of handshake ACK from client - p_high_resolution_clock::time_point _connectionStart; // holds the time_point for creation of this connection - p_high_resolution_clock::time_point _lastReceiveTime; // holds the last time we received anything from sender + p_high_resolution_clock::time_point _connectionStart = p_high_resolution_clock::time_point().min(); // holds the time_point for creation of this connection + p_high_resolution_clock::time_point _lastReceiveTime = p_high_resolution_clock::time_point().min(); // holds the last time we received anything from sender bool _isReceivingData { false }; // flag used for expiry of receipt portion of connection LossList _lossList; // List of all missing packets diff --git a/libraries/networking/src/udt/PacketTimeWindow.h b/libraries/networking/src/udt/PacketTimeWindow.h index c2a90d0f6c..6030e6809f 100644 --- a/libraries/networking/src/udt/PacketTimeWindow.h +++ b/libraries/networking/src/udt/PacketTimeWindow.h @@ -42,8 +42,8 @@ private: std::vector _packetIntervals; // vector of microsecond intervals between packet arrivals std::vector _probeIntervals; // vector of microsecond intervals between probe pair arrivals - p_high_resolution_clock::time_point _lastPacketTime; // the time_point when last packet arrived - p_high_resolution_clock::time_point _firstProbeTime; // the time_point when first probe in pair arrived + p_high_resolution_clock::time_point _lastPacketTime = p_high_resolution_clock::time_point().min(); // the time_point when last packet arrived + p_high_resolution_clock::time_point _firstProbeTime = p_high_resolution_clock::time_point().min(); // the time_point when first probe in pair arrived }; } diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 2890d52c2b..7f1590f995 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -281,7 +281,7 @@ void SendQueue::run() { // if it has been at least 100ms since we last sent a handshake, send another now // hold the time of last send in a static - static auto lastSendHandshake = p_high_resolution_clock::time_point(); + static auto lastSendHandshake = p_high_resolution_clock::time_point().min(); static const auto HANDSHAKE_RESEND_INTERVAL_MS = std::chrono::milliseconds(100); From a27e0e7cc7ca2dae9b9e08cc5f176ac4304aa767 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 14 Sep 2015 17:18:17 -0700 Subject: [PATCH 2/5] remove an unused alias --- libraries/networking/src/udt/SendQueue.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index 401ebd384d..2e7ec90c45 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -45,8 +45,6 @@ class SendQueue : public QObject { Q_OBJECT public: - using time_point = p_high_resolution_clock::time_point; - static std::unique_ptr create(Socket* socket, HifiSockAddr destination); void queuePacket(std::unique_ptr packet); From 6a186ad1fe420ad753fb24cfd38bec8dfba12055 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 15 Sep 2015 09:48:17 -0700 Subject: [PATCH 3/5] ensure a handshake goes out the first time --- libraries/networking/src/udt/SendQueue.cpp | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 7f1590f995..a09ea6ca9a 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -279,17 +279,13 @@ void SendQueue::run() { if (!_hasReceivedHandshakeACK) { // we haven't received a handshake ACK from the client // if it has been at least 100ms since we last sent a handshake, send another now - - // hold the time of last send in a static - static auto lastSendHandshake = p_high_resolution_clock::time_point().min(); - + static const auto HANDSHAKE_RESEND_INTERVAL_MS = std::chrono::milliseconds(100); - // calculation the duration since the last handshake send - auto sinceLastHandshake = std::chrono::duration_cast(p_high_resolution_clock::now() - - lastSendHandshake); + // hold the time of last send in a static + static auto lastSendHandshake = p_high_resolution_clock::now() - HANDSHAKE_RESEND_INTERVAL_MS; - if (sinceLastHandshake >= HANDSHAKE_RESEND_INTERVAL_MS) { + if (p_high_resolution_clock::now() - lastSendHandshake >= HANDSHAKE_RESEND_INTERVAL_MS) { // it has been long enough since last handshake, send another static auto handshakePacket = ControlPacket::create(ControlPacket::Handshake, 0); @@ -299,9 +295,7 @@ void SendQueue::run() { } // we wait for the ACK or the re-send interval to expire - _handshakeACKCondition.wait_until(handshakeLock, - p_high_resolution_clock::now() - + HANDSHAKE_RESEND_INTERVAL_MS); + _handshakeACKCondition.wait_until(handshakeLock, p_high_resolution_clock::now() + HANDSHAKE_RESEND_INTERVAL_MS); // Once we're here we've either received the handshake ACK or it's going to be time to re-send a handshake. // Either way let's continue processing - no packets will be sent if no handshake ACK has been received. From 6756d5364b9739a3995524f589413ec6fed250db Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 15 Sep 2015 10:11:05 -0700 Subject: [PATCH 4/5] sensible defaults for other time_point uses --- .../networking/src/udt/CongestionControl.cpp | 1 - libraries/networking/src/udt/CongestionControl.h | 2 +- libraries/networking/src/udt/Connection.cpp | 14 +++++++------- libraries/networking/src/udt/Connection.h | 7 ++++--- libraries/networking/src/udt/PacketTimeWindow.cpp | 15 +++++++++------ libraries/networking/src/udt/PacketTimeWindow.h | 4 ++-- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/libraries/networking/src/udt/CongestionControl.cpp b/libraries/networking/src/udt/CongestionControl.cpp index 2240f2c193..ea46d60acb 100644 --- a/libraries/networking/src/udt/CongestionControl.cpp +++ b/libraries/networking/src/udt/CongestionControl.cpp @@ -34,7 +34,6 @@ void CongestionControl::setPacketSendPeriod(double newSendPeriod) { } DefaultCC::DefaultCC() : - _lastRCTime(p_high_resolution_clock::now()), _slowStartLastAck(_sendCurrSeqNum), _lastDecreaseMaxSeq(SequenceNumber {SequenceNumber::MAX }) { diff --git a/libraries/networking/src/udt/CongestionControl.h b/libraries/networking/src/udt/CongestionControl.h index 5f9b64c1ad..fa1bf73ecf 100644 --- a/libraries/networking/src/udt/CongestionControl.h +++ b/libraries/networking/src/udt/CongestionControl.h @@ -108,7 +108,7 @@ public: private: void stopSlowStart(); // stops the slow start on loss or timeout - p_high_resolution_clock::time_point _lastRCTime = p_high_resolution_clock::time_point().min(); // last rate increase time + p_high_resolution_clock::time_point _lastRCTime = p_high_resolution_clock::now(); // last rate increase time bool _slowStart { true }; // if in slow start phase SequenceNumber _slowStartLastAck; // last ACKed seq num diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 4aa785fa8c..2fb28f81ee 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -28,7 +28,6 @@ using namespace udt; using namespace std::chrono; Connection::Connection(Socket* parentSocket, HifiSockAddr destination, std::unique_ptr congestionControl) : - _connectionStart(p_high_resolution_clock::now()), _parentSocket(parentSocket), _destination(destination), _congestionControl(move(congestionControl)) @@ -222,7 +221,7 @@ void Connection::recordRetransmission() { } void Connection::sendACK(bool wasCausedBySyncTimeout) { - static p_high_resolution_clock::time_point lastACKSendTime = p_high_resolution_clock::time_point().min(); + static p_high_resolution_clock::time_point lastACKSendTime; auto currentTime = p_high_resolution_clock::now(); SequenceNumber nextACKNumber = nextACK(); @@ -278,11 +277,11 @@ void Connection::sendACK(bool wasCausedBySyncTimeout) { // pack in the receive speed and estimatedBandwidth ackPacket->writePrimitive(packetReceiveSpeed); ackPacket->writePrimitive(estimatedBandwidth); - - // record this as the last ACK send time - lastACKSendTime = p_high_resolution_clock::now(); } + // record this as the last ACK send time + lastACKSendTime = p_high_resolution_clock::now(); + // have the socket send off our packet _parentSocket->writeBasePacket(*ackPacket, _destination); @@ -534,7 +533,8 @@ void Connection::processACK(std::unique_ptr controlPacket) { // This will be the case if it has been longer than the sync interval OR // it looks like they haven't received our ACK2 for this ACK auto currentTime = p_high_resolution_clock::now(); - static p_high_resolution_clock::time_point lastACK2SendTime = p_high_resolution_clock::time_point().min(); + static p_high_resolution_clock::time_point lastACK2SendTime = + p_high_resolution_clock::now() - std::chrono::microseconds(_synInterval); microseconds sinceLastACK2 = duration_cast(currentTime - lastACK2SendTime); @@ -779,7 +779,7 @@ void Connection::resetReceiveState() { // clear the loss list and _lastNAKTime _lossList.clear(); - _lastNAKTime = p_high_resolution_clock::time_point().min(); + _lastNAKTime = p_high_resolution_clock::now(); // the _nakInterval need not be reset, that will happen on loss diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index 346f559165..2b1dec1ae9 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -114,13 +114,14 @@ private: int _nakInterval { -1 }; // NAK timeout interval, in microseconds, set on loss int _minNAKInterval { 100000 }; // NAK timeout interval lower bound, default of 100ms - p_high_resolution_clock::time_point _lastNAKTime = p_high_resolution_clock::time_point().min(); + p_high_resolution_clock::time_point _lastNAKTime = p_high_resolution_clock::now(); bool _hasReceivedHandshake { false }; // flag for receipt of handshake from server bool _hasReceivedHandshakeACK { false }; // flag for receipt of handshake ACK from client - p_high_resolution_clock::time_point _connectionStart = p_high_resolution_clock::time_point().min(); // holds the time_point for creation of this connection - p_high_resolution_clock::time_point _lastReceiveTime = p_high_resolution_clock::time_point().min(); // holds the last time we received anything from sender + p_high_resolution_clock::time_point _connectionStart = p_high_resolution_clock::now(); // holds the time_point for creation of this connection + p_high_resolution_clock::time_point _lastReceiveTime; // holds the last time we received anything from sender + bool _isReceivingData { false }; // flag used for expiry of receipt portion of connection LossList _lossList; // List of all missing packets diff --git a/libraries/networking/src/udt/PacketTimeWindow.cpp b/libraries/networking/src/udt/PacketTimeWindow.cpp index 51bba2c52c..915810b93e 100644 --- a/libraries/networking/src/udt/PacketTimeWindow.cpp +++ b/libraries/networking/src/udt/PacketTimeWindow.cpp @@ -93,15 +93,18 @@ int32_t PacketTimeWindow::getEstimatedBandwidth() const { } void PacketTimeWindow::onPacketArrival() { + // take the current time auto now = p_high_resolution_clock::now(); - // record the interval between this packet and the last one - _packetIntervals[_currentPacketInterval++] = duration_cast(now - _lastPacketTime).count(); - - // reset the currentPacketInterval index when it wraps - if (_currentPacketInterval == _numPacketIntervals) { - _currentPacketInterval = 0; + if (_packetIntervals.size() > 0) { + // record the interval between this packet and the last one + _packetIntervals[_currentPacketInterval++] = duration_cast(now - _lastPacketTime).count(); + + // reset the currentPacketInterval index when it wraps + if (_currentPacketInterval == _numPacketIntervals) { + _currentPacketInterval = 0; + } } // remember this as the last packet arrival time diff --git a/libraries/networking/src/udt/PacketTimeWindow.h b/libraries/networking/src/udt/PacketTimeWindow.h index 6030e6809f..6f7ed9f70f 100644 --- a/libraries/networking/src/udt/PacketTimeWindow.h +++ b/libraries/networking/src/udt/PacketTimeWindow.h @@ -42,8 +42,8 @@ private: std::vector _packetIntervals; // vector of microsecond intervals between packet arrivals std::vector _probeIntervals; // vector of microsecond intervals between probe pair arrivals - p_high_resolution_clock::time_point _lastPacketTime = p_high_resolution_clock::time_point().min(); // the time_point when last packet arrived - p_high_resolution_clock::time_point _firstProbeTime = p_high_resolution_clock::time_point().min(); // the time_point when first probe in pair arrived + p_high_resolution_clock::time_point _lastPacketTime = p_high_resolution_clock::now(); // the time_point when last packet arrived + p_high_resolution_clock::time_point _firstProbeTime = p_high_resolution_clock::now(); // the time_point when first probe in pair arrived }; } From 8454cb916c4ec8a25f39fff85930d13a159da84b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 15 Sep 2015 15:15:26 -0700 Subject: [PATCH 5/5] avoid branching by using %= operator --- libraries/networking/src/udt/PacketTimeWindow.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/libraries/networking/src/udt/PacketTimeWindow.cpp b/libraries/networking/src/udt/PacketTimeWindow.cpp index 915810b93e..0c95d21bc6 100644 --- a/libraries/networking/src/udt/PacketTimeWindow.cpp +++ b/libraries/networking/src/udt/PacketTimeWindow.cpp @@ -102,9 +102,7 @@ void PacketTimeWindow::onPacketArrival() { _packetIntervals[_currentPacketInterval++] = duration_cast(now - _lastPacketTime).count(); // reset the currentPacketInterval index when it wraps - if (_currentPacketInterval == _numPacketIntervals) { - _currentPacketInterval = 0; - } + _currentPacketInterval %= _numPacketIntervals; } // remember this as the last packet arrival time @@ -123,7 +121,5 @@ void PacketTimeWindow::onProbePair2Arrival() { _probeIntervals[_currentProbeInterval++] = duration_cast(now - _firstProbeTime).count(); // reset the currentProbeInterval index when it wraps - if (_currentProbeInterval == _numProbeIntervals) { - _currentProbeInterval = 0; - } + _currentProbeInterval %= _numProbeIntervals; }