From 0d14e0bcf7be99a345347bbd95c5da2e708b9f5a Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 17 Dec 2018 14:07:21 -0800 Subject: [PATCH 01/17] Set minimum timeout for retransmits --- libraries/networking/src/udt/SendQueue.cpp | 11 +++++------ libraries/networking/src/udt/SendQueue.h | 3 +++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/libraries/networking/src/udt/SendQueue.cpp b/libraries/networking/src/udt/SendQueue.cpp index 3178217a36..262b62de12 100644 --- a/libraries/networking/src/udt/SendQueue.cpp +++ b/libraries/networking/src/udt/SendQueue.cpp @@ -61,6 +61,9 @@ private: Mutex2& _mutex2; }; +const microseconds SendQueue::MAXIMUM_ESTIMATED_TIMEOUT = seconds(5); +const microseconds SendQueue::MINIMUM_ESTIMATED_TIMEOUT = milliseconds(10); + std::unique_ptr SendQueue::create(Socket* socket, HifiSockAddr destination, SequenceNumber currentSequenceNumber, MessageNumber currentMessageNumber, bool hasReceivedHandshakeACK) { Q_ASSERT_X(socket, "SendQueue::create", "Must be called with a valid Socket*"); @@ -505,12 +508,8 @@ bool SendQueue::isInactive(bool attemptedToSendPacket) { auto estimatedTimeout = std::chrono::microseconds(_estimatedTimeout); - // cap our maximum estimated timeout to the already unreasonable 5 seconds - const auto MAXIMUM_ESTIMATED_TIMEOUT = std::chrono::seconds(5); - - if (estimatedTimeout > MAXIMUM_ESTIMATED_TIMEOUT) { - estimatedTimeout = MAXIMUM_ESTIMATED_TIMEOUT; - } + // Clamp timeout beween 10 ms and 5 s + estimatedTimeout = std::min(MAXIMUM_ESTIMATED_TIMEOUT, std::max(MINIMUM_ESTIMATED_TIMEOUT, estimatedTimeout)); // use our condition_variable_any to wait auto cvStatus = _emptyCondition.wait_for(locker, estimatedTimeout); diff --git a/libraries/networking/src/udt/SendQueue.h b/libraries/networking/src/udt/SendQueue.h index c1faac3b22..e5c0e370c4 100644 --- a/libraries/networking/src/udt/SendQueue.h +++ b/libraries/networking/src/udt/SendQueue.h @@ -140,6 +140,9 @@ private: std::condition_variable_any _emptyCondition; std::chrono::high_resolution_clock::time_point _lastPacketSentAt; + + static const std::chrono::microseconds MAXIMUM_ESTIMATED_TIMEOUT; + static const std::chrono::microseconds MINIMUM_ESTIMATED_TIMEOUT; }; } From 3a282045ff60d766bc96a5856b32bf147b37ba5d Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Fri, 21 Dec 2018 15:48:58 -0800 Subject: [PATCH 02/17] Checkpoint trait-level flow control --- assignment-client/src/avatars/AvatarMixer.cpp | 1 + .../src/avatars/AvatarMixerClientData.cpp | 26 +++++++++++++++++++ .../src/avatars/AvatarMixerClientData.h | 16 +++++++++++- .../src/avatars/AvatarMixerSlave.cpp | 12 +++++++-- libraries/avatars/src/AvatarHashMap.cpp | 11 ++++++++ libraries/avatars/src/AvatarTraits.h | 4 +++ libraries/networking/src/udt/PacketHeaders.h | 2 +- 7 files changed, 68 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 53fc13e5cf..6b2b60d3fb 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -56,6 +56,7 @@ AvatarMixer::AvatarMixer(ReceivedMessage& message) : packetReceiver.registerListener(PacketType::RequestsDomainListData, this, "handleRequestsDomainListDataPacket"); packetReceiver.registerListener(PacketType::AvatarIdentityRequest, this, "handleAvatarIdentityRequestPacket"); packetReceiver.registerListener(PacketType::SetAvatarTraits, this, "queueIncomingPacket"); + packetReceiver.registerListener(PacketType::BulkAvatarTraitsAck, this, "queueIncomingPacket"); packetReceiver.registerListenerForTypes({ PacketType::ReplicatedAvatarIdentity, diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 5e36d8beaf..076f9ea862 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -68,6 +68,9 @@ int AvatarMixerClientData::processPackets(const SlaveSharedData& slaveSharedData case PacketType::SetAvatarTraits: processSetTraitsMessage(*packet, slaveSharedData, *node); break; + case PacketType::BulkAvatarTraitsAck: + processBulkAvatarTraitsAckMessage(*packet); + break; default: Q_UNREACHABLE(); } @@ -179,6 +182,29 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, } } +void AvatarMixerClientData::processBulkAvatarTraitsAckMessage(ReceivedMessage& message) { + + // Look up the avatar/trait data associated with this ack and update the 'last ack' list + // with it. + AvatarTraits::TraitMessageSequence seq; + message.readPrimitive(&seq); + auto& sentAvatarTraitVersions = _pendingTraitVersions.find(seq); + if (sentAvatarTraitVersions != _pendingTraitVersions.end()) { + for (auto& nodeTraitVersions : sentAvatarTraitVersions->second) { + auto& nodeId = nodeTraitVersions.first; + auto& versions = nodeTraitVersions.second; + auto simpleReceivedIt = versions.simpleCBegin(); + while (simpleReceivedIt != versions.simpleCEnd()) { + auto traitType = static_cast(std::distance(versions.simpleCBegin(), + simpleReceivedIt)); + _ackedTraitVersions[nodeId][traitType] = *simpleReceivedIt; + } + } + _pendingTraitVersions.erase(sentAvatarTraitVersions); + } +} + + void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedData &slaveSharedData, Node& sendingNode, AvatarTraits::TraitVersion traitVersion) { const auto& whitelist = slaveSharedData.skeletonURLWhitelist; diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 8a86af384a..f26ce8504b 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -42,6 +42,7 @@ public: AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID); virtual ~AvatarMixerClientData() {} using HRCTime = p_high_resolution_clock::time_point; + using NodeTraitVersions = std::unordered_map; int parseData(ReceivedMessage& message) override; AvatarData& getAvatar() { return *_avatar; } @@ -124,6 +125,7 @@ public: int processPackets(const SlaveSharedData& slaveSharedData); // returns number of packets processed void processSetTraitsMessage(ReceivedMessage& message, const SlaveSharedData& slaveSharedData, Node& sendingNode); + void processBulkAvatarTraitsAckMessage(ReceivedMessage& message); void checkSkeletonURLAgainstWhitelist(const SlaveSharedData& slaveSharedData, Node& sendingNode, AvatarTraits::TraitVersion traitVersion); @@ -138,7 +140,14 @@ public: void setLastOtherAvatarTraitsSendPoint(Node::LocalID otherAvatar, TraitsCheckTimestamp sendPoint) { _lastSentTraitsTimestamps[otherAvatar] = sendPoint; } + AvatarTraits::TraitMessageSequence getTraitsMessageSequence() const { return _currentTraitsMessageSequence; } + AvatarTraits::TraitMessageSequence nextTraitsMessageSequence() { return ++_currentTraitsMessageSequence; } + AvatarTraits::TraitVersions& getPendingTraitVersions(AvatarTraits::TraitMessageSequence seq, Node::LocalID otherId) { + return _pendingTraitVersions[seq][otherId]; + } + AvatarTraits::TraitVersions& getLastSentTraitVersions(Node::LocalID otherAvatar) { return _sentTraitVersions[otherAvatar]; } + AvatarTraits::TraitVersions& getLastAckedTraitVersions(Node::LocalID otherAvatar) { return _ackedTraitVersions[otherAvatar]; } void resetSentTraitData(Node::LocalID nodeID); @@ -183,8 +192,13 @@ private: AvatarTraits::TraitVersions _lastReceivedTraitVersions; TraitsCheckTimestamp _lastReceivedTraitsChange; + AvatarTraits::TraitMessageSequence _currentTraitsMessageSequence{ 0 }; + + std::unordered_map _pendingTraitVersions; + std::unordered_map _lastSentTraitsTimestamps; - std::unordered_map _sentTraitVersions; + NodeTraitVersions _sentTraitVersions; + NodeTraitVersions _ackedTraitVersions; std::atomic_bool _isIgnoreRadiusEnabled { false }; }; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index cd9d164ef7..b0cda85bec 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -103,6 +103,7 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis // compare trait versions so we can see what exactly needs to go out auto& lastSentVersions = listeningNodeData->getLastSentTraitVersions(otherNodeLocalID); + auto& lastAckedVersions = listeningNodeData->getLastAckedTraitVersions(otherNodeLocalID); const auto& lastReceivedVersions = sendingNodeData->getLastReceivedTraitVersions(); auto simpleReceivedIt = lastReceivedVersions.simpleCBegin(); @@ -112,13 +113,18 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis auto lastReceivedVersion = *simpleReceivedIt; auto& lastSentVersionRef = lastSentVersions[traitType]; + auto& lastAckedVersionRef = lastAckedVersions[traitType]; - if (lastReceivedVersions[traitType] > lastSentVersionRef) { + // hold sending more traits until we've been acked that the last one we sent was received + if (lastAckedVersionRef == lastSentVersionRef && lastReceivedVersions[traitType] > lastSentVersionRef) { // there is an update to this trait, add it to the traits packet bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); - // update the last sent version lastSentVersionRef = lastReceivedVersion; + // Remember which versions we sent in this particular packet + // so we can verify when it's acked. + auto& pendingTraitVersions = listeningNodeData->getPendingTraitVersions(listeningNodeData->getTraitsMessageSequence(), otherNodeLocalID); + pendingTraitVersions[traitType] = lastReceivedVersion; } ++simpleReceivedIt; @@ -419,6 +425,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) int remainingAvatars = (int)sortedAvatars.size(); auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); + traitsPacketList->writePrimitive(nodeData->nextTraitsMessageSequence()); + auto avatarPacket = NLPacket::create(PacketType::BulkAvatarData); const int avatarPacketCapacity = avatarPacket->getPayloadCapacity(); int avatarSpaceAvailable = avatarPacketCapacity; diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 41ca950b3b..ff902945bc 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -328,6 +328,17 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer } void AvatarHashMap::processBulkAvatarTraits(QSharedPointer message, SharedNodePointer sendingNode) { + AvatarTraits::TraitMessageSequence seq; + + message->readPrimitive(&seq); + + // we have a mixer to send to, setup our set traits packet + auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true); + traitsAckPacket->writePrimitive(seq); + auto nodeList = DependencyManager::get(); + SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); + nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); + while (message->getBytesLeftToRead()) { // read the avatar ID to figure out which avatar this is for diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index 5e28515d12..966ae6bdde 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -41,6 +41,10 @@ namespace AvatarTraits { const TraitWireSize DELETED_TRAIT_SIZE = -1; const TraitWireSize MAXIMUM_TRAIT_SIZE = INT16_MAX; + using TraitMessageSequence = int64_t; + const TraitMessageSequence FIRST_TRAIT_SEQUENCE = 0; + const TraitMessageSequence MAX_TRAIT_SEQUENCE = INT64_MAX; + inline qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, TraitVersion traitVersion = NULL_TRAIT_VERSION) { qint64 bytesWritten = 0; diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index b46eb3e9e4..508d46def8 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -133,7 +133,7 @@ public: EntityQueryInitialResultsComplete, BulkAvatarTraits, AudioSoloRequest, - + BulkAvatarTraitsAck, NUM_PACKET_TYPE }; From da70271acfccdfd32d8d61b39978f8743b993065 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Wed, 26 Dec 2018 13:46:08 -0800 Subject: [PATCH 03/17] Checkpoint #2 Case20377 - Add Ack for BulkAvatarTraits --- .../src/avatars/AvatarMixerClientData.cpp | 13 ++++++++ .../src/avatars/AvatarMixerSlave.cpp | 33 ++++++++++++++----- .../src/avatars/AvatarMixerSlave.h | 5 +++ .../networking/src/udt/PacketHeaders.cpp | 2 ++ libraries/networking/src/udt/PacketHeaders.h | 3 +- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 076f9ea862..a20d6504de 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -190,6 +190,10 @@ void AvatarMixerClientData::processBulkAvatarTraitsAckMessage(ReceivedMessage& m message.readPrimitive(&seq); auto& sentAvatarTraitVersions = _pendingTraitVersions.find(seq); if (sentAvatarTraitVersions != _pendingTraitVersions.end()) { + // Note, this is not a simple move of the pending traits + // to the acked traits. Instead, it's a copy where existing + // trait versions in the acked hash are retained for traits not + // included in the pending hash for (auto& nodeTraitVersions : sentAvatarTraitVersions->second) { auto& nodeId = nodeTraitVersions.first; auto& versions = nodeTraitVersions.second; @@ -198,10 +202,19 @@ void AvatarMixerClientData::processBulkAvatarTraitsAckMessage(ReceivedMessage& m auto traitType = static_cast(std::distance(versions.simpleCBegin(), simpleReceivedIt)); _ackedTraitVersions[nodeId][traitType] = *simpleReceivedIt; + simpleReceivedIt++; } } _pendingTraitVersions.erase(sentAvatarTraitVersions); } + else { + // This can happen either the BulkAvatarTraits was sent with no simple traits, + // or if the avatar mixer restarts while there are pending + // BulkAvatarTraits messages in-flight. + if (seq > getTraitsMessageSequence()) { + qWarning() << "Received BulkAvatarTraitsAck with future seq (potential avatar mixer restart) " << seq << " from " << message.getSenderSockAddr(); + } + } } diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index b0cda85bec..1e84da7e55 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -80,6 +80,21 @@ int AvatarMixerSlave::sendIdentityPacket(NLPacketList& packetList, const AvatarM } } +qint64 AvatarMixerSlave::addTraitsNodeHeader(AvatarMixerClientData* listeningNodeData, + const AvatarMixerClientData* sendingNodeData, + NLPacketList& traitsPacketList, + qint64 bytesWritten) { + if (bytesWritten == 0) { + + if (traitsPacketList.getNumPackets() == 0) { + bytesWritten += traitsPacketList.writePrimitive(listeningNodeData->nextTraitsMessageSequence()); + } + // add the avatar ID to mark the beginning of traits for this avatar + bytesWritten += traitsPacketList.write(sendingNodeData->getNodeID().toRfc4122()); + } + return bytesWritten; +} + qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* listeningNodeData, const AvatarMixerClientData* sendingNodeData, NLPacketList& traitsPacketList) { @@ -96,9 +111,6 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis if (timeOfLastTraitsChange > timeOfLastTraitsSent) { // there is definitely new traits data to send - // add the avatar ID to mark the beginning of traits for this avatar - bytesWritten += traitsPacketList.write(sendingNodeData->getNodeID().toRfc4122()); - auto sendingAvatar = sendingNodeData->getAvatarSharedPointer(); // compare trait versions so we can see what exactly needs to go out @@ -116,7 +128,8 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis auto& lastAckedVersionRef = lastAckedVersions[traitType]; // hold sending more traits until we've been acked that the last one we sent was received - if (lastAckedVersionRef == lastSentVersionRef && lastReceivedVersions[traitType] > lastSentVersionRef) { + if (lastSentVersionRef == lastAckedVersionRef && lastReceivedVersions[traitType] > lastSentVersionRef) { + addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); // there is an update to this trait, add it to the traits packet bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); // update the last sent version @@ -156,6 +169,7 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis }); if (!isDeleted && (sentInstanceIt == sentIDValuePairs.end() || receivedVersion > sentInstanceIt->value)) { + addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); // this instance version exists and has never been sent or is newer so we need to send it bytesWritten += sendingAvatar->packTraitInstance(traitType, instanceID, traitsPacketList, receivedVersion); @@ -166,6 +180,7 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis sentIDValuePairs.emplace_back(instanceID, receivedVersion); } } else if (isDeleted && sentInstanceIt != sentIDValuePairs.end() && absoluteReceivedVersion > sentInstanceIt->value) { + addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); // this instance version was deleted and we haven't sent the delete to this client yet bytesWritten += AvatarTraits::packInstancedTraitDelete(traitType, instanceID, traitsPacketList, absoluteReceivedVersion); @@ -177,10 +192,10 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis ++instancedReceivedIt; } - - // write a null trait type to mark the end of trait data for this avatar - bytesWritten += traitsPacketList.writePrimitive(AvatarTraits::NullTrait); - + if (bytesWritten) { + // write a null trait type to mark the end of trait data for this avatar + bytesWritten += traitsPacketList.writePrimitive(AvatarTraits::NullTrait); + } // since we send all traits for this other avatar, update the time of last traits sent // to match the time of last traits change listeningNodeData->setLastOtherAvatarTraitsSendPoint(otherNodeLocalID, timeOfLastTraitsChange); @@ -425,7 +440,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) int remainingAvatars = (int)sortedAvatars.size(); auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); - traitsPacketList->writePrimitive(nodeData->nextTraitsMessageSequence()); auto avatarPacket = NLPacket::create(PacketType::BulkAvatarData); const int avatarPacketCapacity = avatarPacket->getPayloadCapacity(); @@ -558,6 +572,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (traitsPacketList->getNumPackets() >= 1) { // send the traits packet list + nodeList->sendPacketList(std::move(traitsPacketList), *destinationNode); } diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index 2ef90af38e..56c49b5b2e 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -104,6 +104,11 @@ private: int sendIdentityPacket(NLPacketList& packet, const AvatarMixerClientData* nodeData, const Node& destinationNode); int sendReplicatedIdentityPacket(const Node& agentNode, const AvatarMixerClientData* nodeData, const Node& destinationNode); + qint64 addTraitsNodeHeader(AvatarMixerClientData* listeningNodeData, + const AvatarMixerClientData* sendingNodeData, + NLPacketList& traitsPacketList, + qint64 bytesWritten); + qint64 addChangedTraitsToBulkPacket(AvatarMixerClientData* listeningNodeData, const AvatarMixerClientData* sendingNodeData, NLPacketList& traitsPacketList); diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index fad38d565b..5275c2c78e 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -96,6 +96,8 @@ PacketVersion versionForPacketType(PacketType packetType) { return 22; case PacketType::EntityQueryInitialResultsComplete: return static_cast(EntityVersion::ParticleSpin); + case PacketType::BulkAvatarTraitsAck: + return static_cast(AvatarMixerPacketVersion::AvatarTraitsAck); default: return 22; } diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 508d46def8..5da3acdef9 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -299,7 +299,8 @@ enum class AvatarMixerPacketVersion : PacketVersion { MigrateSkeletonURLToTraits, MigrateAvatarEntitiesToTraits, FarGrabJointsRedux, - JointTransScaled + JointTransScaled, + AvatarTraitsAck }; enum class DomainConnectRequestVersion : PacketVersion { From 2e457e2212ef0e7dec1c6719adcfb06442d5326a Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Tue, 1 Jan 2019 20:50:46 -0800 Subject: [PATCH 04/17] Checkpoint trait flow control --- .../src/avatars/AvatarMixerClientData.cpp | 69 +++++++++++-------- .../src/avatars/AvatarMixerSlave.cpp | 24 ++++++- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index a20d6504de..b2b09df6d2 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -19,9 +19,7 @@ #include "AvatarMixerSlave.h" -AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : - NodeData(nodeID, nodeLocalID) -{ +AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : NodeData(nodeID, nodeLocalID) { // in case somebody calls getSessionUUID on the AvatarData instance, make sure it has the right ID _avatar->setID(nodeID); } @@ -82,12 +80,11 @@ int AvatarMixerClientData::processPackets(const SlaveSharedData& slaveSharedData } int AvatarMixerClientData::parseData(ReceivedMessage& message) { - // pull the sequence number from the data first uint16_t sequenceNumber; message.readPrimitive(&sequenceNumber); - + if (sequenceNumber < _lastReceivedSequenceNumber && _lastReceivedSequenceNumber != UINT16_MAX) { incrementNumOutOfOrderSends(); } @@ -98,7 +95,8 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message) { } void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, - const SlaveSharedData& slaveSharedData, Node& sendingNode) { + const SlaveSharedData& slaveSharedData, + Node& sendingNode) { // pull the trait version from the message AvatarTraits::TraitVersion packetTraitVersion; message.readPrimitive(&packetTraitVersion); @@ -137,7 +135,7 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, AvatarTraits::TraitInstanceID instanceID = QUuid::fromRfc4122(message.readWithoutCopy(NUM_BYTES_RFC4122_UUID)); if (message.getBytesLeftToRead() == 0) { - qWarning () << "Received an instanced trait with no size from" << message.getSenderSockAddr(); + qWarning() << "Received an instanced trait with no size from" << message.getSenderSockAddr(); break; } @@ -145,7 +143,8 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, message.readPrimitive(&traitSize); if (traitSize < -1 || traitSize > message.getBytesLeftToRead()) { - qWarning() << "Refusing to process instanced trait of size" << traitSize << "from" << message.getSenderSockAddr(); + qWarning() << "Refusing to process instanced trait of size" << traitSize << "from" + << message.getSenderSockAddr(); break; } @@ -171,7 +170,8 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, message.seek(message.getPosition() + traitSize); } } else { - qWarning() << "Refusing to process traits packet with instanced trait of unprocessable type from" << message.getSenderSockAddr(); + qWarning() << "Refusing to process traits packet with instanced trait of unprocessable type from" + << message.getSenderSockAddr(); break; } } @@ -183,42 +183,56 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, } void AvatarMixerClientData::processBulkAvatarTraitsAckMessage(ReceivedMessage& message) { - // Look up the avatar/trait data associated with this ack and update the 'last ack' list // with it. AvatarTraits::TraitMessageSequence seq; message.readPrimitive(&seq); - auto& sentAvatarTraitVersions = _pendingTraitVersions.find(seq); + auto sentAvatarTraitVersions = _pendingTraitVersions.find(seq); if (sentAvatarTraitVersions != _pendingTraitVersions.end()) { // Note, this is not a simple move of the pending traits // to the acked traits. Instead, it's a copy where existing - // trait versions in the acked hash are retained for traits not - // included in the pending hash + // trait versions in the acked hash are retained for traits not + // included in the pending hash for (auto& nodeTraitVersions : sentAvatarTraitVersions->second) { auto& nodeId = nodeTraitVersions.first; auto& versions = nodeTraitVersions.second; auto simpleReceivedIt = versions.simpleCBegin(); while (simpleReceivedIt != versions.simpleCEnd()) { - auto traitType = static_cast(std::distance(versions.simpleCBegin(), - simpleReceivedIt)); + auto traitType = static_cast(std::distance(versions.simpleCBegin(), simpleReceivedIt)); _ackedTraitVersions[nodeId][traitType] = *simpleReceivedIt; simpleReceivedIt++; } + + // enumerate the sent instanced trait versions + auto instancedSentIt = versions.instancedCBegin(); + while (instancedSentIt != versions.instancedCEnd()) { + auto traitType = instancedSentIt->traitType; + // get or create the sent trait versions for this trait type + auto& sentIDValuePairs = versions.getInstanceIDValuePairs(traitType); + + // enumerate each sent instance + for (auto& sentInstance : instancedSentIt->instances) { + auto instanceID = sentInstance.id; + const auto sentVersion = sentInstance.value; + _ackedTraitVersions[nodeId].instanceInsert(traitType, instanceID, sentVersion); + } + instancedSentIt++; + } } _pendingTraitVersions.erase(sentAvatarTraitVersions); - } - else { + } else { // This can happen either the BulkAvatarTraits was sent with no simple traits, // or if the avatar mixer restarts while there are pending // BulkAvatarTraits messages in-flight. if (seq > getTraitsMessageSequence()) { - qWarning() << "Received BulkAvatarTraitsAck with future seq (potential avatar mixer restart) " << seq << " from " << message.getSenderSockAddr(); + qWarning() << "Received BulkAvatarTraitsAck with future seq (potential avatar mixer restart) " << seq << " from " + << message.getSenderSockAddr(); } } } - -void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedData &slaveSharedData, Node& sendingNode, +void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedData& slaveSharedData, + Node& sendingNode, AvatarTraits::TraitVersion traitVersion) { const auto& whitelist = slaveSharedData.skeletonURLWhitelist; @@ -244,8 +258,8 @@ void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedDa // make sure we're not unecessarily overriding the default avatar with the default avatar if (_avatar->getWireSafeSkeletonModelURL() != slaveSharedData.skeletonReplacementURL) { // we need to change this avatar's skeleton URL, and send them a traits packet informing them of the change - qDebug() << "Overwriting avatar URL" << _avatar->getWireSafeSkeletonModelURL() - << "to replacement" << slaveSharedData.skeletonReplacementURL << "for" << sendingNode.getUUID(); + qDebug() << "Overwriting avatar URL" << _avatar->getWireSafeSkeletonModelURL() << "to replacement" + << slaveSharedData.skeletonReplacementURL << "for" << sendingNode.getUUID(); _avatar->setSkeletonModelURL(slaveSharedData.skeletonReplacementURL); auto packet = NLPacket::create(PacketType::SetAvatarTraits, -1, true); @@ -327,7 +341,7 @@ void AvatarMixerClientData::readViewFrustumPacket(const QByteArray& message) { _currentViewFrustums.clear(); auto sourceBuffer = reinterpret_cast(message.constData()); - + uint8_t numFrustums = 0; memcpy(&numFrustums, sourceBuffer, sizeof(numFrustums)); sourceBuffer += sizeof(numFrustums); @@ -342,9 +356,7 @@ void AvatarMixerClientData::readViewFrustumPacket(const QByteArray& message) { bool AvatarMixerClientData::otherAvatarInView(const AABox& otherAvatarBox) { return std::any_of(std::begin(_currentViewFrustums), std::end(_currentViewFrustums), - [&](const ConicalViewFrustum& viewFrustum) { - return viewFrustum.intersects(otherAvatarBox); - }); + [&](const ConicalViewFrustum& viewFrustum) { return viewFrustum.intersects(otherAvatarBox); }); } void AvatarMixerClientData::loadJSONStats(QJsonObject& jsonObject) const { @@ -355,14 +367,15 @@ void AvatarMixerClientData::loadJSONStats(QJsonObject& jsonObject) const { jsonObject["total_num_out_of_order_sends"] = _numOutOfOrderSends; jsonObject[OUTBOUND_AVATAR_DATA_STATS_KEY] = getOutboundAvatarDataKbps(); - jsonObject[INBOUND_AVATAR_DATA_STATS_KEY] = _avatar->getAverageBytesReceivedPerSecond() / (float) BYTES_PER_KILOBIT; + jsonObject[INBOUND_AVATAR_DATA_STATS_KEY] = _avatar->getAverageBytesReceivedPerSecond() / (float)BYTES_PER_KILOBIT; jsonObject["av_data_receive_rate"] = _avatar->getReceiveRate(); jsonObject["recent_other_av_in_view"] = _recentOtherAvatarsInView; jsonObject["recent_other_av_out_of_view"] = _recentOtherAvatarsOutOfView; } -AvatarMixerClientData::TraitsCheckTimestamp AvatarMixerClientData::getLastOtherAvatarTraitsSendPoint(Node::LocalID otherAvatar) const { +AvatarMixerClientData::TraitsCheckTimestamp AvatarMixerClientData::getLastOtherAvatarTraitsSendPoint( + Node::LocalID otherAvatar) const { auto it = _lastSentTraitsTimestamps.find(otherAvatar); if (it != _lastSentTraitsTimestamps.end()) { diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 1e84da7e55..5ed10dad98 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -150,6 +150,7 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis // get or create the sent trait versions for this trait type auto& sentIDValuePairs = lastSentVersions.getInstanceIDValuePairs(traitType); + auto& ackIDValuePairs = lastAckedVersions.getInstanceIDValuePairs(traitType); // enumerate each received instance for (auto& receivedInstance : instancedReceivedIt->instances) { @@ -167,7 +168,16 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis { return sentInstance.id == instanceID; }); - + // look for existing acked version for this instance + auto ackedInstanceIt = std::find_if(ackIDValuePairs.begin(), ackIDValuePairs.end(), + [instanceID](auto& ackInstance) { return ackInstance.id == instanceID; }); + + // if we have a sent version, then we must have an acked instance of the same trait with the same + // version to go on, otherwise we drop the received trait + if (sentInstanceIt != sentIDValuePairs.end() && + (ackedInstanceIt == ackIDValuePairs.end() || sentInstanceIt->value != ackedInstanceIt->value)) { + continue; + } if (!isDeleted && (sentInstanceIt == sentIDValuePairs.end() || receivedVersion > sentInstanceIt->value)) { addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); @@ -179,6 +189,12 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis } else { sentIDValuePairs.emplace_back(instanceID, receivedVersion); } + + auto& pendingTraitVersions = + listeningNodeData->getPendingTraitVersions(listeningNodeData->getTraitsMessageSequence(), + otherNodeLocalID); + pendingTraitVersions.instanceInsert(traitType, instanceID, receivedVersion); + } else if (isDeleted && sentInstanceIt != sentIDValuePairs.end() && absoluteReceivedVersion > sentInstanceIt->value) { addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); @@ -187,6 +203,12 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis // update the last sent version for this trait instance to the absolute value of the deleted version sentInstanceIt->value = absoluteReceivedVersion; + + auto& pendingTraitVersions = + listeningNodeData->getPendingTraitVersions(listeningNodeData->getTraitsMessageSequence(), + otherNodeLocalID); + pendingTraitVersions.instanceInsert(traitType, instanceID, absoluteReceivedVersion); + } } From 3d8a323fae7dcb97e672d27c6be1d4e957eb6207 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Wed, 2 Jan 2019 14:17:06 -0800 Subject: [PATCH 05/17] Remove unused variable --- .../src/avatars/AvatarMixerClientData.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index b2b09df6d2..37f71103d2 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -19,7 +19,8 @@ #include "AvatarMixerSlave.h" -AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : NodeData(nodeID, nodeLocalID) { +AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : + NodeData(nodeID, nodeLocalID) { // in case somebody calls getSessionUUID on the AvatarData instance, make sure it has the right ID _avatar->setID(nodeID); } @@ -208,7 +209,6 @@ void AvatarMixerClientData::processBulkAvatarTraitsAckMessage(ReceivedMessage& m while (instancedSentIt != versions.instancedCEnd()) { auto traitType = instancedSentIt->traitType; // get or create the sent trait versions for this trait type - auto& sentIDValuePairs = versions.getInstanceIDValuePairs(traitType); // enumerate each sent instance for (auto& sentInstance : instancedSentIt->instances) { @@ -258,8 +258,8 @@ void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedDa // make sure we're not unecessarily overriding the default avatar with the default avatar if (_avatar->getWireSafeSkeletonModelURL() != slaveSharedData.skeletonReplacementURL) { // we need to change this avatar's skeleton URL, and send them a traits packet informing them of the change - qDebug() << "Overwriting avatar URL" << _avatar->getWireSafeSkeletonModelURL() << "to replacement" - << slaveSharedData.skeletonReplacementURL << "for" << sendingNode.getUUID(); + qDebug() << "Overwriting avatar URL" << _avatar->getWireSafeSkeletonModelURL() + << "to replacement" << slaveSharedData.skeletonReplacementURL << "for" << sendingNode.getUUID(); _avatar->setSkeletonModelURL(slaveSharedData.skeletonReplacementURL); auto packet = NLPacket::create(PacketType::SetAvatarTraits, -1, true); @@ -356,7 +356,9 @@ void AvatarMixerClientData::readViewFrustumPacket(const QByteArray& message) { bool AvatarMixerClientData::otherAvatarInView(const AABox& otherAvatarBox) { return std::any_of(std::begin(_currentViewFrustums), std::end(_currentViewFrustums), - [&](const ConicalViewFrustum& viewFrustum) { return viewFrustum.intersects(otherAvatarBox); }); + [&](const ConicalViewFrustum& viewFrustum) { + return viewFrustum.intersects(otherAvatarBox); + }); } void AvatarMixerClientData::loadJSONStats(QJsonObject& jsonObject) const { @@ -367,15 +369,14 @@ void AvatarMixerClientData::loadJSONStats(QJsonObject& jsonObject) const { jsonObject["total_num_out_of_order_sends"] = _numOutOfOrderSends; jsonObject[OUTBOUND_AVATAR_DATA_STATS_KEY] = getOutboundAvatarDataKbps(); - jsonObject[INBOUND_AVATAR_DATA_STATS_KEY] = _avatar->getAverageBytesReceivedPerSecond() / (float)BYTES_PER_KILOBIT; + jsonObject[INBOUND_AVATAR_DATA_STATS_KEY] = _avatar->getAverageBytesReceivedPerSecond() / (float) BYTES_PER_KILOBIT; jsonObject["av_data_receive_rate"] = _avatar->getReceiveRate(); jsonObject["recent_other_av_in_view"] = _recentOtherAvatarsInView; jsonObject["recent_other_av_out_of_view"] = _recentOtherAvatarsOutOfView; } -AvatarMixerClientData::TraitsCheckTimestamp AvatarMixerClientData::getLastOtherAvatarTraitsSendPoint( - Node::LocalID otherAvatar) const { +AvatarMixerClientData::TraitsCheckTimestamp AvatarMixerClientData::getLastOtherAvatarTraitsSendPoint(Node::LocalID otherAvatar) const { auto it = _lastSentTraitsTimestamps.find(otherAvatar); if (it != _lastSentTraitsTimestamps.end()) { From 443b54d931a1850ae1714a68ed267ba03138fcae Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Thu, 3 Jan 2019 11:25:39 -0800 Subject: [PATCH 06/17] comment and naming cleanup --- .../src/avatars/AvatarMixerClientData.cpp | 49 ++++++++++--------- .../src/avatars/AvatarMixerClientData.h | 14 +++--- .../src/avatars/AvatarMixerSlave.cpp | 10 +++- libraries/avatars/src/AvatarHashMap.cpp | 8 +-- 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 37f71103d2..1da592b640 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -184,42 +184,47 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, } void AvatarMixerClientData::processBulkAvatarTraitsAckMessage(ReceivedMessage& message) { + // Avatar Traits flow control marks each outgoing avatar traits packet with a + // sequence number. The mixer caches the traits sent in the traits packet. + // Until an ack with the sequence number comes back, all updates to _traits + // in that packet_ are ignored. Updates to traits not in that packet will + // be sent. + // Look up the avatar/trait data associated with this ack and update the 'last ack' list // with it. AvatarTraits::TraitMessageSequence seq; message.readPrimitive(&seq); - auto sentAvatarTraitVersions = _pendingTraitVersions.find(seq); - if (sentAvatarTraitVersions != _pendingTraitVersions.end()) { - // Note, this is not a simple move of the pending traits - // to the acked traits. Instead, it's a copy where existing - // trait versions in the acked hash are retained for traits not - // included in the pending hash - for (auto& nodeTraitVersions : sentAvatarTraitVersions->second) { - auto& nodeId = nodeTraitVersions.first; - auto& versions = nodeTraitVersions.second; - auto simpleReceivedIt = versions.simpleCBegin(); - while (simpleReceivedIt != versions.simpleCEnd()) { - auto traitType = static_cast(std::distance(versions.simpleCBegin(), simpleReceivedIt)); - _ackedTraitVersions[nodeId][traitType] = *simpleReceivedIt; + auto sentAvatarTraitVersions = _perNodePendingTraitVersions.find(seq); + if (sentAvatarTraitVersions != _perNodePendingTraitVersions.end()) { + for (auto& perNodeTraitVersions : sentAvatarTraitVersions->second) { + auto& nodeId = perNodeTraitVersions.first; + auto& traitVersions = perNodeTraitVersions.second; + // For each trait that was sent in the traits packet, + // update the 'acked' trait version. Traits not + // sent in the traits packet keep their version. + + // process simple traits + auto simpleReceivedIt = traitVersions.simpleCBegin(); + while (simpleReceivedIt != traitVersions.simpleCEnd()) { + auto traitType = static_cast(std::distance(traitVersions.simpleCBegin(), simpleReceivedIt)); + _perNodeAckedTraitVersions[nodeId][traitType] = *simpleReceivedIt; simpleReceivedIt++; } - // enumerate the sent instanced trait versions - auto instancedSentIt = versions.instancedCBegin(); - while (instancedSentIt != versions.instancedCEnd()) { + // process instanced traits + auto instancedSentIt = traitVersions.instancedCBegin(); + while (instancedSentIt != traitVersions.instancedCEnd()) { auto traitType = instancedSentIt->traitType; - // get or create the sent trait versions for this trait type - // enumerate each sent instance for (auto& sentInstance : instancedSentIt->instances) { auto instanceID = sentInstance.id; const auto sentVersion = sentInstance.value; - _ackedTraitVersions[nodeId].instanceInsert(traitType, instanceID, sentVersion); + _perNodeAckedTraitVersions[nodeId].instanceInsert(traitType, instanceID, sentVersion); } instancedSentIt++; } } - _pendingTraitVersions.erase(sentAvatarTraitVersions); + _perNodePendingTraitVersions.erase(sentAvatarTraitVersions); } else { // This can happen either the BulkAvatarTraits was sent with no simple traits, // or if the avatar mixer restarts while there are pending @@ -334,7 +339,7 @@ void AvatarMixerClientData::removeFromRadiusIgnoringSet(const QUuid& other) { void AvatarMixerClientData::resetSentTraitData(Node::LocalID nodeLocalID) { _lastSentTraitsTimestamps[nodeLocalID] = TraitsCheckTimestamp(); - _sentTraitVersions[nodeLocalID].reset(); + _perNodeSentTraitVersions[nodeLocalID].reset(); } void AvatarMixerClientData::readViewFrustumPacket(const QByteArray& message) { @@ -390,5 +395,5 @@ void AvatarMixerClientData::cleanupKilledNode(const QUuid&, Node::LocalID nodeLo removeLastBroadcastSequenceNumber(nodeLocalID); removeLastBroadcastTime(nodeLocalID); _lastSentTraitsTimestamps.erase(nodeLocalID); - _sentTraitVersions.erase(nodeLocalID); + _perNodeSentTraitVersions.erase(nodeLocalID); } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index f26ce8504b..16054e3458 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -42,7 +42,7 @@ public: AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID); virtual ~AvatarMixerClientData() {} using HRCTime = p_high_resolution_clock::time_point; - using NodeTraitVersions = std::unordered_map; + using PerNodeTraitVersions = std::unordered_map; int parseData(ReceivedMessage& message) override; AvatarData& getAvatar() { return *_avatar; } @@ -143,11 +143,11 @@ public: AvatarTraits::TraitMessageSequence getTraitsMessageSequence() const { return _currentTraitsMessageSequence; } AvatarTraits::TraitMessageSequence nextTraitsMessageSequence() { return ++_currentTraitsMessageSequence; } AvatarTraits::TraitVersions& getPendingTraitVersions(AvatarTraits::TraitMessageSequence seq, Node::LocalID otherId) { - return _pendingTraitVersions[seq][otherId]; + return _perNodePendingTraitVersions[seq][otherId]; } - AvatarTraits::TraitVersions& getLastSentTraitVersions(Node::LocalID otherAvatar) { return _sentTraitVersions[otherAvatar]; } - AvatarTraits::TraitVersions& getLastAckedTraitVersions(Node::LocalID otherAvatar) { return _ackedTraitVersions[otherAvatar]; } + AvatarTraits::TraitVersions& getLastSentTraitVersions(Node::LocalID otherAvatar) { return _perNodeSentTraitVersions[otherAvatar]; } + AvatarTraits::TraitVersions& getLastAckedTraitVersions(Node::LocalID otherAvatar) { return _perNodeAckedTraitVersions[otherAvatar]; } void resetSentTraitData(Node::LocalID nodeID); @@ -194,11 +194,11 @@ private: AvatarTraits::TraitMessageSequence _currentTraitsMessageSequence{ 0 }; - std::unordered_map _pendingTraitVersions; + std::unordered_map _perNodePendingTraitVersions; std::unordered_map _lastSentTraitsTimestamps; - NodeTraitVersions _sentTraitVersions; - NodeTraitVersions _ackedTraitVersions; + PerNodeTraitVersions _perNodeSentTraitVersions; + PerNodeTraitVersions _perNodeAckedTraitVersions; std::atomic_bool _isIgnoreRadiusEnabled { false }; }; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 5ed10dad98..5f24ae1b64 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -85,11 +85,11 @@ qint64 AvatarMixerSlave::addTraitsNodeHeader(AvatarMixerClientData* listeningNod NLPacketList& traitsPacketList, qint64 bytesWritten) { if (bytesWritten == 0) { - if (traitsPacketList.getNumPackets() == 0) { + // This is the beginning of the traits packet, write out the sequence number. bytesWritten += traitsPacketList.writePrimitive(listeningNodeData->nextTraitsMessageSequence()); } - // add the avatar ID to mark the beginning of traits for this avatar + // This is the beginning of the traits for a node, write out the node id bytesWritten += traitsPacketList.write(sendingNodeData->getNodeID().toRfc4122()); } return bytesWritten; @@ -99,6 +99,12 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis const AvatarMixerClientData* sendingNodeData, NLPacketList& traitsPacketList) { + // Avatar Traits flow control marks each outgoing avatar traits packet with a + // sequence number. The mixer caches the traits sent in the traits packet. + // Until an ack with the sequence number comes back, all updates to _traits + // in that packet_ are ignored. Updates to traits not in that packet will + // be sent. + auto otherNodeLocalID = sendingNodeData->getNodeLocalID(); // Perform a simple check with two server clock time points diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index ff902945bc..6a67ef6638 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -332,13 +332,15 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess message->readPrimitive(&seq); - // we have a mixer to send to, setup our set traits packet auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true); traitsAckPacket->writePrimitive(seq); auto nodeList = DependencyManager::get(); SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); - nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); - + if (!avatarMixer.isNull()) { + // we have a mixer to send to, acknowledge that we received these + // traits. + nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); + } while (message->getBytesLeftToRead()) { // read the avatar ID to figure out which avatar this is for From 65a7e15f5d1615044c1cc6157e3b66f3ad56253f Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 20 Dec 2018 14:49:03 -0800 Subject: [PATCH 07/17] don't send AvatarEntity updates when not necessary --- interface/src/AvatarBookmarks.cpp | 1 - interface/src/ui/overlays/Base3DOverlay.cpp | 7 +++++++ interface/src/ui/overlays/Base3DOverlay.h | 1 + .../src/avatars-renderer/Avatar.cpp | 4 ---- libraries/entities/src/EntityItemProperties.h | 12 +++++++++++- libraries/entities/src/SimulationOwner.h | 2 +- libraries/physics/src/EntityMotionState.cpp | 16 +++++++++------- 7 files changed, 29 insertions(+), 14 deletions(-) diff --git a/interface/src/AvatarBookmarks.cpp b/interface/src/AvatarBookmarks.cpp index ee639f602d..1d003f19c1 100644 --- a/interface/src/AvatarBookmarks.cpp +++ b/interface/src/AvatarBookmarks.cpp @@ -60,7 +60,6 @@ void addAvatarEntities(const QVariantList& avatarEntities) { entityProperties.setParentID(myNodeID); entityProperties.setEntityHostType(entity::HostType::AVATAR); entityProperties.setOwningAvatarID(myNodeID); - entityProperties.setSimulationOwner(myNodeID, AVATAR_ENTITY_SIMULATION_PRIORITY); entityProperties.markAllChanged(); EntityItemID id = EntityItemID(QUuid::createUuid()); diff --git a/interface/src/ui/overlays/Base3DOverlay.cpp b/interface/src/ui/overlays/Base3DOverlay.cpp index eab7a96a4b..eb43e8cf45 100644 --- a/interface/src/ui/overlays/Base3DOverlay.cpp +++ b/interface/src/ui/overlays/Base3DOverlay.cpp @@ -27,6 +27,9 @@ Base3DOverlay::Base3DOverlay() : _drawInFront(false), _drawHUDLayer(false) { + // HACK: queryAACube stuff not actually relevant for 3DOverlays, and by setting _queryAACubeSet true here + // we can avoid incorrect evaluation for sending updates for entities with 3DOverlays children. + _queryAACubeSet = true; } Base3DOverlay::Base3DOverlay(const Base3DOverlay* base3DOverlay) : @@ -41,6 +44,9 @@ Base3DOverlay::Base3DOverlay(const Base3DOverlay* base3DOverlay) : _isVisibleInSecondaryCamera(base3DOverlay->_isVisibleInSecondaryCamera) { setTransform(base3DOverlay->getTransform()); + // HACK: queryAACube stuff not actually relevant for 3DOverlays, and by setting _queryAACubeSet true here + // we can avoid incorrect evaluation for sending updates for entities with 3DOverlays children. + _queryAACubeSet = true; } QVariantMap convertOverlayLocationFromScriptSemantics(const QVariantMap& properties, bool scalesWithParent) { @@ -209,6 +215,7 @@ void Base3DOverlay::setProperties(const QVariantMap& originalProperties) { transaction.updateItem(itemID); scene->enqueueTransaction(transaction); } + _queryAACubeSet = true; // HACK: just in case some SpatiallyNestable code accidentally set it false } } diff --git a/interface/src/ui/overlays/Base3DOverlay.h b/interface/src/ui/overlays/Base3DOverlay.h index 6cc5182b56..daf15e676f 100644 --- a/interface/src/ui/overlays/Base3DOverlay.h +++ b/interface/src/ui/overlays/Base3DOverlay.h @@ -25,6 +25,7 @@ public: Base3DOverlay(const Base3DOverlay* base3DOverlay); void setVisible(bool visible) override; + bool queryAACubeNeedsUpdate() const override { return false; } // HACK: queryAACube not relevant for Overlays virtual OverlayID getOverlayID() const override { return OverlayID(getID().toString()); } void setOverlayID(OverlayID overlayID) override { setID(overlayID); } diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 50d3c568d9..598f552bcd 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -393,10 +393,6 @@ void Avatar::updateAvatarEntities() { properties.setEntityHostType(entity::HostType::AVATAR); properties.setOwningAvatarID(getID()); - // there's no entity-server to tell us we're the simulation owner, so always set the - // simulationOwner to the owningAvatarID and a high priority. - properties.setSimulationOwner(getID(), AVATAR_ENTITY_SIMULATION_PRIORITY); - if (properties.getParentID() == AVATAR_SELF_ID) { properties.setParentID(getID()); } diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index b741bb8ca4..9f635057d2 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -143,7 +143,7 @@ public: DEFINE_PROPERTY(PROP_CREATED, Created, created, quint64, UNKNOWN_CREATED_TIME); DEFINE_PROPERTY_REF(PROP_LAST_EDITED_BY, LastEditedBy, lastEditedBy, QUuid, ENTITY_ITEM_DEFAULT_LAST_EDITED_BY); DEFINE_PROPERTY_REF_ENUM(PROP_ENTITY_HOST_TYPE, EntityHostType, entityHostType, entity::HostType, entity::HostType::DOMAIN); - DEFINE_PROPERTY_REF(PROP_OWNING_AVATAR_ID, OwningAvatarID, owningAvatarID, QUuid, UNKNOWN_ENTITY_ID); + DEFINE_PROPERTY_REF_WITH_SETTER(PROP_OWNING_AVATAR_ID, OwningAvatarID, owningAvatarID, QUuid, UNKNOWN_ENTITY_ID); DEFINE_PROPERTY_REF(PROP_PARENT_ID, ParentID, parentID, QUuid, UNKNOWN_ENTITY_ID); DEFINE_PROPERTY_REF(PROP_PARENT_JOINT_INDEX, ParentJointIndex, parentJointIndex, quint16, -1); DEFINE_PROPERTY_REF(PROP_QUERY_AA_CUBE, QueryAACube, queryAACube, AACube, AACube()); @@ -482,6 +482,16 @@ void EntityPropertyFlagsFromScriptValue(const QScriptValue& object, EntityProper inline void EntityItemProperties::setPosition(const glm::vec3& value) { _position = glm::clamp(value, (float)-HALF_TREE_SCALE, (float)HALF_TREE_SCALE); _positionChanged = true; } +inline void EntityItemProperties::setOwningAvatarID(const QUuid& id) { + _owningAvatarID = id; + if (!_owningAvatarID.isNull()) { + // for AvatarEntities there's no entity-server to tell us we're the simulation owner, + // so always set the simulationOwner to the owningAvatarID and a high priority. + setSimulationOwner(_owningAvatarID, AVATAR_ENTITY_SIMULATION_PRIORITY); + } + _owningAvatarIDChanged = true; +} + QDebug& operator<<(QDebug& dbg, const EntityPropertyFlags& f); inline QDebug operator<<(QDebug debug, const EntityItemProperties& properties) { diff --git a/libraries/entities/src/SimulationOwner.h b/libraries/entities/src/SimulationOwner.h index 353255728c..bd444d28dd 100644 --- a/libraries/entities/src/SimulationOwner.h +++ b/libraries/entities/src/SimulationOwner.h @@ -96,7 +96,7 @@ const uint8_t RECRUIT_SIMULATION_PRIORITY = VOLUNTEER_SIMULATION_PRIORITY + 1; // When poking objects with scripts an observer will bid at SCRIPT_EDIT priority. const uint8_t SCRIPT_GRAB_SIMULATION_PRIORITY = 128; const uint8_t SCRIPT_POKE_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY - 1; -const uint8_t AVATAR_ENTITY_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY + 1; +const uint8_t AVATAR_ENTITY_SIMULATION_PRIORITY = 255; // PERSONAL priority (needs a better name) is the level at which a simulation observer owns its own avatar // which really just means: things that collide with it will be bid at a priority level one lower diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 4b635ef0be..cf098136d3 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -307,6 +307,8 @@ const btCollisionShape* EntityMotionState::computeNewShape() { return getShapeManager()->getShape(shapeInfo); } +const uint8_t MAX_NUM_INACTIVE_UPDATES = 20; + bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { // NOTE: this method is only ever called when the entity simulation is locally owned DETAILED_PROFILE_RANGE(simulation_physics, "CheckOutOfSync"); @@ -316,15 +318,10 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { // TODO: need to be able to detect when logic dictates we *decrease* priority // WIP: print info whenever _bidPriority mismatches what is known to the entity - if (_entity->dynamicDataNeedsTransmit()) { - return true; - } - int numSteps = simulationStep - _lastStep; float dt = (float)(numSteps) * PHYSICS_ENGINE_FIXED_SUBSTEP; if (_numInactiveUpdates > 0) { - const uint8_t MAX_NUM_INACTIVE_UPDATES = 20; if (_numInactiveUpdates > MAX_NUM_INACTIVE_UPDATES) { // clear local ownership (stop sending updates) and let the server clear itself _entity->clearSimulationOwnership(); @@ -452,8 +449,13 @@ void EntityMotionState::updateSendVelocities() { if (!_body->isKinematicObject()) { clearObjectVelocities(); } - // we pretend we sent the inactive update for this object - _numInactiveUpdates = 1; + if (_entity->getEntityHostType() == entity::HostType::AVATAR) { + // AvatarEntities only ever need to send one update (their updates are sent over a lossless protocol) + // so we set the count to the max to prevent resends + _numInactiveUpdates = MAX_NUM_INACTIVE_UPDATES; + } else { + ++_numInactiveUpdates; + } } else { glm::vec3 gravity = _entity->getGravity(); From 6d791a80c28ebd22cd9f596647fed282164fd161 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Fri, 4 Jan 2019 19:04:01 -0800 Subject: [PATCH 08/17] Bump protocol version for BulkAvatarTraits and add some guard code around packet parsing. --- interface/src/Application.cpp | 2 +- libraries/avatars/src/AvatarHashMap.cpp | 29 ++++++++++++------- .../networking/src/udt/PacketHeaders.cpp | 1 + 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 6d870b58d6..04adc376e6 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2057,7 +2057,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo properties["avatar_ping"] = avatarMixerNode ? avatarMixerNode->getPingMs() : -1; properties["asset_ping"] = assetServerNode ? assetServerNode->getPingMs() : -1; properties["messages_ping"] = messagesMixerNode ? messagesMixerNode->getPingMs() : -1; - properties["atp_in_kbps"] = messagesMixerNode ? assetServerNode->getInboundKbps() : 0.0f; + properties["atp_in_kbps"] = assetServerNode ? assetServerNode->getInboundKbps() : 0.0f; auto loadingRequests = ResourceCache::getLoadingRequests(); diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 6a67ef6638..b3add74f9c 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -330,19 +330,25 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer void AvatarHashMap::processBulkAvatarTraits(QSharedPointer message, SharedNodePointer sendingNode) { AvatarTraits::TraitMessageSequence seq; - message->readPrimitive(&seq); + if (message->getBytesLeftToRead() > sizeof(AvatarTraits::TraitMessageSequence)) { + message->readPrimitive(&seq); - auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true); - traitsAckPacket->writePrimitive(seq); - auto nodeList = DependencyManager::get(); - SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); - if (!avatarMixer.isNull()) { - // we have a mixer to send to, acknowledge that we received these - // traits. - nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); + auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true); + traitsAckPacket->writePrimitive(seq); + auto nodeList = DependencyManager::get(); + SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); + if (!avatarMixer.isNull()) { + // we have a mixer to send to, acknowledge that we received these + // traits. + nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); + } + } + else { + qWarning() << "No BulkAvatarTraits packet sequence number."; + return; } - while (message->getBytesLeftToRead()) { + while (message->getBytesLeftToRead() >= NUM_BYTES_RFC4122_UUID + sizeof(AvatarTraits::TraitType)) { // read the avatar ID to figure out which avatar this is for auto avatarID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); @@ -407,6 +413,9 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess message->readPrimitive(&traitType); } } + if (message->getBytesLeftToRead() > 0) { + qWarning() << "Leftover bytes in BulkAvatarTraits message"; + } } void AvatarHashMap::processKillAvatar(QSharedPointer message, SharedNodePointer sendingNode) { diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 45aa0c2b22..a94d45efc9 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -97,6 +97,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::EntityQueryInitialResultsComplete: return static_cast(EntityVersion::ParticleSpin); case PacketType::BulkAvatarTraitsAck: + case PacketType::BulkAvatarTraits: return static_cast(AvatarMixerPacketVersion::AvatarTraitsAck); default: return 22; From 9e887585fae85fe957fe5f8f39a128858e2294b5 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Sat, 5 Jan 2019 13:49:33 -0800 Subject: [PATCH 09/17] Bad write length calculation was causing faults. --- assignment-client/src/avatars/AvatarMixerSlave.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 5f24ae1b64..6ad5a4dbf1 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -135,7 +135,7 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis // hold sending more traits until we've been acked that the last one we sent was received if (lastSentVersionRef == lastAckedVersionRef && lastReceivedVersions[traitType] > lastSentVersionRef) { - addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); + bytesWritten += addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); // there is an update to this trait, add it to the traits packet bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); // update the last sent version @@ -185,7 +185,7 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis continue; } if (!isDeleted && (sentInstanceIt == sentIDValuePairs.end() || receivedVersion > sentInstanceIt->value)) { - addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); + bytesWritten += addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); // this instance version exists and has never been sent or is newer so we need to send it bytesWritten += sendingAvatar->packTraitInstance(traitType, instanceID, traitsPacketList, receivedVersion); @@ -202,7 +202,7 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis pendingTraitVersions.instanceInsert(traitType, instanceID, receivedVersion); } else if (isDeleted && sentInstanceIt != sentIDValuePairs.end() && absoluteReceivedVersion > sentInstanceIt->value) { - addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); + bytesWritten += addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten); // this instance version was deleted and we haven't sent the delete to this client yet bytesWritten += AvatarTraits::packInstancedTraitDelete(traitType, instanceID, traitsPacketList, absoluteReceivedVersion); From 0a76554d572bb193d4b89461ab4957d4f9849f44 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Sat, 5 Jan 2019 14:04:26 -0800 Subject: [PATCH 10/17] Remove debugging code --- libraries/avatars/src/AvatarHashMap.cpp | 29 +++++++++---------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index b3add74f9c..6a67ef6638 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -330,25 +330,19 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer void AvatarHashMap::processBulkAvatarTraits(QSharedPointer message, SharedNodePointer sendingNode) { AvatarTraits::TraitMessageSequence seq; - if (message->getBytesLeftToRead() > sizeof(AvatarTraits::TraitMessageSequence)) { - message->readPrimitive(&seq); + message->readPrimitive(&seq); - auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true); - traitsAckPacket->writePrimitive(seq); - auto nodeList = DependencyManager::get(); - SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); - if (!avatarMixer.isNull()) { - // we have a mixer to send to, acknowledge that we received these - // traits. - nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); - } - } - else { - qWarning() << "No BulkAvatarTraits packet sequence number."; - return; + auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true); + traitsAckPacket->writePrimitive(seq); + auto nodeList = DependencyManager::get(); + SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); + if (!avatarMixer.isNull()) { + // we have a mixer to send to, acknowledge that we received these + // traits. + nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer); } - while (message->getBytesLeftToRead() >= NUM_BYTES_RFC4122_UUID + sizeof(AvatarTraits::TraitType)) { + while (message->getBytesLeftToRead()) { // read the avatar ID to figure out which avatar this is for auto avatarID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); @@ -413,9 +407,6 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess message->readPrimitive(&traitType); } } - if (message->getBytesLeftToRead() > 0) { - qWarning() << "Leftover bytes in BulkAvatarTraits message"; - } } void AvatarHashMap::processKillAvatar(QSharedPointer message, SharedNodePointer sendingNode) { From 963269f3f6fc57227745a64ec24b7da3636d9ef2 Mon Sep 17 00:00:00 2001 From: Ken Cooke Date: Tue, 8 Jan 2019 10:50:11 -0800 Subject: [PATCH 11/17] Apply master volume to audio streams when solo is active --- assignment-client/src/audio/AudioMixerSlave.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/audio/AudioMixerSlave.cpp b/assignment-client/src/audio/AudioMixerSlave.cpp index 7a6ab9c3e2..681d822e11 100644 --- a/assignment-client/src/audio/AudioMixerSlave.cpp +++ b/assignment-client/src/audio/AudioMixerSlave.cpp @@ -504,7 +504,7 @@ void AudioMixerSlave::addStream(AudioMixerClientData::MixableStream& mixableStre float distance = glm::max(glm::length(relativePosition), EPSILON); float azimuth = isEcho ? 0.0f : computeAzimuth(listeningNodeStream, listeningNodeStream, relativePosition); - float gain = 1.0f; + float gain = masterListenerGain; if (!isSoloing) { gain = computeGain(masterListenerGain, listeningNodeStream, *streamToAdd, relativePosition, distance, isEcho); } From 0297d337d57cdb52075b337732a864961ff68162 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Tue, 8 Jan 2019 10:50:16 -0800 Subject: [PATCH 12/17] Add some comments as per CR request --- .../src/avatars/AvatarMixerClientData.h | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 16054e3458..d1d6dd4d69 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -194,12 +194,26 @@ private: AvatarTraits::TraitMessageSequence _currentTraitsMessageSequence{ 0 }; + // Cache of trait versions sent in a given packet (indexed by sequence number) + // When an ack is received, the sequence number in the ack is used to look up + // the sent trait versions and they are copied to _perNodeAckedTraitVersions. + // We remember the data in _perNodePendingTraitVersions instead of requiring + // the client to return all of the versions for each trait it received in a given packet, + // reducing the size of the ack packet. std::unordered_map _perNodePendingTraitVersions; - std::unordered_map _lastSentTraitsTimestamps; - PerNodeTraitVersions _perNodeSentTraitVersions; + // Versions of traits that have been acked, which will be compared to incoming + // trait updates. Incoming updates going to a given node will be ignored if + // the ack for the previous packet (containing those versions) has not been + // received. PerNodeTraitVersions _perNodeAckedTraitVersions; + std::unordered_map _lastSentTraitsTimestamps; + + // cache of traits sent to a node which are compared to incoming traits to + // prevent sending traits that have already been sent. + PerNodeTraitVersions _perNodeSentTraitVersions; + std::atomic_bool _isIgnoreRadiusEnabled { false }; }; From 73d688610fc53c16d2ebf553d6fbebd9e3be4011 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 8 Jan 2019 18:33:00 -0800 Subject: [PATCH 13/17] Track stats for total AvatarData bytes, traits bytes and identity bytes --- assignment-client/src/avatars/AvatarMixer.cpp | 5 +++- .../src/avatars/AvatarMixerSlave.cpp | 17 ++++++----- .../src/avatars/AvatarMixerSlave.h | 28 +++++++++++++------ libraries/avatars/src/AvatarData.cpp | 4 +-- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index cb2f0636b9..8ace26f2e2 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -767,6 +767,9 @@ void AvatarMixer::sendStatsPacket() { float averageOverBudgetAvatars = averageNodes ? aggregateStats.overBudgetAvatars / averageNodes : 0.0f; slavesAggregatObject["sent_3_averageOverBudgetAvatars"] = TIGHT_LOOP_STAT(averageOverBudgetAvatars); + slavesAggregatObject["sent_4_averageDataBytes"] = TIGHT_LOOP_STAT(aggregateStats.numDataBytesSent); + slavesAggregatObject["sent_5_averageAvatarEntityBytes"] = TIGHT_LOOP_STAT(aggregateStats.numTraitsBytesSent); + slavesAggregatObject["sent_4_averageIdentityBytes"] = TIGHT_LOOP_STAT(aggregateStats.numIdentityBytesSent); slavesAggregatObject["timing_1_processIncomingPackets"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.processIncomingPacketsElapsedTime); slavesAggregatObject["timing_2_ignoreCalculation"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.ignoreCalculationElapsedTime); @@ -775,7 +778,7 @@ void AvatarMixer::sendStatsPacket() { slavesAggregatObject["timing_5_packetSending"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.packetSendingElapsedTime); slavesAggregatObject["timing_6_jobElapsedTime"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.jobElapsedTime); - statsObject["slaves_aggregate"] = slavesAggregatObject; + statsObject["slaves_aggregate (per frame)"] = slavesAggregatObject; _handleViewFrustumPacketElapsedTime = 0; _handleAvatarIdentityPacketElapsedTime = 0; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index cd9d164ef7..22a393a06c 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -73,7 +73,8 @@ int AvatarMixerSlave::sendIdentityPacket(NLPacketList& packetList, const AvatarM QByteArray individualData = nodeData->getConstAvatarData()->identityByteArray(); individualData.replace(0, NUM_BYTES_RFC4122_UUID, nodeData->getNodeID().toRfc4122()); // FIXME, this looks suspicious packetList.write(individualData); - _stats.numIdentityPackets++; + _stats.numIdentityPacketsSent++; + _stats.numIdentityBytesSent += individualData.size(); return individualData.size(); } else { return 0; @@ -109,7 +110,6 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis while (simpleReceivedIt != lastReceivedVersions.simpleCEnd()) { auto traitType = static_cast(std::distance(lastReceivedVersions.simpleCBegin(), simpleReceivedIt)); - auto lastReceivedVersion = *simpleReceivedIt; auto& lastSentVersionRef = lastSentVersions[traitType]; @@ -191,7 +191,8 @@ int AvatarMixerSlave::sendReplicatedIdentityPacket(const Node& agentNode, const auto identityPacket = NLPacketList::create(PacketType::ReplicatedAvatarIdentity, QByteArray(), true, true); identityPacket->write(individualData); DependencyManager::get()->sendPacketList(std::move(identityPacket), destinationNode); - _stats.numIdentityPackets++; + _stats.numIdentityPacketsSent++; + _stats.numIdentityBytesSent += individualData.size(); return individualData.size(); } else { return 0; @@ -539,8 +540,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) ++numPacketsSent; } - _stats.numPacketsSent += numPacketsSent; - _stats.numBytesSent += numAvatarDataBytes; + _stats.numDataPacketsSent += numPacketsSent; + _stats.numDataBytesSent += numAvatarDataBytes; // record the bytes sent for other avatar data in the AvatarMixerClientData nodeData->recordSentAvatarData(numAvatarDataBytes); @@ -550,6 +551,8 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) if (traitsPacketList->getNumPackets() >= 1) { // send the traits packet list + _stats.numTraitsBytesSent += traitBytesSent; + _stats.numTraitsPacketsSent += (int) traitsPacketList->getNumPackets(); nodeList->sendPacketList(std::move(traitsPacketList), *destinationNode); } @@ -685,8 +688,8 @@ void AvatarMixerSlave::broadcastAvatarDataToDownstreamMixer(const SharedNodePoin // close the current packet so that we're always sending something avatarPacketList->closeCurrentPacket(true); - _stats.numPacketsSent += (int)avatarPacketList->getNumPackets(); - _stats.numBytesSent += numAvatarDataBytes; + _stats.numDataPacketsSent += (int)avatarPacketList->getNumPackets(); + _stats.numDataBytesSent += numAvatarDataBytes; // send the replicated bulk avatar data auto nodeList = DependencyManager::get(); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index 2ef90af38e..5c38cda590 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -24,9 +24,12 @@ public: int nodesBroadcastedTo { 0 }; int downstreamMixersBroadcastedTo { 0 }; - int numPacketsSent { 0 }; - int numBytesSent { 0 }; - int numIdentityPackets { 0 }; + int numDataBytesSent { 0 }; + int numTraitsBytesSent { 0 }; + int numIdentityBytesSent { 0 }; + int numDataPacketsSent { 0 }; + int numTraitsPacketsSent { 0 }; + int numIdentityPacketsSent { 0 }; int numOthersIncluded { 0 }; int overBudgetAvatars { 0 }; @@ -45,9 +48,13 @@ public: // sending job stats nodesBroadcastedTo = 0; downstreamMixersBroadcastedTo = 0; - numPacketsSent = 0; - numBytesSent = 0; - numIdentityPackets = 0; + + numDataBytesSent = 0; + numTraitsBytesSent = 0; + numIdentityBytesSent = 0; + numDataPacketsSent = 0; + numTraitsPacketsSent = 0; + numIdentityPacketsSent = 0; numOthersIncluded = 0; overBudgetAvatars = 0; @@ -65,9 +72,12 @@ public: nodesBroadcastedTo += rhs.nodesBroadcastedTo; downstreamMixersBroadcastedTo += rhs.downstreamMixersBroadcastedTo; - numPacketsSent += rhs.numPacketsSent; - numBytesSent += rhs.numBytesSent; - numIdentityPackets += rhs.numIdentityPackets; + numDataBytesSent += rhs.numDataBytesSent; + numTraitsBytesSent += rhs.numTraitsBytesSent; + numIdentityBytesSent += rhs.numIdentityBytesSent; + numDataPacketsSent += rhs.numDataPacketsSent; + numTraitsPacketsSent += rhs.numTraitsPacketsSent; + numIdentityPacketsSent += rhs.numIdentityPacketsSent; numOthersIncluded += rhs.numOthersIncluded; overBudgetAvatars += rhs.overBudgetAvatars; diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index e72fa3a6eb..2f1c8d3d82 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1987,9 +1987,9 @@ qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTr qint64 bytesWritten = 0; if (traitType == AvatarTraits::AvatarEntity) { - packAvatarEntityTraitInstance(traitType, traitInstanceID, destination, traitVersion); + bytesWritten += packAvatarEntityTraitInstance(traitType, traitInstanceID, destination, traitVersion); } else if (traitType == AvatarTraits::Grab) { - packGrabTraitInstance(traitType, traitInstanceID, destination, traitVersion); + bytesWritten += packGrabTraitInstance(traitType, traitInstanceID, destination, traitVersion); } return bytesWritten; From cb6b86a236f8612eb4cdeeb451cf2628b7c59d32 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 9 Jan 2019 09:36:17 -0800 Subject: [PATCH 14/17] Tweak web display --- assignment-client/src/avatars/AvatarMixer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 8ace26f2e2..1415db7a27 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -768,8 +768,8 @@ void AvatarMixer::sendStatsPacket() { float averageOverBudgetAvatars = averageNodes ? aggregateStats.overBudgetAvatars / averageNodes : 0.0f; slavesAggregatObject["sent_3_averageOverBudgetAvatars"] = TIGHT_LOOP_STAT(averageOverBudgetAvatars); slavesAggregatObject["sent_4_averageDataBytes"] = TIGHT_LOOP_STAT(aggregateStats.numDataBytesSent); - slavesAggregatObject["sent_5_averageAvatarEntityBytes"] = TIGHT_LOOP_STAT(aggregateStats.numTraitsBytesSent); - slavesAggregatObject["sent_4_averageIdentityBytes"] = TIGHT_LOOP_STAT(aggregateStats.numIdentityBytesSent); + slavesAggregatObject["sent_5_averageTraitsBytes"] = TIGHT_LOOP_STAT(aggregateStats.numTraitsBytesSent); + slavesAggregatObject["sent_6_averageIdentityBytes"] = TIGHT_LOOP_STAT(aggregateStats.numIdentityBytesSent); slavesAggregatObject["timing_1_processIncomingPackets"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.processIncomingPacketsElapsedTime); slavesAggregatObject["timing_2_ignoreCalculation"] = TIGHT_LOOP_STAT_UINT64(aggregateStats.ignoreCalculationElapsedTime); From 3c6b7e3bd005d0a77ee6f94709b3c0f4f7485fa2 Mon Sep 17 00:00:00 2001 From: birarda Date: Tue, 18 Dec 2018 13:17:37 -0800 Subject: [PATCH 15/17] add a cap to cycled message channels in SendQueue --- libraries/networking/src/udt/PacketQueue.cpp | 46 +++++++++++++------- libraries/networking/src/udt/PacketQueue.h | 14 +++--- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/libraries/networking/src/udt/PacketQueue.cpp b/libraries/networking/src/udt/PacketQueue.cpp index 0560855ecb..b2e3000f74 100644 --- a/libraries/networking/src/udt/PacketQueue.cpp +++ b/libraries/networking/src/udt/PacketQueue.cpp @@ -16,7 +16,8 @@ using namespace udt; PacketQueue::PacketQueue(MessageNumber messageNumber) : _currentMessageNumber(messageNumber) { - _channels.emplace_back(new std::list()); + _channels.emplace_front(new std::list()); + _currentChannel = _channels.begin(); } MessageNumber PacketQueue::getNextMessageNumber() { @@ -27,21 +28,28 @@ MessageNumber PacketQueue::getNextMessageNumber() { bool PacketQueue::isEmpty() const { LockGuard locker(_packetsLock); + // Only the main channel and it is empty - return (_channels.size() == 1) && _channels.front()->empty(); + return _channels.size() == 1 && _channels.front()->empty(); } PacketQueue::PacketPointer PacketQueue::takePacket() { LockGuard locker(_packetsLock); + if (isEmpty()) { return PacketPointer(); } - // Find next non empty channel - if (_channels[nextIndex()]->empty()) { - nextIndex(); + // handle the case where we are looking at the first channel and it is empty + if (_currentChannel == _channels.begin() && (*_currentChannel)->empty()) { + ++_currentChannel; } - auto& channel = _channels[_currentIndex]; + + // at this point the current channel should always not be at the end and should also not be empty + Q_ASSERT(_currentChannel != _channels.end()); + + auto& channel = *_currentChannel; + Q_ASSERT(!channel->empty()); // Take front packet @@ -49,20 +57,28 @@ PacketQueue::PacketPointer PacketQueue::takePacket() { channel->pop_front(); // Remove now empty channel (Don't remove the main channel) - if (channel->empty() && _currentIndex != 0) { - channel->swap(*_channels.back()); - _channels.pop_back(); - --_currentIndex; + if (channel->empty() && _currentChannel != _channels.begin()) { + // erase the current channel and slide the iterator to the next channel + _currentChannel = _channels.erase(_currentChannel); + } else { + ++_currentChannel; + } + + // push forward our number of channels taken from + ++_channelsVisitedCount; + + // check if we need to restart back at the front channel (main) + // to respect our capped number of channels considered concurrently + static const int MAX_CHANNELS_SENT_CONCURRENTLY = 16; + + if (_currentChannel == _channels.end() || _channelsVisitedCount >= MAX_CHANNELS_SENT_CONCURRENTLY) { + _channelsVisitedCount = 0; + _currentChannel = _channels.begin(); } return packet; } -unsigned int PacketQueue::nextIndex() { - _currentIndex = (_currentIndex + 1) % _channels.size(); - return _currentIndex; -} - void PacketQueue::queuePacket(PacketPointer packet) { LockGuard locker(_packetsLock); _channels.front()->push_back(std::move(packet)); diff --git a/libraries/networking/src/udt/PacketQueue.h b/libraries/networking/src/udt/PacketQueue.h index bc4c5e3432..f696864e6b 100644 --- a/libraries/networking/src/udt/PacketQueue.h +++ b/libraries/networking/src/udt/PacketQueue.h @@ -30,8 +30,9 @@ class PacketQueue { using LockGuard = std::lock_guard; using PacketPointer = std::unique_ptr; using PacketListPointer = std::unique_ptr; - using Channel = std::unique_ptr>; - using Channels = std::vector; + using RawChannel = std::list; + using Channel = std::unique_ptr; + using Channels = std::list; public: PacketQueue(MessageNumber messageNumber = 0); @@ -47,16 +48,17 @@ public: private: MessageNumber getNextMessageNumber(); - unsigned int nextIndex(); - + MessageNumber _currentMessageNumber { 0 }; mutable Mutex _packetsLock; // Protects the packets to be sent. Channels _channels; // One channel per packet list + Main channel - unsigned int _currentIndex { 0 }; + + Channels::iterator _currentChannel; + unsigned int _channelsVisitedCount { 0 }; }; } -#endif // hifi_PacketQueue_h \ No newline at end of file +#endif // hifi_PacketQueue_h From 5bcdf143b9d01f459c689a18a0634225dbf836d1 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Wed, 9 Jan 2019 13:45:07 -0800 Subject: [PATCH 16/17] Add per-destination-avatar traits-rate stats --- assignment-client/src/avatars/AvatarMixer.cpp | 5 +++-- assignment-client/src/avatars/AvatarMixerClientData.cpp | 3 ++- assignment-client/src/avatars/AvatarMixerClientData.h | 9 ++++++++- assignment-client/src/avatars/AvatarMixerSlave.cpp | 7 ++++--- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 1415db7a27..62b58f3d73 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -803,7 +803,8 @@ void AvatarMixer::sendStatsPacket() { // add the key to ask the domain-server for a username replacement, if it has it avatarStats[USERNAME_UUID_REPLACEMENT_STATS_KEY] = uuidStringWithoutCurlyBraces(node->getUUID()); - avatarStats[NODE_OUTBOUND_KBPS_STAT_KEY] = node->getOutboundKbps(); + float outboundAvatarDataKbps = node->getOutboundKbps(); + avatarStats[NODE_OUTBOUND_KBPS_STAT_KEY] = outboundAvatarDataKbps; avatarStats[NODE_INBOUND_KBPS_STAT_KEY] = node->getInboundKbps(); AvatarMixerClientData* clientData = static_cast(node->getLinkedData()); @@ -814,7 +815,7 @@ void AvatarMixer::sendStatsPacket() { // add the diff between the full outbound bandwidth and the measured bandwidth for AvatarData send only avatarStats["delta_full_vs_avatar_data_kbps"] = - avatarStats[NODE_OUTBOUND_KBPS_STAT_KEY].toDouble() - avatarStats[OUTBOUND_AVATAR_DATA_STATS_KEY].toDouble(); + (double)outboundAvatarDataKbps - avatarStats[OUTBOUND_AVATAR_DATA_STATS_KEY].toDouble(); } } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index f215fd4448..7313634cff 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -317,7 +317,8 @@ void AvatarMixerClientData::loadJSONStats(QJsonObject& jsonObject) const { jsonObject["total_num_out_of_order_sends"] = _numOutOfOrderSends; jsonObject[OUTBOUND_AVATAR_DATA_STATS_KEY] = getOutboundAvatarDataKbps(); - jsonObject[INBOUND_AVATAR_DATA_STATS_KEY] = _avatar->getAverageBytesReceivedPerSecond() / (float) BYTES_PER_KILOBIT; + jsonObject[OUTBOUND_AVATAR_TRAITS_STATS_KEY] = getOutboundAvatarTraitsKbps(); + jsonObject[INBOUND_AVATAR_DATA_STATS_KEY] = _avatar->getAverageBytesReceivedPerSecond() / (float)BYTES_PER_KILOBIT; jsonObject["av_data_receive_rate"] = _avatar->getReceiveRate(); jsonObject["recent_other_av_in_view"] = _recentOtherAvatarsInView; diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 8a86af384a..18144b1e01 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -32,6 +32,7 @@ #include const QString OUTBOUND_AVATAR_DATA_STATS_KEY = "outbound_av_data_kbps"; +const QString OUTBOUND_AVATAR_TRAITS_STATS_KEY = "outbound_av_traits_kbps"; const QString INBOUND_AVATAR_DATA_STATS_KEY = "inbound_av_data_kbps"; struct SlaveSharedData; @@ -85,10 +86,15 @@ public: void incrementNumFramesSinceFRDAdjustment() { ++_numFramesSinceAdjustment; } void resetNumFramesSinceFRDAdjustment() { _numFramesSinceAdjustment = 0; } - void recordSentAvatarData(int numBytes) { _avgOtherAvatarDataRate.updateAverage((float) numBytes); } + void recordSentAvatarData(int numDataBytes, int numTraitsBytes = 0) { + _avgOtherAvatarDataRate.updateAverage(numDataBytes); + _avgOtherAvatarTraitsRate.updateAverage(numTraitsBytes); + } float getOutboundAvatarDataKbps() const { return _avgOtherAvatarDataRate.getAverageSampleValuePerSecond() / (float) BYTES_PER_KILOBIT; } + float getOutboundAvatarTraitsKbps() const + { return _avgOtherAvatarTraitsRate.getAverageSampleValuePerSecond() / BYTES_PER_KILOBIT; } void loadJSONStats(QJsonObject& jsonObject) const; @@ -171,6 +177,7 @@ private: int _numOutOfOrderSends = 0; SimpleMovingAverage _avgOtherAvatarDataRate; + SimpleMovingAverage _avgOtherAvatarTraitsRate; std::vector _radiusIgnoredOthers; ConicalViewFrustums _currentViewFrustums; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 22a393a06c..0460407f01 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -543,9 +543,6 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.numDataPacketsSent += numPacketsSent; _stats.numDataBytesSent += numAvatarDataBytes; - // record the bytes sent for other avatar data in the AvatarMixerClientData - nodeData->recordSentAvatarData(numAvatarDataBytes); - // close the current traits packet list traitsPacketList->closeCurrentPacket(); @@ -562,6 +559,10 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) nodeList->sendPacketList(std::move(identityPacketList), *destinationNode); } + // record the bytes sent for other avatar data in the AvatarMixerClientData + nodeData->recordSentAvatarData(numAvatarDataBytes, traitBytesSent); + + // record the number of avatars held back this frame nodeData->recordNumOtherAvatarStarves(numAvatarsHeldBack); nodeData->recordNumOtherAvatarSkips(numAvatarsWithSkippedFrames); From df97ca57b4cb1f86fd6b2266ed8afb8bc05d31c7 Mon Sep 17 00:00:00 2001 From: Roxanne Skelly Date: Thu, 10 Jan 2019 12:04:48 -0800 Subject: [PATCH 17/17] Bulk Avatar Traits related ack data was not reset when avatars were ignored, etc. --- assignment-client/src/avatars/AvatarMixerClientData.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 7f1b40142e..01cc9ff7a0 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -341,6 +341,10 @@ void AvatarMixerClientData::removeFromRadiusIgnoringSet(const QUuid& other) { void AvatarMixerClientData::resetSentTraitData(Node::LocalID nodeLocalID) { _lastSentTraitsTimestamps[nodeLocalID] = TraitsCheckTimestamp(); _perNodeSentTraitVersions[nodeLocalID].reset(); + _perNodeAckedTraitVersions[nodeLocalID].reset(); + for (auto && pendingTraitVersions : _perNodePendingTraitVersions) { + pendingTraitVersions.second[nodeLocalID].reset(); + } } void AvatarMixerClientData::readViewFrustumPacket(const QByteArray& message) {