From 2ec04fb7563273adfd853e5fe6a421ae44c08126 Mon Sep 17 00:00:00 2001 From: wangyix Date: Wed, 25 Jun 2014 17:20:57 -0700 Subject: [PATCH] replaced old seqnum tracking code with SequenceNumberStats --- .../octree/OctreeInboundPacketProcessor.cpp | 75 +------- .../src/octree/OctreeInboundPacketProcessor.h | 12 +- interface/src/Application.cpp | 2 +- interface/src/ui/OctreeStatsDialog.cpp | 17 +- .../networking/src/SequenceNumbersStats.cpp | 5 +- .../networking/src/SequenceNumbersStats.h | 3 + libraries/octree/src/OctreeSceneStats.cpp | 171 +----------------- libraries/octree/src/OctreeSceneStats.h | 20 +- tests/audio/src/main.cpp | 1 + 9 files changed, 37 insertions(+), 269 deletions(-) diff --git a/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp b/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp index 76a6845342..eaea2650b8 100644 --- a/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp +++ b/assignment-client/src/octree/OctreeInboundPacketProcessor.cpp @@ -206,7 +206,7 @@ int OctreeInboundPacketProcessor::sendNackPackets() { } const SharedNodePointer& destinationNode = NodeList::getInstance()->getNodeHash().value(nodeUUID); - const QSet& missingSequenceNumbers = nodeStats.getMissingSequenceNumbers(); + const QSet& missingSequenceNumbers = nodeStats.getSequenceNumberStats().getMissingSet(); // construct nack packet(s) for this node int numSequenceNumbersAvailable = missingSequenceNumbers.size(); @@ -254,8 +254,7 @@ SingleSenderStats::SingleSenderStats() _totalLockWaitTime(0), _totalElementsInPacket(0), _totalPackets(0), - _incomingLastSequence(0), - _missingSequenceNumbers() + _sequenceNumberStats() { } @@ -263,74 +262,8 @@ SingleSenderStats::SingleSenderStats() void SingleSenderStats::trackInboundPacket(unsigned short int incomingSequence, quint64 transitTime, int editsInPacket, quint64 processTime, quint64 lockWaitTime) { - const int UINT16_RANGE = std::numeric_limits::max() + 1; - const int MAX_REASONABLE_SEQUENCE_GAP = 1000; // this must be less than UINT16_RANGE / 2 for rollover handling to work - const int MAX_MISSING_SEQUENCE_SIZE = 100; - - unsigned short int expectedSequence = _totalPackets == 0 ? incomingSequence : _incomingLastSequence + (unsigned short int)1; - - if (incomingSequence == expectedSequence) { // on time - _incomingLastSequence = incomingSequence; - } else { // out of order - int incoming = (int)incomingSequence; - int expected = (int)expectedSequence; - - // check if the gap between incoming and expected is reasonable, taking possible rollover into consideration - int absGap = std::abs(incoming - expected); - if (absGap >= UINT16_RANGE - MAX_REASONABLE_SEQUENCE_GAP) { - // rollover likely occurred between incoming and expected. - // correct the larger of the two so that it's within [-UINT16_RANGE, -1] while the other remains within [0, UINT16_RANGE-1] - if (incoming > expected) { - incoming -= UINT16_RANGE; - } else { - expected -= UINT16_RANGE; - } - } else if (absGap > MAX_REASONABLE_SEQUENCE_GAP) { - // ignore packet if gap is unreasonable - qDebug() << "ignoring unreasonable packet... sequence:" << incomingSequence - << "_incomingLastSequence:" << _incomingLastSequence; - return; - } - - // now that rollover has been corrected for (if it occurred), incoming and expected can be - // compared to each other directly, though one of them might be negative - if (incoming > expected) { // early - // add all sequence numbers that were skipped to the missing sequence numbers list - for (int missingSequence = expected; missingSequence < incoming; missingSequence++) { - _missingSequenceNumbers.insert(missingSequence < 0 ? missingSequence + UINT16_RANGE : missingSequence); - } - _incomingLastSequence = incomingSequence; - } else { // late - // remove this from missing sequence number if it's in there - _missingSequenceNumbers.remove(incomingSequence); - - // do not update _incomingLastSequence; it shouldn't become smaller - } - } - - // prune missing sequence list if it gets too big; sequence numbers that are older than MAX_REASONABLE_SEQUENCE_GAP - // will be removed. - if (_missingSequenceNumbers.size() > MAX_MISSING_SEQUENCE_SIZE) { - // some older sequence numbers may be from before a rollover point; this must be handled. - // some sequence numbers in this list may be larger than _incomingLastSequence, indicating that they were received - // before the most recent rollover. - int cutoff = (int)_incomingLastSequence - MAX_REASONABLE_SEQUENCE_GAP; - if (cutoff >= 0) { - foreach(unsigned short int missingSequence, _missingSequenceNumbers) { - unsigned short int nonRolloverCutoff = (unsigned short int)cutoff; - if (missingSequence > _incomingLastSequence || missingSequence <= nonRolloverCutoff) { - _missingSequenceNumbers.remove(missingSequence); - } - } - } else { - unsigned short int rolloverCutoff = (unsigned short int)(cutoff + UINT16_RANGE); - foreach(unsigned short int missingSequence, _missingSequenceNumbers) { - if (missingSequence > _incomingLastSequence && missingSequence <= rolloverCutoff) { - _missingSequenceNumbers.remove(missingSequence); - } - } - } - } + // track sequence number + _sequenceNumberStats.sequenceNumberReceived(incomingSequence); // update other stats _totalTransitTime += transitTime; diff --git a/assignment-client/src/octree/OctreeInboundPacketProcessor.h b/assignment-client/src/octree/OctreeInboundPacketProcessor.h index 46a57205cb..1c4f00c0a9 100644 --- a/assignment-client/src/octree/OctreeInboundPacketProcessor.h +++ b/assignment-client/src/octree/OctreeInboundPacketProcessor.h @@ -14,9 +14,10 @@ #ifndef hifi_OctreeInboundPacketProcessor_h #define hifi_OctreeInboundPacketProcessor_h -#include - #include + +#include "SequenceNumbersStats.h" + class OctreeServer; class SingleSenderStats { @@ -32,7 +33,8 @@ public: { return _totalElementsInPacket == 0 ? 0 : _totalProcessTime / _totalElementsInPacket; } quint64 getAverageLockWaitTimePerElement() const { return _totalElementsInPacket == 0 ? 0 : _totalLockWaitTime / _totalElementsInPacket; } - const QSet& getMissingSequenceNumbers() const { return _missingSequenceNumbers; } + + const SequenceNumberStats& getSequenceNumberStats() const { return _sequenceNumberStats; } void trackInboundPacket(unsigned short int incomingSequence, quint64 transitTime, int editsInPacket, quint64 processTime, quint64 lockWaitTime); @@ -42,9 +44,7 @@ public: quint64 _totalLockWaitTime; quint64 _totalElementsInPacket; quint64 _totalPackets; - - unsigned short int _incomingLastSequence; - QSet _missingSequenceNumbers; + SequenceNumberStats _sequenceNumberStats; }; typedef QHash NodeToSenderStatsMap; diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 75c3f442ca..f40185f34f 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2164,7 +2164,7 @@ int Application::sendNackPackets() { OctreeSceneStats& stats = _octreeServerSceneStats[nodeUUID]; // make copy of missing sequence numbers from stats - const QSet missingSequenceNumbers = stats.getMissingSequenceNumbers(); + const QSet missingSequenceNumbers = stats.getSequenceNumberStats().getMissingSet(); _octreeSceneStatsLock.unlock(); diff --git a/interface/src/ui/OctreeStatsDialog.cpp b/interface/src/ui/OctreeStatsDialog.cpp index 3296d8ccb2..6303ff25d1 100644 --- a/interface/src/ui/OctreeStatsDialog.cpp +++ b/interface/src/ui/OctreeStatsDialog.cpp @@ -365,13 +365,14 @@ void OctreeStatsDialog::showOctreeServersOfType(int& serverCount, NodeType_t ser QString incomingPacketsString = locale.toString((uint)stats.getIncomingPackets()); QString incomingBytesString = locale.toString((uint)stats.getIncomingBytes()); QString incomingWastedBytesString = locale.toString((uint)stats.getIncomingWastedBytes()); - QString incomingOutOfOrderString = locale.toString((uint)stats.getIncomingOutOfOrder()); - QString incomingLateString = locale.toString((uint)stats.getIncomingLate()); - QString incomingReallyLateString = locale.toString((uint)stats.getIncomingReallyLate()); - QString incomingEarlyString = locale.toString((uint)stats.getIncomingEarly()); - QString incomingLikelyLostString = locale.toString((uint)stats.getIncomingLikelyLost()); - QString incomingRecovered = locale.toString((uint)stats.getIncomingRecovered()); - QString incomingDuplicateString = locale.toString((uint)stats.getIncomingPossibleDuplicate()); + const SequenceNumberStats& seqStats = stats.getSequenceNumberStats(); + QString incomingOutOfOrderString = locale.toString((uint)seqStats.getNumOutOfOrder()); + QString incomingLateString = locale.toString((uint)seqStats.getNumLate()); + QString incomingUnreasonableString = locale.toString((uint)seqStats.getNumUnreasonable()); + QString incomingEarlyString = locale.toString((uint)seqStats.getNumEarly()); + QString incomingLikelyLostString = locale.toString((uint)seqStats.getNumLost()); + QString incomingRecovered = locale.toString((uint)seqStats.getNumRecovered()); + QString incomingDuplicateString = locale.toString((uint)seqStats.getNumDuplicate()); int clockSkewInMS = node->getClockSkewUsec() / (int)USECS_PER_MSEC; QString incomingFlightTimeString = locale.toString((int)stats.getIncomingFlightTimeAverage()); @@ -385,7 +386,7 @@ void OctreeStatsDialog::showOctreeServersOfType(int& serverCount, NodeType_t ser serverDetails << "
" << " Out of Order: " << qPrintable(incomingOutOfOrderString) << "/ Early: " << qPrintable(incomingEarlyString) << "/ Late: " << qPrintable(incomingLateString) << - "/ Really Late: " << qPrintable(incomingReallyLateString) << + "/ Unreasonable: " << qPrintable(incomingUnreasonableString) << "/ Duplicate: " << qPrintable(incomingDuplicateString); serverDetails << "
" << diff --git a/libraries/networking/src/SequenceNumbersStats.cpp b/libraries/networking/src/SequenceNumbersStats.cpp index 97a9ed6b03..b1b6adc9ac 100644 --- a/libraries/networking/src/SequenceNumbersStats.cpp +++ b/libraries/networking/src/SequenceNumbersStats.cpp @@ -17,6 +17,7 @@ SequenceNumberStats::SequenceNumberStats() : _lastReceived(std::numeric_limits::max()), _missingSet(), _numReceived(0), + _numUnreasonable(0), _numEarly(0), _numLate(0), _numLost(0), @@ -30,6 +31,8 @@ void SequenceNumberStats::sequenceNumberReceived(quint16 incoming, const bool wa static const int UINT16_RANGE = std::numeric_limits::max() + 1; static const int MAX_REASONABLE_SEQUENCE_GAP = 1000; // this must be less than UINT16_RANGE / 2 for rollover handling to work + _numReceived++; + // determine our expected sequence number... handle rollover appropriately quint16 expected = _numReceived > 0 ? _lastReceived + (quint16)1 : incoming; @@ -58,6 +61,7 @@ void SequenceNumberStats::sequenceNumberReceived(quint16 incoming, const bool wa // ignore packet if gap is unreasonable qDebug() << "ignoring unreasonable packet... sequence:" << incoming << "previous:" << _lastReceived; + _numUnreasonable++; return; } @@ -100,7 +104,6 @@ void SequenceNumberStats::sequenceNumberReceived(quint16 incoming, const bool wa // do not update _incomingLastSequence; it shouldn't become smaller } } - _numReceived++; // prune missing sequence list if it gets too big; sequence numbers that are older than MAX_REASONABLE_SEQUENCE_GAP // will be removed. diff --git a/libraries/networking/src/SequenceNumbersStats.h b/libraries/networking/src/SequenceNumbersStats.h index 3e9ef6f0bc..8dbb876a5b 100644 --- a/libraries/networking/src/SequenceNumbersStats.h +++ b/libraries/networking/src/SequenceNumbersStats.h @@ -21,6 +21,8 @@ public: void sequenceNumberReceived(quint16 incoming, const bool wantExtraDebugging = false); quint32 getNumReceived() const { return _numReceived; } + quint32 getNumUnreasonable() const { return _numUnreasonable; } + quint32 getNumOutOfOrder() const { return _numEarly + _numLate; } quint32 getNumEarly() const { return _numEarly; } quint32 getNumLate() const { return _numLate; } quint32 getNumLost() const { return _numLost; } @@ -33,6 +35,7 @@ private: QSet _missingSet; quint32 _numReceived; + quint32 _numUnreasonable; quint32 _numEarly; quint32 _numLate; quint32 _numLost; diff --git a/libraries/octree/src/OctreeSceneStats.cpp b/libraries/octree/src/OctreeSceneStats.cpp index 28445ec327..5fbc4b0945 100644 --- a/libraries/octree/src/OctreeSceneStats.cpp +++ b/libraries/octree/src/OctreeSceneStats.cpp @@ -39,14 +39,7 @@ OctreeSceneStats::OctreeSceneStats() : _incomingPacket(0), _incomingBytes(0), _incomingWastedBytes(0), - _incomingLastSequence(0), - _incomingLikelyLost(0), - _incomingRecovered(0), - _incomingEarly(0), - _incomingLate(0), - _incomingReallyLate(0), - _incomingPossibleDuplicate(0), - _missingSequenceNumbers(), + _sequenceNumberStats(), _incomingFlightTimeAverage(samples), _jurisdictionRoot(NULL) { @@ -150,15 +143,8 @@ void OctreeSceneStats::copyFromOther(const OctreeSceneStats& other) { _incomingPacket = other._incomingPacket; _incomingBytes = other._incomingBytes; _incomingWastedBytes = other._incomingWastedBytes; - _incomingLastSequence = other._incomingLastSequence; - _incomingLikelyLost = other._incomingLikelyLost; - _incomingRecovered = other._incomingRecovered; - _incomingEarly = other._incomingEarly; - _incomingLate = other._incomingLate; - _incomingReallyLate = other._incomingReallyLate; - _incomingPossibleDuplicate = other._incomingPossibleDuplicate; - - _missingSequenceNumbers = other._missingSequenceNumbers; + + _sequenceNumberStats = other._sequenceNumberStats; } @@ -875,155 +861,8 @@ void OctreeSceneStats::trackIncomingOctreePacket(const QByteArray& packet, qDebug() << "ignoring unreasonable packet... flightTime:" << flightTime; return; // ignore any packets that are unreasonable } - - const int UINT16_RANGE = std::numeric_limits::max() + 1; - - // determine our expected sequence number... handle rollover appropriately - OCTREE_PACKET_SEQUENCE expected = _incomingPacket > 0 ? _incomingLastSequence + (quint16)1 : sequence; - - const int USECS_PER_MSEC = 1000; - float flightTimeMsecs = flightTime / USECS_PER_MSEC; - _incomingFlightTimeAverage.updateAverage(flightTimeMsecs); - - // track out of order and possibly lost packets... - if (sequence == _incomingLastSequence) { - if (wantExtraDebugging) { - qDebug() << "last packet duplicate got:" << sequence << "_incomingLastSequence:" << _incomingLastSequence; - } - } else { - if (sequence != expected) { - if (wantExtraDebugging) { - qDebug() << "out of order... got:" << sequence << "expected:" << expected; - } - - int sequenceInt = (int)sequence; - int expectedInt = (int)expected; - - // if distance between sequence and expected are more than half of the total range of possible seq numbers, - // assume that a rollover occurred between the two. - // correct the larger one so it's in the range [-UINT16_RANGE, -1] while the other remains in [0, UINT16_RANGE-1] - // after doing so, sequenceInt and expectedInt can be correctly compared to each other, though one may be negative - if (std::abs(sequenceInt - expectedInt) > UINT16_RANGE / 2) { - if (sequenceInt > expectedInt) { - sequenceInt -= UINT16_RANGE; - } - else { - expectedInt -= UINT16_RANGE; - } - } - - // Guard against possible corrupted packets... with bad sequence numbers - const int MAX_RESONABLE_SEQUENCE_OFFSET = 2000; - const int MIN_RESONABLE_SEQUENCE_OFFSET = -2000; - - int sequenceOffset = (sequenceInt - expectedInt); - if (sequenceOffset > MAX_RESONABLE_SEQUENCE_OFFSET || sequenceOffset < MIN_RESONABLE_SEQUENCE_OFFSET) { - qDebug() << "ignoring unreasonable packet... sequence:" << sequence << "_incomingLastSequence:" << _incomingLastSequence; - return; // ignore any packets that are unreasonable - } - - // if the sequence is less than our expected, then this might be a packet - // that was delayed and so we should find it in our lostSequence list - if (sequenceInt < expectedInt) { - - // if no rollover between them: sequenceInt, expectedInt are both in range [0, UINT16_RANGE-1] - // if rollover between them: sequenceInt in [-UINT16_RANGE, -1], expectedInt in [0, UINT16_RANGE-1] - - if (wantExtraDebugging) { - qDebug() << "this packet is later than expected..."; - } - if (sequenceInt < expectedInt - MAX_MISSING_SEQUENCE_OLD_AGE) { - _incomingReallyLate++; - } - else { - _incomingLate++; - } - - if (_missingSequenceNumbers.contains(sequence)) { - if (wantExtraDebugging) { - qDebug() << "found it in _missingSequenceNumbers"; - } - _missingSequenceNumbers.remove(sequence); - _incomingLikelyLost--; - _incomingRecovered++; - } - else { - // if we're still in our pruning window, and we didn't find it in our missing list, - // than this is really unexpected and can probably only happen if the packet was a - // duplicate - if (sequenceInt >= expectedInt - MAX_MISSING_SEQUENCE_OLD_AGE) { - if (wantExtraDebugging) { - qDebug() << "sequence:" << sequence << "WAS NOT found in _missingSequenceNumbers, and not that old... (expected - MAX_MISSING_SEQUENCE_OLD_AGE):" - << (uint16_t)(expectedInt - MAX_MISSING_SEQUENCE_OLD_AGE); - } - _incomingPossibleDuplicate++; - } - } - - // don't update _incomingLastSequence in this case. - // only bump the last sequence if it was greater than our expected sequence, this will keep us from - // accidentally going backwards when an out of order (recovered) packet comes in - - } else { // sequenceInt > expectedInt - - // if no rollover between them: sequenceInt, expectedInt are both in range [0, UINT16_RANGE-1] - // if rollover between them: sequenceInt in [0, UINT16_RANGE-1], expectedInt in [-UINT16_RANGE, -1] - - if (wantExtraDebugging) { - qDebug() << "this packet is earlier than expected..."; - } - _incomingEarly++; - - // hmm... so, we either didn't get some packets, or this guy came early... - int missing = sequenceInt - expectedInt; - if (wantExtraDebugging) { - qDebug() << ">>>>>>>> missing gap=" << missing; - } - _incomingLikelyLost += missing; - for (int missingSequenceInt = expectedInt; missingSequenceInt < sequenceInt; missingSequenceInt++) { - OCTREE_PACKET_SEQUENCE missingSequence = missingSequenceInt >= 0 ? missingSequenceInt : missingSequenceInt + UINT16_RANGE; - _missingSequenceNumbers << missingSequence; - } - - _incomingLastSequence = sequence; - } - } else { // sequence = expected - - _incomingLastSequence = sequence; - } - } - - // do some garbage collecting on our _missingSequenceNumbers - if (_missingSequenceNumbers.size() > MAX_MISSING_SEQUENCE) { - if (wantExtraDebugging) { - qDebug() << "too many _missingSequenceNumbers:" << _missingSequenceNumbers.size(); - } - - int oldAgeCutoff = (int)_incomingLastSequence - MAX_MISSING_SEQUENCE_OLD_AGE; - - foreach(uint16_t missingItem, _missingSequenceNumbers) { - if (wantExtraDebugging) { - qDebug() << "checking item:" << missingItem << "is it in need of pruning?"; - qDebug() << "(_incomingLastSequence - MAX_MISSING_SEQUENCE_OLD_AGE):" - << (uint16_t)((int)_incomingLastSequence - MAX_MISSING_SEQUENCE_OLD_AGE); - } - - bool prune; - if (oldAgeCutoff >= 0) { - prune = (missingItem <= oldAgeCutoff || missingItem > _incomingLastSequence); - } - else { - prune = (missingItem <= oldAgeCutoff + UINT16_RANGE && missingItem > _incomingLastSequence); - } - - if (prune) { - if (wantExtraDebugging) { - qDebug() << "pruning really old missing sequence:" << missingItem; - } - _missingSequenceNumbers.remove(missingItem); - } - } - } + + _sequenceNumberStats.sequenceNumberReceived(sequence); // track packets here... _incomingPacket++; diff --git a/libraries/octree/src/OctreeSceneStats.h b/libraries/octree/src/OctreeSceneStats.h index 1c468a8dc6..173f4c225f 100644 --- a/libraries/octree/src/OctreeSceneStats.h +++ b/libraries/octree/src/OctreeSceneStats.h @@ -17,6 +17,7 @@ #include #include "JurisdictionMap.h" #include "OctreePacketData.h" +#include "SequenceNumbersStats.h" #define GREENISH 0x40ff40d0 #define YELLOWISH 0xffef40c0 @@ -164,16 +165,9 @@ public: quint32 getIncomingPackets() const { return _incomingPacket; } quint64 getIncomingBytes() const { return _incomingBytes; } quint64 getIncomingWastedBytes() const { return _incomingWastedBytes; } - quint32 getIncomingOutOfOrder() const { return _incomingLate + _incomingEarly; } - quint32 getIncomingLikelyLost() const { return _incomingLikelyLost; } - quint32 getIncomingRecovered() const { return _incomingRecovered; } - quint32 getIncomingEarly() const { return _incomingEarly; } - quint32 getIncomingLate() const { return _incomingLate; } - quint32 getIncomingReallyLate() const { return _incomingReallyLate; } - quint32 getIncomingPossibleDuplicate() const { return _incomingPossibleDuplicate; } float getIncomingFlightTimeAverage() { return _incomingFlightTimeAverage.getAverage(); } - const QSet& getMissingSequenceNumbers() const { return _missingSequenceNumbers; } + const SequenceNumberStats& getSequenceNumberStats() const { return _sequenceNumberStats; } private: @@ -268,14 +262,8 @@ private: quint64 _incomingBytes; quint64 _incomingWastedBytes; - quint16 _incomingLastSequence; /// last incoming sequence number - quint32 _incomingLikelyLost; /// count of packets likely lost, may be off by _incomingReallyLate count - quint32 _incomingRecovered; /// packets that were late, and we had in our missing list, we consider recovered - quint32 _incomingEarly; /// out of order earlier than expected - quint32 _incomingLate; /// out of order later than expected - quint32 _incomingReallyLate; /// out of order and later than MAX_MISSING_SEQUENCE_OLD_AGE late - quint32 _incomingPossibleDuplicate; /// out of order possibly a duplicate - QSet _missingSequenceNumbers; + SequenceNumberStats _sequenceNumberStats; + SimpleMovingAverage _incomingFlightTimeAverage; // features related items diff --git a/tests/audio/src/main.cpp b/tests/audio/src/main.cpp index bf798982a9..10f1a2e522 100644 --- a/tests/audio/src/main.cpp +++ b/tests/audio/src/main.cpp @@ -13,6 +13,7 @@ int main(int argc, char** argv) { AudioRingBufferTests::runAllTests(); + printf("all tests passed. press enter to exit\n"); getchar(); return 0; }