From b5e2913eb90bea2b1ee36bbb67dc7f94ad0b02b1 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 15 Aug 2016 09:22:56 -0700 Subject: [PATCH] Move node received bandwidth tracking to LimitedNodeList --- interface/src/Application.cpp | 2 +- libraries/networking/src/LimitedNodeList.cpp | 11 +++++++++-- libraries/networking/src/LimitedNodeList.h | 3 ++- libraries/networking/src/PacketReceiver.cpp | 18 ------------------ libraries/networking/src/PacketReceiver.h | 3 --- 5 files changed, 12 insertions(+), 25 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 3d519d1790..71bb7fbb66 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -845,7 +845,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : QSharedPointer bandwidthRecorder = DependencyManager::get(); connect(nodeList.data(), &LimitedNodeList::dataSent, bandwidthRecorder.data(), &BandwidthRecorder::updateOutboundData); - connect(&nodeList->getPacketReceiver(), &PacketReceiver::dataReceived, + connect(nodeList.data(), &LimitedNodeList::dataReceived, bandwidthRecorder.data(), &BandwidthRecorder::updateInboundData); // FIXME -- I'm a little concerned about this. diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 431d372089..6d9de2dbc1 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -175,7 +175,11 @@ QUdpSocket& LimitedNodeList::getDTLSSocket() { } bool LimitedNodeList::isPacketVerified(const udt::Packet& packet) { - return packetVersionMatch(packet) && packetSourceAndHashMatch(packet); + // We track bandwidth when doing packet verification to avoid needing to do a node lookup + // later when we already do it in packetSourceAndHashMatchAndTrackBandwidth. A node lookup + // incurs a lock, so it is ideal to avoid needing to do it 2+ times for each packet + // received. + return packetVersionMatch(packet) && packetSourceAndHashMatchAndTrackBandwidth(packet); } bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { @@ -224,11 +228,12 @@ bool LimitedNodeList::packetVersionMatch(const udt::Packet& packet) { } } -bool LimitedNodeList::packetSourceAndHashMatch(const udt::Packet& packet) { +bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet) { PacketType headerType = NLPacket::typeInHeader(packet); if (NON_SOURCED_PACKETS.contains(headerType)) { + emit dataReceived(NodeType::Unassigned, packet.getPayloadSize()); return true; } else { QUuid sourceID = NLPacket::sourceIDInHeader(packet); @@ -260,6 +265,8 @@ bool LimitedNodeList::packetSourceAndHashMatch(const udt::Packet& packet) { // from this sending node matchingNode->setLastHeardMicrostamp(usecTimestampNow()); + emit dataReceived(matchingNode->getType(), packet.getPayloadSize()); + return true; } else { diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 48379b5e39..49a3a155a2 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -243,6 +243,7 @@ public slots: signals: void dataSent(quint8 channelType, int bytes); + void dataReceived(quint8 channelType, int bytes); // QUuid might be zero for non-sourced packet types. void packetVersionMismatch(PacketType type, const HifiSockAddr& senderSockAddr, const QUuid& senderUUID); @@ -279,7 +280,7 @@ protected: void setLocalSocket(const HifiSockAddr& sockAddr); - bool packetSourceAndHashMatch(const udt::Packet& packet); + bool packetSourceAndHashMatchAndTrackBandwidth(const udt::Packet& packet); void processSTUNResponse(std::unique_ptr packet); void handleNodeKill(const SharedNodePointer& node); diff --git a/libraries/networking/src/PacketReceiver.cpp b/libraries/networking/src/PacketReceiver.cpp index df38a68515..21ba7129d4 100644 --- a/libraries/networking/src/PacketReceiver.cpp +++ b/libraries/networking/src/PacketReceiver.cpp @@ -215,15 +215,6 @@ void PacketReceiver::handleVerifiedPacket(std::unique_ptr packet) { _inPacketCount += 1; _inByteCount += nlPacket->size(); - SharedNodePointer matchingNode; - NodeType_t nodeType = NodeType::Unassigned; - if (!nlPacket->getSourceID().isNull()) { - auto nodeList = DependencyManager::get(); - matchingNode = nodeList->nodeWithUUID(nlPacket->getSourceID()); - nodeType = matchingNode->getType(); - } - emit dataReceived(nodeType, nlPacket->getPayloadSize()); - handleVerifiedMessage(receivedMessage, true); } @@ -233,15 +224,6 @@ void PacketReceiver::handleVerifiedMessagePacket(std::unique_ptr pa _inPacketCount += 1; _inByteCount += nlPacket->size(); - SharedNodePointer matchingNode; - NodeType_t nodeType = NodeType::Unassigned; - if (!nlPacket->getSourceID().isNull()) { - auto nodeList = DependencyManager::get(); - matchingNode = nodeList->nodeWithUUID(nlPacket->getSourceID()); - nodeType = matchingNode->getType(); - } - emit dataReceived(nodeType, nlPacket->getPayloadSize()); - auto key = std::pair(nlPacket->getSenderSockAddr(), nlPacket->getMessageNumber()); auto it = _pendingMessages.find(key); QSharedPointer message; diff --git a/libraries/networking/src/PacketReceiver.h b/libraries/networking/src/PacketReceiver.h index ff4ab3e15c..4b4d260409 100644 --- a/libraries/networking/src/PacketReceiver.h +++ b/libraries/networking/src/PacketReceiver.h @@ -67,9 +67,6 @@ public: void handleVerifiedPacket(std::unique_ptr packet); void handleVerifiedMessagePacket(std::unique_ptr message); void handleMessageFailure(HifiSockAddr from, udt::Packet::MessageNumber messageNumber); - -signals: - void dataReceived(quint8 channelType, int bytes); private: struct Listener {