From 1203aae96b4a541708c9fe938f390cdafcf5c144 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 18 Oct 2016 10:29:09 -0700 Subject: [PATCH 1/5] don't create ScriptEngines for ACs that don't need them --- assignment-client/src/AssignmentClient.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 800f00b352..0577d8c02b 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -53,10 +53,14 @@ AssignmentClient::AssignmentClient(Assignment::Type requestAssignmentType, QStri QSettings::setDefaultFormat(QSettings::IniFormat); DependencyManager::set(); - + auto scriptableAvatar = DependencyManager::set(); auto addressManager = DependencyManager::set(); - auto scriptEngines = DependencyManager::set(); + + if (requestAssignmentType != Assignment::AvatarMixerType && + requestAssignmentType != Assignment::AudioMixerType) { + auto scriptEngines = DependencyManager::set(); + } // create a NodeList as an unassigned client, must be after addressManager auto nodeList = DependencyManager::set(NodeType::Unassigned, listenPort); From 0cda01f105304010d94f6343ded155d58569c9ce Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 18 Oct 2016 10:29:35 -0700 Subject: [PATCH 2/5] send stats to domain-server with networking thread --- libraries/networking/src/NodeList.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index f2b28a04a7..e233860d18 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -251,11 +251,16 @@ void NodeList::addSetOfNodeTypesToNodeInterestSet(const NodeSet& setOfNodeTypes) } void NodeList::sendDomainServerCheckIn() { + if (thread() != QThread::currentThread()) { + QMetaObject::invokeMethod(this, "sendDomainServerCheckIn", Qt::QueuedConnection); + return; + } + if (_isShuttingDown) { qCDebug(networking) << "Refusing to send a domain-server check in while shutting down."; return; } - + if (_publicSockAddr.isNull()) { // we don't know our public socket and we need to send it to the domain server qCDebug(networking) << "Waiting for inital public socket from STUN. Will not send domain-server check in."; From 0c6dedff4588b6e256bd26516977e82b71d34f42 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 18 Oct 2016 13:05:53 -0700 Subject: [PATCH 3/5] avoid various data-races related to stats collection and delivery --- assignment-client/src/audio/AudioMixer.cpp | 2 +- libraries/networking/src/LimitedNodeList.h | 5 ++- libraries/networking/src/NodeList.cpp | 19 +++++++--- libraries/networking/src/NodeList.h | 4 +-- libraries/shared/src/SimpleMovingAverage.cpp | 20 +++++++++-- libraries/shared/src/SimpleMovingAverage.h | 38 +++++++++++++------- 6 files changed, 63 insertions(+), 25 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index ccd80a4a11..08e4ff8972 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -636,7 +636,7 @@ QString AudioMixer::percentageForMixStats(int counter) { } void AudioMixer::sendStatsPacket() { - static QJsonObject statsObject; + QJsonObject statsObject; statsObject["useDynamicJitterBuffers"] = _numStaticJitterFrames == -1; statsObject["trailing_sleep_percentage"] = _trailingSleepRatio * 100.0f; diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 2f9462ae8b..598964c2b7 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -321,9 +321,8 @@ protected: PacketReceiver* _packetReceiver; - // XXX can BandwidthRecorder be used for this? - int _numCollectedPackets; - int _numCollectedBytes; + std::atomic _numCollectedPackets; + std::atomic _numCollectedBytes; QElapsedTimer _packetStatTimer; NodePermissions _permissions; diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index e233860d18..7fa8f13652 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -127,19 +127,30 @@ NodeList::NodeList(char newOwnerType, int socketListenPort, int dtlsListenPort) packetReceiver.registerListener(PacketType::DomainServerRemovedNode, this, "processDomainServerRemovedNode"); } -qint64 NodeList::sendStats(const QJsonObject& statsObject, const HifiSockAddr& destination) { +qint64 NodeList::sendStats(QJsonObject statsObject, HifiSockAddr destination) { + if (thread() != QThread::currentThread()) { + QMetaObject::invokeMethod(this, "sendStats", Qt::QueuedConnection, + Q_ARG(QJsonObject, statsObject), + Q_ARG(HifiSockAddr, destination)); + return 0; + } + auto statsPacketList = NLPacketList::create(PacketType::NodeJsonStats, QByteArray(), true, true); QJsonDocument jsonDocument(statsObject); statsPacketList->write(jsonDocument.toBinaryData()); sendPacketList(std::move(statsPacketList), destination); - - // enumerate the resulting strings, breaking them into MTU sized packets return 0; } -qint64 NodeList::sendStatsToDomainServer(const QJsonObject& statsObject) { +qint64 NodeList::sendStatsToDomainServer(QJsonObject statsObject) { + if (thread() != QThread::currentThread()) { + QMetaObject::invokeMethod(this, "sendStatsToDomainServer", Qt::QueuedConnection, + Q_ARG(QJsonObject, statsObject)); + return 0; + } + return sendStats(statsObject, _domainHandler.getSockAddr()); } diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index 7f98b8c736..259ce7c6fe 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -54,8 +54,8 @@ public: NodeType_t getOwnerType() const { return _ownerType; } void setOwnerType(NodeType_t ownerType) { _ownerType = ownerType; } - qint64 sendStats(const QJsonObject& statsObject, const HifiSockAddr& destination); - qint64 sendStatsToDomainServer(const QJsonObject& statsObject); + Q_INVOKABLE qint64 sendStats(QJsonObject statsObject, HifiSockAddr destination); + Q_INVOKABLE qint64 sendStatsToDomainServer(QJsonObject statsObject); int getNumNoReplyDomainCheckIns() const { return _numNoReplyDomainCheckIns; } DomainHandler& getDomainHandler() { return _domainHandler; } diff --git a/libraries/shared/src/SimpleMovingAverage.cpp b/libraries/shared/src/SimpleMovingAverage.cpp index e1c9a27390..f75180afb5 100644 --- a/libraries/shared/src/SimpleMovingAverage.cpp +++ b/libraries/shared/src/SimpleMovingAverage.cpp @@ -19,15 +19,29 @@ SimpleMovingAverage::SimpleMovingAverage(int numSamplesToAverage) : _eventDeltaAverage(0.0f), WEIGHTING(1.0f / numSamplesToAverage), ONE_MINUS_WEIGHTING(1 - WEIGHTING) { - } +SimpleMovingAverage::SimpleMovingAverage(const SimpleMovingAverage& other) { + *this = other; +} + +SimpleMovingAverage& SimpleMovingAverage::operator=(const SimpleMovingAverage& other) { + _numSamples = (int)other._numSamples; + _lastEventTimestamp = (uint64_t)other._lastEventTimestamp; + _average = (float)other._average; + _eventDeltaAverage = (float)other._eventDeltaAverage; + WEIGHTING = other.WEIGHTING; + ONE_MINUS_WEIGHTING = other.ONE_MINUS_WEIGHTING; + return *this; +} + + int SimpleMovingAverage::updateAverage(float sample) { if (_numSamples > 0) { _average = (ONE_MINUS_WEIGHTING * _average) + (WEIGHTING * sample); - + float eventDelta = (usecTimestampNow() - _lastEventTimestamp) / 1000000.0f; - + if (_numSamples > 1) { _eventDeltaAverage = (ONE_MINUS_WEIGHTING * _eventDeltaAverage) + (WEIGHTING * eventDelta); diff --git a/libraries/shared/src/SimpleMovingAverage.h b/libraries/shared/src/SimpleMovingAverage.h index dd25705f7e..858ee21371 100644 --- a/libraries/shared/src/SimpleMovingAverage.h +++ b/libraries/shared/src/SimpleMovingAverage.h @@ -16,27 +16,31 @@ #include #include +#include class SimpleMovingAverage { public: SimpleMovingAverage(int numSamplesToAverage = 100); - + SimpleMovingAverage(const SimpleMovingAverage& other); + SimpleMovingAverage& operator=(const SimpleMovingAverage& other); + int updateAverage(float sample); void reset(); - + int getSampleCount() const { return _numSamples; }; float getAverage() const { return _average; }; float getEventDeltaAverage() const; // returned in seconds float getAverageSampleValuePerSecond() const { return _average * (1.0f / getEventDeltaAverage()); } - + uint64_t getUsecsSinceLastEvent() const; + private: - int _numSamples; - uint64_t _lastEventTimestamp; - float _average; - float _eventDeltaAverage; - + std::atomic _numSamples; + std::atomic _lastEventTimestamp; + std::atomic _average; + std::atomic _eventDeltaAverage; + float WEIGHTING; float ONE_MINUS_WEIGHTING; }; @@ -44,10 +48,20 @@ private: template class MovingAverage { public: + MovingAverage() {} + MovingAverage(const MovingAverage& other) { + *this = other; + } + MovingAverage& operator=(const MovingAverage& other) { + numSamples = (int)other.numSamples; + average = (T)other.average; + return *this; + } + const float WEIGHTING = 1.0f / (float)MAX_NUM_SAMPLES; const float ONE_MINUS_WEIGHTING = 1.0f - WEIGHTING; - int numSamples{ 0 }; - T average; + std::atomic numSamples{ 0 }; + std::atomic average; void clear() { numSamples = 0; @@ -72,7 +86,7 @@ public: _samples = 0; } - bool isAverageValid() const { + bool isAverageValid() const { std::unique_lock lock(_lock); return (_samples > 0); } @@ -87,7 +101,7 @@ public: _samples++; } - T getAverage() const { + T getAverage() const { std::unique_lock lock(_lock); return _average; } From 9cb4756fce54c42634c401e0179b8f9ba1ad034c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 18 Oct 2016 13:57:30 -0700 Subject: [PATCH 4/5] only ScriptEngines for agent ACs --- assignment-client/src/AssignmentClient.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index 0577d8c02b..2bba2e2427 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -57,8 +57,7 @@ AssignmentClient::AssignmentClient(Assignment::Type requestAssignmentType, QStri auto scriptableAvatar = DependencyManager::set(); auto addressManager = DependencyManager::set(); - if (requestAssignmentType != Assignment::AvatarMixerType && - requestAssignmentType != Assignment::AudioMixerType) { + if (requestAssignmentType == Assignment::AgentType) { auto scriptEngines = DependencyManager::set(); } From 21ccc58e04ff99f701c123810bcab9c9198b2f24 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 18 Oct 2016 14:41:33 -0700 Subject: [PATCH 5/5] don't pass statsObject by reference --- libraries/networking/src/ThreadedAssignment.cpp | 3 +-- libraries/networking/src/ThreadedAssignment.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/ThreadedAssignment.cpp b/libraries/networking/src/ThreadedAssignment.cpp index 9277aa70cc..906bc240e6 100644 --- a/libraries/networking/src/ThreadedAssignment.cpp +++ b/libraries/networking/src/ThreadedAssignment.cpp @@ -87,11 +87,10 @@ void ThreadedAssignment::commonInit(const QString& targetName, NodeType_t nodeTy connect(&nodeList->getDomainHandler(), &DomainHandler::disconnectedFromDomain, &_statsTimer, &QTimer::stop); } -void ThreadedAssignment::addPacketStatsAndSendStatsPacket(QJsonObject &statsObject) { +void ThreadedAssignment::addPacketStatsAndSendStatsPacket(QJsonObject statsObject) { auto nodeList = DependencyManager::get(); float packetsPerSecond, bytesPerSecond; - // XXX can BandwidthRecorder be used for this? nodeList->getPacketStats(packetsPerSecond, bytesPerSecond); nodeList->resetPacketStats(); diff --git a/libraries/networking/src/ThreadedAssignment.h b/libraries/networking/src/ThreadedAssignment.h index 13b9b5bf79..e9832a2a91 100644 --- a/libraries/networking/src/ThreadedAssignment.h +++ b/libraries/networking/src/ThreadedAssignment.h @@ -26,7 +26,7 @@ public: void setFinished(bool isFinished); virtual void aboutToFinish() { }; - void addPacketStatsAndSendStatsPacket(QJsonObject& statsObject); + void addPacketStatsAndSendStatsPacket(QJsonObject statsObject); public slots: /// threaded run of assignment