From 0e3d3451d7d2356f6df6b2ee890e08d4ebdfc8dd Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 18 Nov 2015 10:24:10 -0800 Subject: [PATCH 1/3] make keep alive pings the responsibility of NL --- interface/src/Application.cpp | 11 ----- interface/src/Application.h | 1 - interface/src/Menu.cpp | 1 - interface/src/Menu.h | 1 - interface/src/ui/Stats.cpp | 49 ++++++++------------ libraries/networking/src/LimitedNodeList.cpp | 16 ------- libraries/networking/src/LimitedNodeList.h | 1 - libraries/networking/src/NodeList.cpp | 16 +++++++ libraries/networking/src/NodeList.h | 4 ++ 9 files changed, 40 insertions(+), 60 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index c77bb9a114..b1e5a676d6 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -999,10 +999,6 @@ void Application::initializeGL() { connect(&_octreeProcessor, &OctreePacketProcessor::packetVersionMismatch, this, &Application::notifyPacketVersionMismatch); _entityEditSender.initialize(_enableProcessOctreeThread); - // call our timer function every second - connect(&pingTimer, &QTimer::timeout, this, &Application::ping); - pingTimer.start(1000); - _idleLoopStdev.reset(); // update before the first render @@ -2130,13 +2126,6 @@ bool Application::acceptSnapshot(const QString& urlString) { return true; } -// Every second, send a ping, if menu item is checked. -void Application::ping() { - if (Menu::getInstance()->isOptionChecked(MenuOption::TestPing)) { - DependencyManager::get()->sendPingPackets(); - } -} - void Application::idle(uint64_t now) { if (_aboutToQuit) { return; // bail early, nothing to do here. diff --git a/interface/src/Application.h b/interface/src/Application.h index 39e3879707..39f93f4b72 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -306,7 +306,6 @@ public slots: private slots: void clearDomainOctreeDetails(); - void ping(); void idle(uint64_t now); void aboutToQuit(); diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 24033325f6..0f97f5a975 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -502,7 +502,6 @@ Menu::Menu() { addCheckableActionToQMenuAndActionHash(perfTimerMenu, MenuOption::ExpandOtherAvatarTiming, 0, false); addCheckableActionToQMenuAndActionHash(perfTimerMenu, MenuOption::ExpandPaintGLTiming, 0, false); - addCheckableActionToQMenuAndActionHash(timingMenu, MenuOption::TestPing, 0, true); addCheckableActionToQMenuAndActionHash(timingMenu, MenuOption::FrameTimer); addActionToQMenuAndActionHash(timingMenu, MenuOption::RunTimingTests, 0, qApp, SLOT(runTests())); addCheckableActionToQMenuAndActionHash(timingMenu, MenuOption::PipelineWarnings); diff --git a/interface/src/Menu.h b/interface/src/Menu.h index dfa2cfa41b..868afb0de8 100644 --- a/interface/src/Menu.h +++ b/interface/src/Menu.h @@ -280,7 +280,6 @@ namespace MenuOption { const QString Stats = "Stats"; const QString StopAllScripts = "Stop All Scripts"; const QString SuppressShortTimings = "Suppress Timings Less than 10ms"; - const QString TestPing = "Test Ping"; const QString ThirdPerson = "Third Person"; const QString ThreePointCalibration = "3 Point Calibration"; const QString ThrottleFPSIfNotFocus = "Throttle FPS If Not Focus"; // FIXME - this value duplicated in Basic2DWindowOpenGLDisplayPlugin.cpp diff --git a/interface/src/ui/Stats.cpp b/interface/src/ui/Stats.cpp index 1c0c03e16c..3127e00783 100644 --- a/interface/src/ui/Stats.cpp +++ b/interface/src/ui/Stats.cpp @@ -127,36 +127,27 @@ void Stats::updateStats(bool force) { STAT_UPDATE_FLOAT(mbpsOut, (float)bandwidthRecorder->getCachedTotalAverageOutputKilobitsPerSecond() / 1000.0f, 0.01f); // Second column: ping - if (Menu::getInstance()->isOptionChecked(MenuOption::TestPing)) { - SharedNodePointer audioMixerNode = nodeList->soloNodeOfType(NodeType::AudioMixer); - SharedNodePointer avatarMixerNode = nodeList->soloNodeOfType(NodeType::AvatarMixer); - SharedNodePointer assetServerNode = nodeList->soloNodeOfType(NodeType::AssetServer); - STAT_UPDATE(audioPing, audioMixerNode ? audioMixerNode->getPingMs() : -1); - STAT_UPDATE(avatarPing, avatarMixerNode ? avatarMixerNode->getPingMs() : -1); - STAT_UPDATE(assetPing, assetServerNode ? assetServerNode->getPingMs() : -1); - - //// Now handle entity servers, since there could be more than one, we average their ping times - int totalPingOctree = 0; - int octreeServerCount = 0; - int pingOctreeMax = 0; - nodeList->eachNode([&](const SharedNodePointer& node) { - // TODO: this should also support entities - if (node->getType() == NodeType::EntityServer) { - totalPingOctree += node->getPingMs(); - octreeServerCount++; - if (pingOctreeMax < node->getPingMs()) { - pingOctreeMax = node->getPingMs(); - } - } - }); - - // update the entities ping with the average for all connected entity servers - STAT_UPDATE(entitiesPing, octreeServerCount ? totalPingOctree / octreeServerCount : -1); - } else { - // -2 causes the QML to hide the ping column - STAT_UPDATE(audioPing, -2); - } + SharedNodePointer audioMixerNode = nodeList->soloNodeOfType(NodeType::AudioMixer); + SharedNodePointer avatarMixerNode = nodeList->soloNodeOfType(NodeType::AvatarMixer); + SharedNodePointer assetServerNode = nodeList->soloNodeOfType(NodeType::AssetServer); + STAT_UPDATE(audioPing, audioMixerNode ? audioMixerNode->getPingMs() : -1); + STAT_UPDATE(avatarPing, avatarMixerNode ? avatarMixerNode->getPingMs() : -1); + STAT_UPDATE(assetPing, assetServerNode ? assetServerNode->getPingMs() : -1); + //// Now handle entity servers, since there could be more than one, we average their ping times + int totalPingOctree = 0; + int octreeServerCount = 0; + int pingOctreeMax = 0; + nodeList->eachNode([&](const SharedNodePointer& node) { + // TODO: this should also support entities + if (node->getType() == NodeType::EntityServer) { + totalPingOctree += node->getPingMs(); + octreeServerCount++; + if (pingOctreeMax < node->getPingMs()) { + pingOctreeMax = node->getPingMs(); + } + } + }); // Third column, avatar stats MyAvatar* myAvatar = avatarManager->getMyAvatar(); diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 4a7844ecc7..e717856ca2 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -700,22 +700,6 @@ void LimitedNodeList::sendSTUNRequest() { _nodeSocket.writeDatagram(stunRequestPacket, sizeof(stunRequestPacket), _stunSockAddr); } -void LimitedNodeList::sendPingPackets() { - eachMatchingNode([](const SharedNodePointer& node)->bool { - switch (node->getType()) { - case NodeType::AvatarMixer: - case NodeType::AudioMixer: - case NodeType::EntityServer: - case NodeType::AssetServer: - return true; - default: - return false; - } - }, [&](const SharedNodePointer& node) { - sendPacket(constructPingPacket(), *node); - }); -} - void LimitedNodeList::processSTUNResponse(std::unique_ptr packet) { // check the cookie to make sure this is actually a STUN response // and read the first attribute and make sure it is a XOR_MAPPED_ADDRESS diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 26e648421a..8daeb3d256 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -228,7 +228,6 @@ public slots: void startSTUNPublicSocketUpdate(); virtual void sendSTUNRequest(); - void sendPingPackets(); bool killNodeWithUUID(const QUuid& nodeUUID); diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index e03ac47854..1e2485345f 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -32,6 +32,7 @@ #include "udt/PacketHeaders.h" #include "SharedUtil.h" +const int KEEPALIVE_PING_INTERVAL_MS = 1000; NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned short dtlsListenPort) : LimitedNodeList(socketListenPort, dtlsListenPort), @@ -87,6 +88,12 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned // anytime we get a new node we will want to attempt to punch to it connect(this, &LimitedNodeList::nodeAdded, this, &NodeList::startNodeHolePunch); + + // setup our timer to send keepalive pings (it's started and stopped on domain connect/disconnect) + _keepAlivePingTimer.setInterval(KEEPALIVE_PING_INTERVAL_MS); + connect(&_keepAlivePingTimer, &QTimer::timeout, this, &NodeList::sendKeepAlivePings); + connect(&_domainHandler, SIGNAL(connectedToDomain()), &_keepAlivePingTimer, SLOT(start())); + connect(&_domainHandler, &DomainHandler::disconnectedFromDomain, &_keepAlivePingTimer, &QTimer::stop); // we definitely want STUN to update our public socket, so call the LNL to kick that off startSTUNPublicSocketUpdate(); @@ -632,3 +639,12 @@ void NodeList::activateSocketFromNodeCommunication(QSharedPointer pack flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::SetAudioMixerSocket); } } + +void NodeList::sendKeepAlivePings() { + qDebug() << "Sending keepalive pings!"; + eachMatchingNode([this](const SharedNodePointer& node)->bool { + return _nodeTypesOfInterest.contains(node->getType()); + }, [&](const SharedNodePointer& node) { + sendPacket(constructPingPacket(), *node); + }); +} diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 5b9a4e5ae5..02f49d2918 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -85,6 +85,7 @@ public slots: void processPingReplyPacket(QSharedPointer packet, SharedNodePointer sendingNode); void processICEPingPacket(QSharedPointer packet); + signals: void limitOfSilentDomainCheckInsReached(); private slots: @@ -95,6 +96,8 @@ private slots: void handleNodePingTimeout(); void pingPunchForDomainServer(); + + void sendKeepAlivePings(); private: NodeList() : LimitedNodeList(0, 0) { assert(false); } // Not implemented, needed for DependencyManager templates compile NodeList(char ownerType, unsigned short socketListenPort = 0, unsigned short dtlsListenPort = 0); @@ -118,6 +121,7 @@ private: int _numNoReplyDomainCheckIns; HifiSockAddr _assignmentServerSocket; bool _isShuttingDown { false }; + QTimer _keepAlivePingTimer; }; #endif // hifi_NodeList_h From d90c073102ee8a7d98f145827c3dc6b409ccaba5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 18 Nov 2015 10:25:18 -0800 Subject: [PATCH 2/3] leverage NL keep alive pings in Agent --- assignment-client/src/Agent.cpp | 27 --------------------------- assignment-client/src/Agent.h | 2 -- 2 files changed, 29 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 1f56118177..8cc706992a 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -105,7 +105,6 @@ void Agent::handleAudioPacket(QSharedPointer packet) { } const QString AGENT_LOGGING_NAME = "agent"; -const int PING_INTERVAL = 1000; void Agent::run() { ThreadedAssignment::commonInit(AGENT_LOGGING_NAME, NodeType::Agent); @@ -127,10 +126,6 @@ void Agent::run() { << NodeType::MessagesMixer ); - _pingTimer = new QTimer(this); - connect(_pingTimer, SIGNAL(timeout()), SLOT(sendPingRequests())); - _pingTimer->start(PING_INTERVAL); - // figure out the URL for the script for this agent assignment QUrl scriptURL; if (_payload.isEmpty()) { @@ -383,28 +378,6 @@ void Agent::aboutToFinish() { _scriptEngine->stop(); } - if (_pingTimer) { - _pingTimer->stop(); - delete _pingTimer; - } - // our entity tree is going to go away so tell that to the EntityScriptingInterface DependencyManager::get()->setEntityTree(NULL); } - -void Agent::sendPingRequests() { - auto nodeList = DependencyManager::get(); - - nodeList->eachMatchingNode([](const SharedNodePointer& node)->bool { - switch (node->getType()) { - case NodeType::AvatarMixer: - case NodeType::AudioMixer: - case NodeType::EntityServer: - return true; - default: - return false; - } - }, [nodeList](const SharedNodePointer& node) { - nodeList->sendPacket(nodeList->constructPingPacket(), *node); - }); -} diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index ab000015d5..fe1fffce5a 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -58,14 +58,12 @@ private slots: void handleAudioPacket(QSharedPointer packet); void handleOctreePacket(QSharedPointer packet, SharedNodePointer senderNode); void handleJurisdictionPacket(QSharedPointer packet, SharedNodePointer senderNode); - void sendPingRequests(); void processAgentAvatarAndAudio(float deltaTime); private: std::unique_ptr _scriptEngine; EntityEditPacketSender _entityEditSender; EntityTreeHeadlessViewer _entityViewer; - QTimer* _pingTimer; MixedAudioStream _receivedAudioStream; float _lastReceivedAudioLoudness; From c54dffac1232aa5d4d8df61911d449a6aeceac31 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 18 Nov 2015 10:32:35 -0800 Subject: [PATCH 3/3] fix entity-server avg ping, remove debug --- interface/src/ui/Stats.cpp | 3 +++ libraries/networking/src/NodeList.cpp | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/interface/src/ui/Stats.cpp b/interface/src/ui/Stats.cpp index 3127e00783..c379f31aab 100644 --- a/interface/src/ui/Stats.cpp +++ b/interface/src/ui/Stats.cpp @@ -148,6 +148,9 @@ void Stats::updateStats(bool force) { } } }); + + // update the entities ping with the average for all connected entity servers + STAT_UPDATE(entitiesPing, octreeServerCount ? totalPingOctree / octreeServerCount : -1); // Third column, avatar stats MyAvatar* myAvatar = avatarManager->getMyAvatar(); diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 1e2485345f..925c64c77a 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -92,7 +92,7 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned // setup our timer to send keepalive pings (it's started and stopped on domain connect/disconnect) _keepAlivePingTimer.setInterval(KEEPALIVE_PING_INTERVAL_MS); connect(&_keepAlivePingTimer, &QTimer::timeout, this, &NodeList::sendKeepAlivePings); - connect(&_domainHandler, SIGNAL(connectedToDomain()), &_keepAlivePingTimer, SLOT(start())); + connect(&_domainHandler, SIGNAL(connectedToDomain(QString)), &_keepAlivePingTimer, SLOT(start())); connect(&_domainHandler, &DomainHandler::disconnectedFromDomain, &_keepAlivePingTimer, &QTimer::stop); // we definitely want STUN to update our public socket, so call the LNL to kick that off @@ -641,7 +641,6 @@ void NodeList::activateSocketFromNodeCommunication(QSharedPointer pack } void NodeList::sendKeepAlivePings() { - qDebug() << "Sending keepalive pings!"; eachMatchingNode([this](const SharedNodePointer& node)->bool { return _nodeTypesOfInterest.contains(node->getType()); }, [&](const SharedNodePointer& node) {