From 29723d0ef32d3dfc6467fea1cd68e4f2f1197eb5 Mon Sep 17 00:00:00 2001 From: wangyix Date: Wed, 18 Jun 2014 11:06:15 -0700 Subject: [PATCH] changed octree data nacks to repeatedly nack missing seq nums --- .../octree/OctreeInboundPacketProcessor.cpp | 17 ++-- interface/src/Application.cpp | 86 ++++++++++--------- interface/src/Application.h | 2 +- libraries/octree/src/OctreeSceneStats.cpp | 16 +--- libraries/octree/src/OctreeSceneStats.h | 6 +- 5 files changed, 58 insertions(+), 69 deletions(-) diff --git a/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp b/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp index 78ab9259fd..76a6845342 100644 --- a/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp +++ b/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp @@ -184,7 +184,8 @@ void OctreeInboundPacketProcessor::trackInboundPacket(const QUuid& nodeUUID, uns int OctreeInboundPacketProcessor::sendNackPackets() { int packetsSent = 0; - + char packet[MAX_PACKET_SIZE]; + NodeToSenderStatsMapIterator i = _singleSenderStats.begin(); while (i != _singleSenderStats.end()) { @@ -206,15 +207,10 @@ int OctreeInboundPacketProcessor::sendNackPackets() { const SharedNodePointer& destinationNode = NodeList::getInstance()->getNodeHash().value(nodeUUID); const QSet& missingSequenceNumbers = nodeStats.getMissingSequenceNumbers(); - - // check if there are any sequence numbers that need to be nacked - int numSequenceNumbersAvailable = missingSequenceNumbers.size(); - + // construct nack packet(s) for this node - - QSet::const_iterator missingSequenceNumberIterator = missingSequenceNumbers.begin(); - char packet[MAX_PACKET_SIZE]; - + int numSequenceNumbersAvailable = missingSequenceNumbers.size(); + QSet::const_iterator missingSequenceNumberIterator = missingSequenceNumbers.constBegin(); while (numSequenceNumbersAvailable > 0) { char* dataAt = packet; @@ -243,8 +239,7 @@ int OctreeInboundPacketProcessor::sendNackPackets() { numSequenceNumbersAvailable -= numSequenceNumbers; // send it - qint64 bytesWritten = NodeList::getInstance()->writeUnverifiedDatagram(packet, dataAt - packet, destinationNode); - + NodeList::getInstance()->writeUnverifiedDatagram(packet, dataAt - packet, destinationNode); packetsSent++; } i++; diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 02cf59ad87..efe98819bd 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2102,19 +2102,19 @@ void Application::updateMyAvatar(float deltaTime) { const quint64 TOO_LONG_SINCE_LAST_NACK = 1 * USECS_PER_SECOND; if (sinceLastNack > TOO_LONG_SINCE_LAST_NACK) { _lastNackTime = now; - sendNack(); + sendNackPackets(); } } } -void Application::sendNack() { +int Application::sendNackPackets() { if (Menu::getInstance()->isOptionChecked(MenuOption::DisableNackPackets)) { - return; + return 0; } + int packetsSent = 0; char packet[MAX_PACKET_SIZE]; - NodeList* nodeList = NodeList::getInstance(); // iterates thru all nodes in NodeList foreach(const SharedNodePointer& node, NodeList::getInstance()->getNodeHash()) { @@ -2125,14 +2125,14 @@ void Application::sendNack() { || node->getType() == NodeType::ModelServer) ) { - // if there are octree packets from this node that are waiting to be processed, - // don't send a NACK since the missing packets may be among those waiting packets. - if (_octreeProcessor.hasPacketsToProcessFrom(node)) { - continue; - } - QUuid nodeUUID = node->getUUID(); + // if there are octree packets from this node that are waiting to be processed, + // don't send a NACK since the missing packets may be among those waiting packets. + if (_octreeProcessor.hasPacketsToProcessFrom(nodeUUID)) { + continue; + } + _octreeSceneStatsLock.lockForRead(); // retreive octree scene stats of this node @@ -2142,40 +2142,48 @@ void Application::sendNack() { } OctreeSceneStats& stats = _octreeServerSceneStats[nodeUUID]; - // check if there are any sequence numbers that need to be nacked - int numSequenceNumbersAvailable = stats.getNumSequenceNumbersToNack(); - if (numSequenceNumbersAvailable == 0) { - _octreeSceneStatsLock.unlock(); - continue; - } + // make copy of missing sequence numbers from stats + const QSet missingSequenceNumbers = stats.getMissingSequenceNumbers(); - char* dataAt = packet; - int bytesRemaining = MAX_PACKET_SIZE; - - // pack header - int numBytesPacketHeader = populatePacketHeader(packet, PacketTypeOctreeDataNack); - dataAt += numBytesPacketHeader; - bytesRemaining -= numBytesPacketHeader; - int numSequenceNumbersRoomFor = (bytesRemaining - sizeof(uint16_t)) / sizeof(OCTREE_PACKET_SEQUENCE); - - // calculate and pack the number of sequence numbers - uint16_t numSequenceNumbers = min(numSequenceNumbersAvailable, numSequenceNumbersRoomFor); - uint16_t* numSequenceNumbersAt = (uint16_t*)dataAt; - *numSequenceNumbersAt = numSequenceNumbers; - dataAt += sizeof(uint16_t); - - // pack sequence numbers - for (int i = 0; i < numSequenceNumbers; i++) { - OCTREE_PACKET_SEQUENCE* sequenceNumberAt = (OCTREE_PACKET_SEQUENCE*)dataAt; - *sequenceNumberAt = stats.getNextSequenceNumberToNack(); - dataAt += sizeof(OCTREE_PACKET_SEQUENCE); - } - _octreeSceneStatsLock.unlock(); - nodeList->writeUnverifiedDatagram(packet, dataAt - packet, node); + // construct nack packet(s) for this node + int numSequenceNumbersAvailable = missingSequenceNumbers.size(); + QSet::const_iterator missingSequenceNumbersIterator = missingSequenceNumbers.constBegin(); + while (numSequenceNumbersAvailable > 0) { + + char* dataAt = packet; + int bytesRemaining = MAX_PACKET_SIZE; + + // pack header + int numBytesPacketHeader = populatePacketHeader(packet, PacketTypeOctreeDataNack); + dataAt += numBytesPacketHeader; + bytesRemaining -= numBytesPacketHeader; + + // calculate and pack the number of sequence numbers + int numSequenceNumbersRoomFor = (bytesRemaining - sizeof(uint16_t)) / sizeof(OCTREE_PACKET_SEQUENCE); + uint16_t numSequenceNumbers = min(numSequenceNumbersAvailable, numSequenceNumbersRoomFor); + uint16_t* numSequenceNumbersAt = (uint16_t*)dataAt; + *numSequenceNumbersAt = numSequenceNumbers; + dataAt += sizeof(uint16_t); + + // pack sequence numbers + for (int i = 0; i < numSequenceNumbers; i++) { + OCTREE_PACKET_SEQUENCE* sequenceNumberAt = (OCTREE_PACKET_SEQUENCE*)dataAt; + *sequenceNumberAt = *missingSequenceNumbersIterator; + dataAt += sizeof(OCTREE_PACKET_SEQUENCE); + + missingSequenceNumbersIterator++; + } + numSequenceNumbersAvailable -= numSequenceNumbers; + + // send it + NodeList::getInstance()->writeUnverifiedDatagram(packet, dataAt - packet, node); + packetsSent++; + } } } + return packetsSent; } void Application::queryOctree(NodeType_t serverType, PacketType packetType, NodeToJurisdictionMap& jurisdictions) { diff --git a/interface/src/Application.h b/interface/src/Application.h index 2889dcb301..e63c7c35b8 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -411,7 +411,7 @@ private: static void attachNewHeadToNode(Node *newNode); static void* networkReceive(void* args); // network receive thread - void sendNack(); + int sendNackPackets(); MainWindow* _window; GLCanvas* _glWidget; // our GLCanvas has a couple extra features diff --git a/libraries/octree/src/OctreeSceneStats.cpp b/libraries/octree/src/OctreeSceneStats.cpp index d037ec79ad..1d6185fb3e 100644 --- a/libraries/octree/src/OctreeSceneStats.cpp +++ b/libraries/octree/src/OctreeSceneStats.cpp @@ -47,7 +47,6 @@ OctreeSceneStats::OctreeSceneStats() : _incomingReallyLate(0), _incomingPossibleDuplicate(0), _missingSequenceNumbers(), - _sequenceNumbersToNack(), _incomingFlightTimeAverage(samples), _jurisdictionRoot(NULL) { @@ -160,7 +159,6 @@ void OctreeSceneStats::copyFromOther(const OctreeSceneStats& other) { _incomingPossibleDuplicate = other._incomingPossibleDuplicate; _missingSequenceNumbers = other._missingSequenceNumbers; - _sequenceNumbersToNack = other._sequenceNumbersToNack; } @@ -946,7 +944,6 @@ void OctreeSceneStats::trackIncomingOctreePacket(const QByteArray& packet, qDebug() << "found it in _missingSequenceNumbers"; } _missingSequenceNumbers.remove(sequence); - _sequenceNumbersToNack.remove(sequence); _incomingLikelyLost--; _incomingRecovered++; } @@ -986,7 +983,6 @@ void OctreeSceneStats::trackIncomingOctreePacket(const QByteArray& packet, for (int missingSequenceInt = expectedInt; missingSequenceInt < sequenceInt; missingSequenceInt++) { OCTREE_PACKET_SEQUENCE missingSequence = missingSequenceInt >= 0 ? missingSequenceInt : missingSequenceInt + UINT16_RANGE; _missingSequenceNumbers << missingSequence; - _sequenceNumbersToNack << missingSequence; } _incomingLastSequence = sequence; @@ -1025,7 +1021,6 @@ void OctreeSceneStats::trackIncomingOctreePacket(const QByteArray& packet, qDebug() << "pruning really old missing sequence:" << missingItem; } _missingSequenceNumbers.remove(missingItem); - _sequenceNumbersToNack.remove(missingItem); } } } @@ -1038,13 +1033,6 @@ void OctreeSceneStats::trackIncomingOctreePacket(const QByteArray& packet, } } -int OctreeSceneStats::getNumSequenceNumbersToNack() const { - return _sequenceNumbersToNack.size(); -} - -uint16_t OctreeSceneStats::getNextSequenceNumberToNack() { - QSet::Iterator it = _sequenceNumbersToNack.begin(); - uint16_t sequenceNumber = *it; - _sequenceNumbersToNack.remove(sequenceNumber); - return sequenceNumber; +const QSet& OctreeSceneStats::getMissingSequenceNumbers() const { + return _missingSequenceNumbers; } diff --git a/libraries/octree/src/OctreeSceneStats.h b/libraries/octree/src/OctreeSceneStats.h index 182bd6c86c..088eb787f8 100644 --- a/libraries/octree/src/OctreeSceneStats.h +++ b/libraries/octree/src/OctreeSceneStats.h @@ -172,9 +172,8 @@ public: quint32 getIncomingReallyLate() const { return _incomingReallyLate; } quint32 getIncomingPossibleDuplicate() const { return _incomingPossibleDuplicate; } float getIncomingFlightTimeAverage() { return _incomingFlightTimeAverage.getAverage(); } - - int getNumSequenceNumbersToNack() const; - OCTREE_PACKET_SEQUENCE getNextSequenceNumberToNack(); + + const QSet& getMissingSequenceNumbers() const; private: @@ -277,7 +276,6 @@ private: quint32 _incomingReallyLate; /// out of order and later than MAX_MISSING_SEQUENCE_OLD_AGE late quint32 _incomingPossibleDuplicate; /// out of order possibly a duplicate QSet _missingSequenceNumbers; - QSet _sequenceNumbersToNack; SimpleMovingAverage _incomingFlightTimeAverage; // features related items