From c48be75a377a9c9557f4282c7d36046d7a634ebf Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Mon, 21 Mar 2016 14:49:42 -0700 Subject: [PATCH 01/15] Make sure everyone calls sendPeriod's setter --- .../networking/src/udt/CongestionControl.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/libraries/networking/src/udt/CongestionControl.cpp b/libraries/networking/src/udt/CongestionControl.cpp index bac178377e..8eff5e3a01 100644 --- a/libraries/networking/src/udt/CongestionControl.cpp +++ b/libraries/networking/src/udt/CongestionControl.cpp @@ -39,7 +39,7 @@ DefaultCC::DefaultCC() : _mss = udt::MAX_PACKET_SIZE_WITH_UDP_HEADER; _congestionWindowSize = 16.0; - _packetSendPeriod = 1.0; + setPacketSendPeriod(1.0); } void DefaultCC::onACK(SequenceNumber ackNum) { @@ -73,10 +73,10 @@ void DefaultCC::onACK(SequenceNumber ackNum) { if (_receiveRate > 0) { // if we have a valid receive rate we set the send period to whatever the receive rate dictates - _packetSendPeriod = USECS_PER_SECOND / _receiveRate; + setPacketSendPeriod(USECS_PER_SECOND / _receiveRate); } else { // no valid receive rate, packet send period is dictated by estimated RTT and current congestion window size - _packetSendPeriod = (_rtt + synInterval()) / _congestionWindowSize; + setPacketSendPeriod((_rtt + synInterval()) / _congestionWindowSize); } } } else { @@ -148,8 +148,8 @@ void DefaultCC::onLoss(SequenceNumber rangeStart, SequenceNumber rangeEnd) { if (rangeStart > _lastDecreaseMaxSeq) { _lastDecreasePeriod = _packetSendPeriod; - - _packetSendPeriod = ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE); + + setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); // use EWMA to update the average number of NAKs per congestion static const double NAK_EWMA_ALPHA = 0.125; @@ -175,7 +175,7 @@ void DefaultCC::onLoss(SequenceNumber rangeStart, SequenceNumber rangeEnd) { // there have been fewer than MAX_DECREASES_PER_CONGESTION_EPOCH AND this NAK matches the random count at which we // decided we would decrease the packet send period - _packetSendPeriod = ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE); + setPacketSendPeriod(ceil(_packetSendPeriod * INTER_PACKET_ARRIVAL_INCREASE)); _lastDecreaseMaxSeq = _sendCurrSeqNum; } } @@ -198,12 +198,12 @@ void DefaultCC::stopSlowStart() { if (_receiveRate > 0) { // Set the sending rate to the receiving rate. - _packetSendPeriod = USECS_PER_SECOND / _receiveRate; + setPacketSendPeriod(USECS_PER_SECOND / _receiveRate); } else { // If no receiving rate is observed, we have to compute the sending // rate according to the current window size, and decrease it // using the method below. - _packetSendPeriod = _congestionWindowSize / (_rtt + synInterval()); + setPacketSendPeriod(_congestionWindowSize / (_rtt + synInterval())); } } From 6d2498b4f23503ae2c2a3bdaa30626f3e8ad1ac6 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 16 Mar 2016 16:56:40 -0700 Subject: [PATCH 02/15] Remove debug print --- libraries/shared/src/LogHandler.cpp | 1 - libraries/ui/src/QmlWindowClass.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp index a338dfe8c2..b67c86ecef 100644 --- a/libraries/shared/src/LogHandler.cpp +++ b/libraries/shared/src/LogHandler.cpp @@ -82,7 +82,6 @@ void LogHandler::flushRepeatedMessages() { } QString LogHandler::printMessage(LogMsgType type, const QMessageLogContext& context, const QString& message) { - if (message.isEmpty()) { return QString(); } diff --git a/libraries/ui/src/QmlWindowClass.cpp b/libraries/ui/src/QmlWindowClass.cpp index 5bd40b10a9..c0826420c6 100644 --- a/libraries/ui/src/QmlWindowClass.cpp +++ b/libraries/ui/src/QmlWindowClass.cpp @@ -114,7 +114,6 @@ QScriptValue QmlWindowClass::internalConstructor(const QString& qmlSource, } } else { auto argumentObject = context->argument(0); - qDebug() << argumentObject.toString(); if (!argumentObject.property(TITLE_PROPERTY).isUndefined()) { title = argumentObject.property(TITLE_PROPERTY).toString(); } From d5f6ca5e1ce9f9ab6f3ac41be3b27bb5d8ef4e98 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 17 Mar 2016 09:49:59 -0700 Subject: [PATCH 03/15] Add max bandwidth setting --- assignment-client/src/assets/AssetServer.cpp | 12 ++++++++++++ domain-server/resources/describe-settings.json | 9 +++++++++ 2 files changed, 21 insertions(+) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index f5f83399d6..fad8ece7bf 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -77,6 +77,18 @@ void AssetServer::completeSetup() { auto assetServerObject = settingsObject[ASSET_SERVER_SETTINGS_KEY].toObject(); + static const QString MAX_BANDWIDTH_OPTION = "max_bandwidth"; + auto maxBandwidthValue = assetServerObject[MAX_BANDWIDTH_OPTION]; + auto maxBandwidthFloat = maxBandwidthValue.toDouble(-1); + + if (maxBandwidthFloat > 0.0) { + const int BYTES_PER_MEGABITS = (1024 * 1024) / 8; + int maxBandwidth = maxBandwidthFloat * BYTES_PER_MEGABITS; + nodeList->setConnectionMaxBandwidth(maxBandwidth); + qInfo() << "Set maximum bandwith per connection to" << maxBandwidthFloat << "Mb/s." + " (" << maxBandwidth << "bytes/sec)"; + } + // get the path to the asset folder from the domain server settings static const QString ASSETS_PATH_OPTION = "assets_path"; auto assetsJSONValue = assetServerObject[ASSETS_PATH_OPTION]; diff --git a/domain-server/resources/describe-settings.json b/domain-server/resources/describe-settings.json index cde398e01f..80ee32efa1 100644 --- a/domain-server/resources/describe-settings.json +++ b/domain-server/resources/describe-settings.json @@ -186,6 +186,15 @@ "help": "The path to the directory assets are stored in.
If this path is relative, it will be relative to the application data directory.
If you change this path you will need to manually copy any existing assets from the previous directory.", "default": "", "advanced": true + }, + { + "name": "max_bandwidth", + "type": "double", + "label": "Max Bandwidth Per User", + "help": "The maximum upstream bandwidth each user can use (in Mb/s).", + "placeholder": "10.0", + "default": "", + "advanced": true } ] }, From 170433997c45e9b399461c22c725d0cc9d4e799f Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 17 Mar 2016 13:26:01 -0700 Subject: [PATCH 04/15] Set Asset server max bandwidth wiring --- libraries/networking/src/LimitedNodeList.h | 2 ++ .../networking/src/udt/CongestionControl.cpp | 12 +++++++++--- libraries/networking/src/udt/CongestionControl.h | 5 +++-- libraries/networking/src/udt/Connection.cpp | 4 ++++ libraries/networking/src/udt/Connection.h | 2 ++ libraries/networking/src/udt/Socket.cpp | 15 ++++++++++++++- libraries/networking/src/udt/Socket.h | 7 +++++-- 7 files changed, 39 insertions(+), 8 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index 43393ef69c..0cbe9668b3 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -219,6 +219,8 @@ public: udt::Socket::StatsVector sampleStatsForAllConnections() { return _nodeSocket.sampleStatsForAllConnections(); } + void setConnectionMaxBandwidth(int maxBandwidth) { _nodeSocket.setConnectionMaxBandwidth(maxBandwidth); } + public slots: void reset(); void eraseAllNodes(); diff --git a/libraries/networking/src/udt/CongestionControl.cpp b/libraries/networking/src/udt/CongestionControl.cpp index 8eff5e3a01..1d1a6628fe 100644 --- a/libraries/networking/src/udt/CongestionControl.cpp +++ b/libraries/networking/src/udt/CongestionControl.cpp @@ -20,13 +20,19 @@ using namespace std::chrono; static const double USECS_PER_SECOND = 1000000.0; +void CongestionControl::setMaxBandwidth(int maxBandwidth) { + _maxBandwidth = maxBandwidth; + setPacketSendPeriod(_packetSendPeriod); +} + void CongestionControl::setPacketSendPeriod(double newSendPeriod) { Q_ASSERT_X(newSendPeriod >= 0, "CongestionControl::setPacketPeriod", "Can not set a negative packet send period"); - - if (_maxBandwidth > 0) { + + auto maxBandwidth = _maxBandwidth.load(); + if (maxBandwidth > 0) { // anytime the packet send period is about to be increased, make sure it stays below the minimum period, // calculated based on the maximum desired bandwidth - double minPacketSendPeriod = USECS_PER_SECOND / (((double) _maxBandwidth) / _mss); + double minPacketSendPeriod = USECS_PER_SECOND / (((double) maxBandwidth) / _mss); _packetSendPeriod = std::max(newSendPeriod, minPacketSendPeriod); } else { _packetSendPeriod = newSendPeriod; diff --git a/libraries/networking/src/udt/CongestionControl.h b/libraries/networking/src/udt/CongestionControl.h index 3a5c8d0d00..8297b5f6bd 100644 --- a/libraries/networking/src/udt/CongestionControl.h +++ b/libraries/networking/src/udt/CongestionControl.h @@ -12,6 +12,7 @@ #ifndef hifi_CongestionControl_h #define hifi_CongestionControl_h +#include #include #include #include @@ -37,6 +38,7 @@ public: virtual ~CongestionControl() {} int synInterval() const { return _synInterval; } + void setMaxBandwidth(int maxBandwidth); virtual void init() {} virtual void onACK(SequenceNumber ackNum) {} @@ -49,7 +51,6 @@ protected: void setMSS(int mss) { _mss = mss; } void setMaxCongestionWindowSize(int window) { _maxCongestionWindowSize = window; } void setBandwidth(int bandwidth) { _bandwidth = bandwidth; } - void setMaxBandwidth(int maxBandwidth) { _maxBandwidth = maxBandwidth; } virtual void setInitialSendSequenceNumber(SequenceNumber seqNum) = 0; void setSendCurrentSequenceNumber(SequenceNumber seqNum) { _sendCurrSeqNum = seqNum; } void setReceiveRate(int rate) { _receiveRate = rate; } @@ -60,7 +61,7 @@ protected: double _congestionWindowSize { 16.0 }; // Congestion window size, in packets int _bandwidth { 0 }; // estimated bandwidth, packets per second - int _maxBandwidth { -1 }; // Maximum desired bandwidth, packets per second + std::atomic _maxBandwidth { -1 }; // Maximum desired bandwidth, bytes per second double _maxCongestionWindowSize { 0.0 }; // maximum cwnd size, in packets int _mss { 0 }; // Maximum Packet Size, including all packet headers diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index f75a9535f5..e5f3508b81 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -80,6 +80,10 @@ void Connection::resetRTT() { _rttVariance = _rtt / 2; } +void Connection::setMaxBandwidth(int maxBandwidth) { + _congestionControl->setMaxBandwidth(maxBandwidth); +} + SendQueue& Connection::getSendQueue() { if (!_sendQueue) { diff --git a/libraries/networking/src/udt/Connection.h b/libraries/networking/src/udt/Connection.h index 8d80e736af..4f5a8793e7 100644 --- a/libraries/networking/src/udt/Connection.h +++ b/libraries/networking/src/udt/Connection.h @@ -76,6 +76,8 @@ public: HifiSockAddr getDestination() const { return _destination; } + void setMaxBandwidth(int maxBandwidth); + signals: void packetSent(); void connectionInactive(const HifiSockAddr& sockAddr); diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 1eb7c04331..e9af1577fb 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -176,7 +176,9 @@ Connection& Socket::findOrCreateConnection(const HifiSockAddr& sockAddr) { auto it = _connectionsHash.find(sockAddr); if (it == _connectionsHash.end()) { - auto connection = std::unique_ptr(new Connection(this, sockAddr, _ccFactory->create())); + auto congestionControl = _ccFactory->create(); + congestionControl->setMaxBandwidth(_maxBandwidth); + auto connection = std::unique_ptr(new Connection(this, sockAddr, std::move(congestionControl))); // we queue the connection to cleanup connection in case it asks for it during its own rate control sync QObject::connect(connection.get(), &Connection::connectionInactive, this, &Socket::cleanupConnection); @@ -350,6 +352,17 @@ void Socket::setCongestionControlFactory(std::unique_ptrsynInterval(); } + +void Socket::setConnectionMaxBandwidth(int maxBandwidth) { + qInfo() << "Setting socket's maximum bandwith to" << maxBandwidth << ". (" + << _connectionsHash.size() << "live connections)"; + _maxBandwidth = maxBandwidth; + for (auto& pair : _connectionsHash) { + auto& connection = pair.second; + connection->setMaxBandwidth(_maxBandwidth); + } +} + ConnectionStats::Stats Socket::sampleStatsForConnection(const HifiSockAddr& destination) { auto it = _connectionsHash.find(destination); if (it != _connectionsHash.end()) { diff --git a/libraries/networking/src/udt/Socket.h b/libraries/networking/src/udt/Socket.h index 88db8e3d86..424158045f 100644 --- a/libraries/networking/src/udt/Socket.h +++ b/libraries/networking/src/udt/Socket.h @@ -72,6 +72,7 @@ public: { _unfilteredHandlers[senderSockAddr] = handler; } void setCongestionControlFactory(std::unique_ptr ccFactory); + void setConnectionMaxBandwidth(int maxBandwidth); void messageReceived(std::unique_ptr packet); void messageFailed(Connection* connection, Packet::MessageNumber messageNumber); @@ -109,8 +110,10 @@ private: std::unordered_map _unreliableSequenceNumbers; std::unordered_map> _connectionsHash; - int _synInterval = 10; // 10ms - QTimer* _synTimer; + int _synInterval { 10 }; // 10ms + QTimer* _synTimer { nullptr }; + + int _maxBandwidth { -1 }; std::unique_ptr _ccFactory { new CongestionControlFactory() }; From 1a2f74f43a5013b023a3f7a47a1a28fa7be19672 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 22 Mar 2016 11:26:10 -0700 Subject: [PATCH 05/15] Add protection around ScriptEngine::runInThread being called twice --- libraries/script-engine/src/ScriptEngine.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index 61ebfe4515..3f403b3677 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -158,6 +158,13 @@ void ScriptEngine::disconnectNonEssentialSignals() { } void ScriptEngine::runInThread() { + Q_ASSERT_X(!_isThreaded, "ScriptEngine::runInThread()", "runInThread should not be called more than once"); + + if (_isThreaded) { + qCWarning(scriptengine) << "ScriptEngine already running in thread: " << getFilename(); + return; + } + _isThreaded = true; QThread* workerThread = new QThread(); // thread is not owned, so we need to manage the delete QString scriptEngineName = QString("Script Thread:") + getFilename(); From 9f0084dbb13343bebef975f3d00f4dc6431c2731 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 22 Mar 2016 16:01:31 -0700 Subject: [PATCH 06/15] make Blender keep a shared pointer to the model it's blending --- libraries/render-utils/src/Model.cpp | 19 +++++++++---------- libraries/render-utils/src/Model.h | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index df186802f6..0a483914df 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -29,7 +29,7 @@ using namespace std; -static int modelPointerTypeId = qRegisterMetaType >(); +static int nakedModelPointerTypeId = qRegisterMetaType(); static int weakNetworkGeometryPointerTypeId = qRegisterMetaType >(); static int vec3VectorTypeId = qRegisterMetaType >(); float Model::FAKE_DIMENSION_PLACEHOLDER = -1.0f; @@ -821,21 +821,21 @@ QStringList Model::getJointNames() const { class Blender : public QRunnable { public: - Blender(Model* model, int blendNumber, const QWeakPointer& geometry, + Blender(ModelPointer model, int blendNumber, const QWeakPointer& geometry, const QVector& meshes, const QVector& blendshapeCoefficients); virtual void run(); private: - QPointer _model; + ModelPointer _model; int _blendNumber; QWeakPointer _geometry; QVector _meshes; QVector _blendshapeCoefficients; }; -Blender::Blender(Model* model, int blendNumber, const QWeakPointer& geometry, +Blender::Blender(ModelPointer model, int blendNumber, const QWeakPointer& geometry, const QVector& meshes, const QVector& blendshapeCoefficients) : _model(model), _blendNumber(blendNumber), @@ -847,7 +847,7 @@ Blender::Blender(Model* model, int blendNumber, const QWeakPointer vertices, normals; - if (!_model.isNull()) { + if (_model) { int offset = 0; foreach (const FBXMesh& mesh, _meshes) { if (mesh.blendshapes.isEmpty()) { @@ -877,7 +877,7 @@ void Blender::run() { } // post the result to the geometry cache, which will dispatch to the model if still alive QMetaObject::invokeMethod(DependencyManager::get().data(), "setBlendedVertices", - Q_ARG(const QPointer&, _model), Q_ARG(int, _blendNumber), + Q_ARG(ModelPointer, _model), Q_ARG(int, _blendNumber), Q_ARG(const QWeakPointer&, _geometry), Q_ARG(const QVector&, vertices), Q_ARG(const QVector&, normals)); } @@ -1088,7 +1088,7 @@ float Model::getLimbLength(int jointIndex) const { bool Model::maybeStartBlender() { const FBXGeometry& fbxGeometry = _geometry->getFBXGeometry(); if (fbxGeometry.hasBlendedMeshes()) { - QThreadPool::globalInstance()->start(new Blender(this, ++_blendNumber, _geometry, + QThreadPool::globalInstance()->start(new Blender(getThisPointer(), ++_blendNumber, _geometry, fbxGeometry.meshes, _blendshapeCoefficients)); return true; } @@ -1284,10 +1284,9 @@ void ModelBlender::noteRequiresBlend(ModelPointer model) { } } -void ModelBlender::setBlendedVertices(const QPointer& model, int blendNumber, +void ModelBlender::setBlendedVertices(ModelPointer model, int blendNumber, const QWeakPointer& geometry, const QVector& vertices, const QVector& normals) { - - if (!model.isNull()) { + if (model) { model->setBlendedVertices(blendNumber, geometry, vertices, normals); } _pendingBlenders--; diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index 53ed82f418..4e51dc4f33 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -384,7 +384,7 @@ protected: RigPointer _rig; }; -Q_DECLARE_METATYPE(QPointer) +Q_DECLARE_METATYPE(ModelPointer) Q_DECLARE_METATYPE(QWeakPointer) /// Handle management of pending models that need blending @@ -398,7 +398,7 @@ public: void noteRequiresBlend(ModelPointer model); public slots: - void setBlendedVertices(const QPointer& model, int blendNumber, const QWeakPointer& geometry, + void setBlendedVertices(ModelPointer model, int blendNumber, const QWeakPointer& geometry, const QVector& vertices, const QVector& normals); private: From 9c11474dd78785eb8811b1084847534c480acbcf Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 15:39:03 -0700 Subject: [PATCH 07/15] Expose qApp->updateHeartbeat --- interface/src/Application.cpp | 24 ++++++++++++------------ interface/src/Application.h | 4 ++++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 7e88ea28dc..65a8f83871 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -239,11 +239,7 @@ class DeadlockWatchdogThread : public QThread { public: static const unsigned long HEARTBEAT_CHECK_INTERVAL_SECS = 1; static const unsigned long HEARTBEAT_UPDATE_INTERVAL_SECS = 1; -#ifdef DEBUG - static const unsigned long MAX_HEARTBEAT_AGE_USECS = 600 * USECS_PER_SECOND; -#else static const unsigned long MAX_HEARTBEAT_AGE_USECS = 10 * USECS_PER_SECOND; -#endif // Set the heartbeat on launch DeadlockWatchdogThread() { @@ -511,8 +507,8 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : auto nodeList = DependencyManager::get(); // Set up a watchdog thread to intentionally crash the application on deadlocks - auto deadlockWatchdog = new DeadlockWatchdogThread(); - deadlockWatchdog->start(); + _deadlockWatchdogThread = new DeadlockWatchdogThread(); + _deadlockWatchdogThread->start(); qCDebug(interfaceapp) << "[VERSION] Build sequence:" << qPrintable(applicationVersion()); @@ -586,7 +582,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : ResourceManager::init(); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); // Setup MessagesClient auto messagesClient = DependencyManager::get(); @@ -734,7 +730,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : initializeGL(); _offscreenContext->makeCurrent(); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); // Tell our entity edit sender about our known jurisdictions _entityEditSender.setServerJurisdictions(&_entityServerJurisdictions); @@ -746,7 +742,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : _overlays.init(); // do this before scripts load // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); connect(this, SIGNAL(aboutToQuit()), this, SLOT(aboutToQuit())); @@ -900,11 +896,11 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : // do this as late as possible so that all required subsystems are initialized scriptEngines->loadScripts(); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); loadSettings(); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); int SAVE_SETTINGS_INTERVAL = 10 * MSECS_PER_SECOND; // Let's save every seconds for now connect(&_settingsTimer, &QTimer::timeout, this, &Application::saveSettings); @@ -1019,7 +1015,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : }); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); connect(this, &Application::applicationStateChanged, this, &Application::activeChanged); qCDebug(interfaceapp, "Startup time: %4.2f seconds.", (double)startupTimer.elapsed() / 1000.0); @@ -1060,6 +1056,10 @@ void Application::showCursor(const QCursor& cursor) { _cursorNeedsChanging = true; } +void Application::updateHeartbeat() { + static_cast(_deadlockWatchdogThread)->updateHeartbeat(); +} + void Application::aboutToQuit() { emit beforeAboutToQuit(); diff --git a/interface/src/Application.h b/interface/src/Application.h index 4c12164a5f..3b906a7d17 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -271,6 +271,8 @@ public slots: void reloadResourceCaches(); + void updateHeartbeat(); + void crashApplication(); void deadlockApplication(); @@ -505,6 +507,8 @@ private: mutable QMutex _changeCursorLock { QMutex::Recursive }; QCursor _desiredCursor{ Qt::BlankCursor }; bool _cursorNeedsChanging { false }; + + QThread* _deadlockWatchdogThread; }; #endif // hifi_Application_h From 745438148a760b10260d832dcfbeb99f5851c112 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 17:17:36 -0700 Subject: [PATCH 08/15] Reset deadlock watch before qml load --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index 563c590874..4cfd2527ea 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -426,7 +426,11 @@ void OffscreenQmlSurface::setBaseUrl(const QUrl& baseUrl) { } QObject* OffscreenQmlSurface::load(const QUrl& qmlSource, std::function f) { - _qmlComponent->loadUrl(qmlSource); + // Synchronous loading may take a while; restart the deadlock timer + QMetaObject::invokeMethod(qApp, "updateHeartbeat", Qt::DirectConnection); + + _qmlComponent->loadUrl(qmlSource, QQmlComponent::PreferSynchronous); + if (_qmlComponent->isLoading()) { connect(_qmlComponent, &QQmlComponent::statusChanged, this, [this, f](QQmlComponent::Status){ From b41c106f3c84ae84c51e3f83d70548f4f3f08c5e Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 18:40:26 -0700 Subject: [PATCH 09/15] Only set the context once per QML surface --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 22 +++------------------ 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index 563c590874..6056e8234f 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -214,22 +214,19 @@ void OffscreenQmlRenderThread::init() { connect(_renderControl, &QQuickRenderControl::sceneChanged, _surface, &OffscreenQmlSurface::requestUpdate); if (!_canvas.makeCurrent()) { - qWarning("Failed to make context current on render thread"); + // Failed to make GL context current, this OffscreenQmlSurface is basically dead + qWarning("Failed to make context current on QML Renderer Thread"); return; } + _renderControl->initialize(_canvas.getContext()); setupFbo(); _escrow.setRecycler([this](GLuint texture){ _textures.recycleTexture(texture); }); - _canvas.doneCurrent(); } void OffscreenQmlRenderThread::cleanup() { - if (!_canvas.makeCurrent()) { - qFatal("Failed to make context current on render thread"); - return; - } _renderControl->invalidate(); _fbo.reset(); @@ -237,7 +234,6 @@ void OffscreenQmlRenderThread::cleanup() { _textures.clear(); _canvas.doneCurrent(); - _canvas.getContextObject()->moveToThread(QCoreApplication::instance()->thread()); _quit = true; @@ -267,18 +263,11 @@ void OffscreenQmlRenderThread::resize() { } _size = newOffscreenSize; - // Clear out any fbos with the old size - if (!_canvas.makeCurrent()) { - qWarning("Failed to make context current on render thread"); - return; - } - qDebug() << "Offscreen UI resizing to " << _newSize.width() << "x" << _newSize.height() << " with pixel ratio " << pixelRatio; locker.unlock(); setupFbo(); - _canvas.doneCurrent(); } void OffscreenQmlRenderThread::render() { @@ -287,11 +276,6 @@ void OffscreenQmlRenderThread::render() { return; } - if (!_canvas.makeCurrent()) { - qWarning("Failed to make context current on render thread"); - return; - } - QMutexLocker locker(&_mutex); _renderControl->sync(); _waitCondition.wakeOne(); From fe1d8b24b40a91ba6bbcb9c2e97539b3c4ea1158 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 18:42:46 -0700 Subject: [PATCH 10/15] Lock the QML Surface resizing better --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 46 ++++++++++----------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index 6056e8234f..8e02ef2e4f 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -241,32 +241,32 @@ void OffscreenQmlRenderThread::cleanup() { void OffscreenQmlRenderThread::resize() { // Lock _newSize changes - QMutexLocker locker(&_mutex); + { + QMutexLocker locker(&_mutex); - // Update our members - if (_quickWindow) { - _quickWindow->setGeometry(QRect(QPoint(), _newSize)); - _quickWindow->contentItem()->setSize(_newSize); + // Update our members + if (_quickWindow) { + _quickWindow->setGeometry(QRect(QPoint(), _newSize)); + _quickWindow->contentItem()->setSize(_newSize); + } + + // Qt bug in 5.4 forces this check of pixel ratio, + // even though we're rendering offscreen. + qreal pixelRatio = 1.0; + if (_renderControl && _renderControl->_renderWindow) { + pixelRatio = _renderControl->_renderWindow->devicePixelRatio(); + } + + uvec2 newOffscreenSize = toGlm(_newSize * pixelRatio); + if (newOffscreenSize == _size) { + return; + } + + qDebug() << "Offscreen UI resizing to " << _newSize.width() << "x" << _newSize.height() << " with pixel ratio " << pixelRatio; + _size = newOffscreenSize; } - // Qt bug in 5.4 forces this check of pixel ratio, - // even though we're rendering offscreen. - qreal pixelRatio = 1.0; - if (_renderControl && _renderControl->_renderWindow) { - pixelRatio = _renderControl->_renderWindow->devicePixelRatio(); - } - - uvec2 newOffscreenSize = toGlm(_newSize * pixelRatio); - _textures.setSize(newOffscreenSize); - if (newOffscreenSize == _size) { - return; - } - _size = newOffscreenSize; - - qDebug() << "Offscreen UI resizing to " << _newSize.width() << "x" << _newSize.height() << " with pixel ratio " << pixelRatio; - - locker.unlock(); - + _textures.setSize(_size); setupFbo(); } From 40b21933a574474757008492bef4e66c16334051 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 18:42:54 -0700 Subject: [PATCH 11/15] Clean QML Surface render log --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index 8e02ef2e4f..49b2a6fd34 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -322,10 +322,10 @@ OffscreenQmlSurface::~OffscreenQmlSurface() { QObject::disconnect(&_updateTimer); QObject::disconnect(qApp); - qDebug() << "Stopping QML render thread " << _renderer->currentThreadId(); + qDebug() << "Stopping QML Renderer Thread " << _renderer->currentThreadId(); _renderer->_queue.add(STOP); if (!_renderer->wait(MAX_SHUTDOWN_WAIT_SECS * USECS_PER_SECOND)) { - qWarning() << "Failed to shut down the QML render thread"; + qWarning() << "Failed to shut down the QML Renderer Thread"; } delete _rootItem; From 2fdb92e8cded5675ad64c64db4f218b9728f03ed Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 22 Mar 2016 22:19:10 -0700 Subject: [PATCH 12/15] Move oglplus dependency from sourceforge to s3 --- cmake/externals/oglplus/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/externals/oglplus/CMakeLists.txt b/cmake/externals/oglplus/CMakeLists.txt index 43730129a0..089ee5bb95 100644 --- a/cmake/externals/oglplus/CMakeLists.txt +++ b/cmake/externals/oglplus/CMakeLists.txt @@ -4,7 +4,7 @@ string(TOUPPER ${EXTERNAL_NAME} EXTERNAL_NAME_UPPER) include(ExternalProject) ExternalProject_Add( ${EXTERNAL_NAME} - URL http://iweb.dl.sourceforge.net/project/oglplus/oglplus-0.63.x/oglplus-0.63.0.zip + URL http://hifi-public.s3.amazonaws.com/dependencies/oglplus-0.63.0.zip URL_MD5 de984ab245b185b45c87415c0e052135 CONFIGURE_COMMAND "" BUILD_COMMAND "" From 252a49eea4ea92c88d1baf4d021a840bd246e0e7 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 22 Mar 2016 20:21:36 -0700 Subject: [PATCH 13/15] Add a tracker and logging of memory allocated by the GPU library --- libraries/gpu/src/gpu/Resource.cpp | 64 ++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/libraries/gpu/src/gpu/Resource.cpp b/libraries/gpu/src/gpu/Resource.cpp index 1788f6ece5..197263f392 100644 --- a/libraries/gpu/src/gpu/Resource.cpp +++ b/libraries/gpu/src/gpu/Resource.cpp @@ -10,11 +10,74 @@ // #include "Resource.h" +#include + +#include +#include #include using namespace gpu; +class AllocationDebugger { +public: + void operator+=(size_t size) { + _allocatedMemory += size; + maybeReport(); + } + + void operator-=(size_t size) { + _allocatedMemory -= size; + maybeReport(); + } + +private: + QString formatSize(size_t size) { + float num = size; + QStringList list; + list << "KB" << "MB" << "GB" << "TB"; + + QStringListIterator i(list); + QString unit("bytes"); + + while (num >= K && i.hasNext()) { + unit = i.next(); + num /= K; + } + return QString().setNum(num, 'f', 2) + " " + unit; + } + + void maybeReport() { + auto now = usecTimestampNow(); + if (now - _lastReportTime < MAX_REPORT_FREQUENCY) { + return; + } + size_t current = _allocatedMemory; + size_t last = _lastReportedMemory; + size_t delta = (current > last) ? (current - last) : (last - current); + if (delta > MIN_REPORT_DELTA) { + _lastReportTime = now; + _lastReportedMemory = current; + qDebug() << "Total allocation " << formatSize(current); + } + } + + std::atomic _allocatedMemory; + std::atomic _lastReportedMemory; + std::atomic _lastReportTime; + + static const float K; + // Report changes of 5 megabytes + static const size_t MIN_REPORT_DELTA = 1024 * 1024 * 5; + // Report changes no more frequently than every 15 seconds + static const uint64_t MAX_REPORT_FREQUENCY = USECS_PER_SECOND * 15; +}; + +const float AllocationDebugger::K = 1024.0f; + +static AllocationDebugger allocationDebugger; + Resource::Size Resource::Sysmem::allocateMemory(Byte** dataAllocated, Size size) { + allocationDebugger += size; if ( !dataAllocated ) { qWarning() << "Buffer::Sysmem::allocateMemory() : Must have a valid dataAllocated pointer."; return NOT_ALLOCATED; @@ -38,6 +101,7 @@ Resource::Size Resource::Sysmem::allocateMemory(Byte** dataAllocated, Size size) } void Resource::Sysmem::deallocateMemory(Byte* dataAllocated, Size size) { + allocationDebugger -= size; if (dataAllocated) { delete[] dataAllocated; } From 84dc15aff62408bff63543f132282dbcc6426e51 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 23 Mar 2016 12:06:52 -0700 Subject: [PATCH 14/15] Fix potential deadlock in QML --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 34 +++++++++++++++------ libraries/shared/src/Finally.h | 4 +++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index d3699b54e6..e224adad07 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include "OffscreenGLCanvas.h" #include "GLEscrow.h" @@ -84,6 +85,7 @@ protected: Queue _queue; QMutex _mutex; QWaitCondition _waitCondition; + std::atomic _rendering { false }; private: // Event-driven methods @@ -271,15 +273,25 @@ void OffscreenQmlRenderThread::resize() { } void OffscreenQmlRenderThread::render() { - if (_surface->_paused) { + // Ensure we always release the main thread + Finally releaseMainThread([this] { _waitCondition.wakeOne(); + }); + + if (_surface->_paused) { return; } - QMutexLocker locker(&_mutex); - _renderControl->sync(); - _waitCondition.wakeOne(); - locker.unlock(); + _rendering = true; + Finally unmarkRenderingFlag([this] { + _rendering = false; + }); + + { + QMutexLocker locker(&_mutex); + _renderControl->sync(); + releaseMainThread.trigger(); + } using namespace oglplus; @@ -292,6 +304,7 @@ void OffscreenQmlRenderThread::render() { _fbo->AttachTexture(Framebuffer::Target::Draw, FramebufferAttachment::Color, *texture, 0); _fbo->Complete(Framebuffer::Target::Draw); { + PROFILE_RANGE("qml_render->rendercontrol") _renderControl->render(); // FIXME The web browsers seem to be leaving GL in an error state. // Need a debug context with sync logging to figure out why. @@ -380,8 +393,6 @@ void OffscreenQmlSurface::resize(const QSize& newSize_) { std::max(static_cast(scale * newSize.height()), 10)); } - - QSize currentSize = _renderer->_quickWindow->geometry().size(); if (newSize == currentSize) { return; @@ -492,7 +503,12 @@ QObject* OffscreenQmlSurface::finishQmlLoad(std::functionallowNewFrame(_maxFps)) { + // If we're + // a) not set up + // b) already rendering a frame + // c) rendering too fast + // then skip this + if (!_renderer || _renderer->_rendering || !_renderer->allowNewFrame(_maxFps)) { return; } @@ -502,11 +518,11 @@ void OffscreenQmlSurface::updateQuick() { } if (_render) { + PROFILE_RANGE(__FUNCTION__); // Lock the GUI size while syncing QMutexLocker locker(&(_renderer->_mutex)); _renderer->_queue.add(RENDER); _renderer->_waitCondition.wait(&(_renderer->_mutex)); - _render = false; } diff --git a/libraries/shared/src/Finally.h b/libraries/shared/src/Finally.h index 59e8be0228..9070d49647 100644 --- a/libraries/shared/src/Finally.h +++ b/libraries/shared/src/Finally.h @@ -20,6 +20,10 @@ public: template Finally(F f) : _f(f) {} ~Finally() { _f(); } + void trigger() { + _f(); + _f = [] {}; + } private: std::function _f; }; From fdd7bb9f58a6cb5165e57799d084428f4cabda2a Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Wed, 23 Mar 2016 13:18:46 -0700 Subject: [PATCH 15/15] Update readme --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a2eb058ae6..48e0de03af 100644 --- a/README.md +++ b/README.md @@ -8,14 +8,14 @@ like to get paid for your work, make sure you report the bug via a job on [Worklist.net](https://worklist.net). We're hiring! We're looking for skilled developers; -send your resume to hiring@highfidelity.io +send your resume to hiring@highfidelity.com ##### Chat with us Come chat with us in [our Gitter](http://gitter.im/highfidelity/hifi) if you have any questions or just want to say hi! Documentation ========= -Documentation is available at [docs.highfidelity.io](http://docs.highfidelity.io), if something is missing, please suggest it via a new job on Worklist (add to the hifi-docs project). +Documentation is available at [docs.highfidelity.com](http://docs.highfidelity.com), if something is missing, please suggest it via a new job on Worklist (add to the hifi-docs project). Build Instructions =========