From 9079f891e97a5c82d2a4f1137b9dca10e2b7e5a0 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 6 Oct 2015 14:09:21 -0700 Subject: [PATCH 1/5] repair broken node bandwidth stats --- libraries/networking/src/LimitedNodeList.cpp | 7 +++++++ libraries/networking/src/NetworkPeer.cpp | 8 ++++---- libraries/networking/src/NetworkPeer.h | 8 ++++---- libraries/networking/src/PacketReceiver.cpp | 2 ++ 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index c13a82f821..41789315c1 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -274,10 +274,14 @@ void LimitedNodeList::fillPacketHeader(const NLPacket& packet, const QUuid& conn qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode) { Q_ASSERT(!packet.isPartOfMessage()); + if (!destinationNode.getActiveSocket()) { return 0; } + emit dataSent(destinationNode.getType(), packet.getDataSize()); + destinationNode.recordBytesSent(packet.getDataSize()); + return sendUnreliablePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getConnectionSecret()); } @@ -298,7 +302,10 @@ qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& if (!destinationNode.getActiveSocket()) { return 0; } + emit dataSent(destinationNode.getType(), packet->getDataSize()); + destinationNode.recordBytesSent(packet->getDataSize()); + return sendPacket(std::move(packet), *destinationNode.getActiveSocket(), destinationNode.getConnectionSecret()); } diff --git a/libraries/networking/src/NetworkPeer.cpp b/libraries/networking/src/NetworkPeer.cpp index b17656aea0..9253243a7f 100644 --- a/libraries/networking/src/NetworkPeer.cpp +++ b/libraries/networking/src/NetworkPeer.cpp @@ -232,22 +232,22 @@ BandwidthRecorder& getBandwidthRecorder(const QUuid & uuid) { return *PEER_BANDWIDTH[uuid].data(); } -void NetworkPeer::recordBytesSent(int count) { +void NetworkPeer::recordBytesSent(int count) const { auto& bw = getBandwidthRecorder(_uuid); bw.updateOutboundData(0, count); } -void NetworkPeer::recordBytesReceived(int count) { +void NetworkPeer::recordBytesReceived(int count) const { auto& bw = getBandwidthRecorder(_uuid); bw.updateInboundData(0, count); } -float NetworkPeer::getOutboundBandwidth() { +float NetworkPeer::getOutboundBandwidth() const { auto& bw = getBandwidthRecorder(_uuid); return bw.getAverageOutputKilobitsPerSecond(0); } -float NetworkPeer::getInboundBandwidth() { +float NetworkPeer::getInboundBandwidth() const { auto& bw = getBandwidthRecorder(_uuid); return bw.getAverageInputKilobitsPerSecond(0); } diff --git a/libraries/networking/src/NetworkPeer.h b/libraries/networking/src/NetworkPeer.h index 8446586121..c10d44bfa9 100644 --- a/libraries/networking/src/NetworkPeer.h +++ b/libraries/networking/src/NetworkPeer.h @@ -70,11 +70,11 @@ public: void incrementConnectionAttempts() { ++_connectionAttempts; } void resetConnectionAttempts() { _connectionAttempts = 0; } - void recordBytesSent(int count); - void recordBytesReceived(int count); + void recordBytesSent(int count) const; + void recordBytesReceived(int count) const; - float getOutboundBandwidth(); // in kbps - float getInboundBandwidth(); // in kbps + float getOutboundBandwidth() const; // in kbps + float getInboundBandwidth() const; // in kbps friend QDataStream& operator<<(QDataStream& out, const NetworkPeer& peer); friend QDataStream& operator>>(QDataStream& in, NetworkPeer& peer); diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index 0efb8bba7c..9d25724f6c 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -404,6 +404,8 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { if (matchingNode) { emit dataReceived(matchingNode->getType(), nlPacket->getDataSize()); + matchingNode->recordBytesReceived(nlPacket->getDataSize()); + QMetaMethod metaMethod = listener.second; static const QByteArray QSHAREDPOINTER_NODE_NORMALIZED = QMetaObject::normalizedType("QSharedPointer"); From fbf6d166b74b3b033f18ffd002200a2b22ed9d8e Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 6 Oct 2015 14:48:28 -0700 Subject: [PATCH 2/5] add wire size to BasePacket --- libraries/networking/src/udt/BasePacket.cpp | 4 ---- libraries/networking/src/udt/BasePacket.h | 5 ++++- libraries/networking/src/udt/Constants.h | 3 ++- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/networking/src/udt/BasePacket.cpp b/libraries/networking/src/udt/BasePacket.cpp index 0d661ad34a..54342d9cba 100644 --- a/libraries/networking/src/udt/BasePacket.cpp +++ b/libraries/networking/src/udt/BasePacket.cpp @@ -126,10 +126,6 @@ BasePacket& BasePacket::operator=(BasePacket&& other) { return *this; } -qint64 BasePacket::getDataSize() const { - return (_payloadStart - _packet.get()) + _payloadSize; -} - void BasePacket::setPayloadSize(qint64 payloadSize) { if (isWritable()) { Q_ASSERT(payloadSize <= _payloadCapacity); diff --git a/libraries/networking/src/udt/BasePacket.h b/libraries/networking/src/udt/BasePacket.h index cc18c75a80..392a7c4384 100644 --- a/libraries/networking/src/udt/BasePacket.h +++ b/libraries/networking/src/udt/BasePacket.h @@ -48,7 +48,10 @@ public: const char* getData() const { return _packet.get(); } // Returns the size of the packet, including the header - qint64 getDataSize() const; + qint64 getDataSize() const { return (_payloadStart - _packet.get()) + _payloadSize; } + + // Returns the size of the packet, including the header AND the UDP/IP header + qint64 getWireSize() const { return getDataSize() + UDP_IPV4_HEADER_SIZE; } // Returns the size of the payload only qint64 getPayloadSize() const { return _payloadSize; } diff --git a/libraries/networking/src/udt/Constants.h b/libraries/networking/src/udt/Constants.h index 0152444f84..9f5f1db883 100644 --- a/libraries/networking/src/udt/Constants.h +++ b/libraries/networking/src/udt/Constants.h @@ -17,8 +17,9 @@ #include "SequenceNumber.h" namespace udt { + static const int UDP_IPV4_HEADER_SIZE = 28; static const int MAX_PACKET_SIZE_WITH_UDP_HEADER = 1500; - static const int MAX_PACKET_SIZE = MAX_PACKET_SIZE_WITH_UDP_HEADER - 28; + static const int MAX_PACKET_SIZE = MAX_PACKET_SIZE_WITH_UDP_HEADER - UDP_IPV4_HEADER_SIZE; static const int MAX_PACKETS_IN_FLIGHT = 25600; static const int CONNECTION_RECEIVE_BUFFER_SIZE_PACKETS = 8192; static const int CONNECTION_SEND_BUFFER_SIZE_PACKETS = 8192; From 593153deec3e0d53514fd8bdf325fe20804d0761 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 6 Oct 2015 14:58:51 -0700 Subject: [PATCH 3/5] add receive rate and inbound kbps to av stats --- assignment-client/src/avatars/AvatarMixer.cpp | 3 +++ .../src/avatars/AvatarMixerClientData.cpp | 11 +++++++---- assignment-client/src/avatars/AvatarMixerClientData.h | 1 + 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 66b924b8ab..820ff09a7f 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -485,10 +485,13 @@ void AvatarMixer::sendStatsPacket() { QJsonObject avatarStats; const QString NODE_OUTBOUND_KBPS_STAT_KEY = "outbound_kbps"; + const QString NODE_INBOUND_KBPS_STAT_KEY = "inbound_kbps"; // 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->getOutboundBandwidth(); + avatarStats[NODE_INBOUND_KBPS_STAT_KEY] = node->getInboundBandwidth(); AvatarMixerClientData* clientData = static_cast(node->getLinkedData()); if (clientData) { diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 006f58eb44..0a9be20691 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -40,11 +40,14 @@ uint16_t AvatarMixerClientData::getLastBroadcastSequenceNumber(const QUuid& node void AvatarMixerClientData::loadJSONStats(QJsonObject& jsonObject) const { jsonObject["display_name"] = _avatar.getDisplayName(); jsonObject["full_rate_distance"] = _fullRateDistance; - jsonObject["max_avatar_distance"] = _maxAvatarDistance; - jsonObject["num_avatars_sent_last_frame"] = _numAvatarsSentLastFrame; - jsonObject["avg_other_avatar_starves_per_second"] = getAvgNumOtherAvatarStarvesPerSecond(); - jsonObject["avg_other_avatar_skips_per_second"] = getAvgNumOtherAvatarSkipsPerSecond(); + jsonObject["max_av_distance"] = _maxAvatarDistance; + jsonObject["num_avs_sent_last_frame"] = _numAvatarsSentLastFrame; + jsonObject["avg_other_av_starves_per_second"] = getAvgNumOtherAvatarStarvesPerSecond(); + jsonObject["avg_other_av_skips_per_second"] = getAvgNumOtherAvatarSkipsPerSecond(); 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["av_data_receive_rate"] = _avatar.getReceiveRate(); } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 3dbe917ee2..e68a54c265 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -27,6 +27,7 @@ #include const QString OUTBOUND_AVATAR_DATA_STATS_KEY = "outbound_av_data_kbps"; +const QString INBOUND_AVATAR_DATA_STATS_KEY = "inbound_av_data_kbps"; class AvatarMixerClientData : public NodeData { Q_OBJECT From 4c695b179a0a238472dfa5854ffb2a228233e35c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 6 Oct 2015 15:04:48 -0700 Subject: [PATCH 4/5] remove out of order assert in avatar-mixer --- assignment-client/src/avatars/AvatarMixer.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 820ff09a7f..25f23ca580 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -262,14 +262,9 @@ void AvatarMixer::broadcastAvatarData() { AvatarDataSequenceNumber lastSeqToReceiver = nodeData->getLastBroadcastSequenceNumber(otherNode->getUUID()); AvatarDataSequenceNumber lastSeqFromSender = otherNodeData->getLastReceivedSequenceNumber(); - if (lastSeqToReceiver > lastSeqFromSender) { - // Did we somehow get out of order packets from the sender? - // We don't expect this to happen - in RELEASE we add this to a trackable stat - // and in DEBUG we crash on the assert - + if (lastSeqToReceiver > lastSeqFromSender && lastSeqToReceiver != UINT16_MAX) { + // we gout out of order packets from the sender, track it otherNodeData->incrementNumOutOfOrderSends(); - - assert(false); } // make sure we haven't already sent this data from this sender to this receiver From 0db88ba5726bfef27bb21db28e568ccf6e7919c4 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 6 Oct 2015 15:06:28 -0700 Subject: [PATCH 5/5] fix a typo in comment --- assignment-client/src/avatars/AvatarMixer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 25f23ca580..833b53b729 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -263,7 +263,7 @@ void AvatarMixer::broadcastAvatarData() { AvatarDataSequenceNumber lastSeqFromSender = otherNodeData->getLastReceivedSequenceNumber(); if (lastSeqToReceiver > lastSeqFromSender && lastSeqToReceiver != UINT16_MAX) { - // we gout out of order packets from the sender, track it + // we got out out of order packets from the sender, track it otherNodeData->incrementNumOutOfOrderSends(); }