From 08d1364f81616093bec3700dd26b98aec008fadf Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 8 Jul 2015 22:35:23 -0700 Subject: [PATCH 1/3] Move file log persistence to a separate thread --- interface/src/AbstractLoggerInterface.h | 4 +- interface/src/FileLogger.cpp | 42 +++++++++---- interface/src/FileLogger.h | 10 ++-- libraries/shared/src/GenericQueueThread.h | 72 +++++++++++++++++++++++ libraries/shared/src/GenericThread.cpp | 3 +- libraries/shared/src/GenericThread.h | 2 +- 6 files changed, 115 insertions(+), 18 deletions(-) create mode 100644 libraries/shared/src/GenericQueueThread.h diff --git a/interface/src/AbstractLoggerInterface.h b/interface/src/AbstractLoggerInterface.h index fe45346e4c..3823060b13 100644 --- a/interface/src/AbstractLoggerInterface.h +++ b/interface/src/AbstractLoggerInterface.h @@ -24,7 +24,7 @@ public: inline bool extraDebugging() { return _extraDebugging; } inline void setExtraDebugging(bool debugging) { _extraDebugging = debugging; } - virtual void addMessage(QString) = 0; + virtual void addMessage(const QString&) = 0; virtual QString getLogData() = 0; virtual void locateLog() = 0; @@ -32,7 +32,7 @@ signals: void logReceived(QString message); private: - bool _extraDebugging; + bool _extraDebugging{ false }; }; #endif // hifi_AbstractLoggerInterface_h diff --git a/interface/src/FileLogger.cpp b/interface/src/FileLogger.cpp index 4808842036..bcae96b2ea 100644 --- a/interface/src/FileLogger.cpp +++ b/interface/src/FileLogger.cpp @@ -21,11 +21,35 @@ const QString FILENAME_FORMAT = "hifi-log_%1_%2.txt"; const QString DATETIME_FORMAT = "yyyy-MM-dd_hh.mm.ss"; const QString LOGS_DIRECTORY = "Logs"; +class FilePersistThread : public GenericQueueThread < QString > { +public: + FilePersistThread(FileLogger& logger) : _logger(logger) { + setObjectName("LogFileWriter"); + } + +protected: + virtual bool processQueueItems(const Queue& messages) { + QFile file(_logger._fileName); + if (file.open(QIODevice::WriteOnly | QIODevice::Append | QIODevice::Text)) { + QTextStream out(&file); + foreach(const QString& message, messages) { + out << message; + } + } + return true; + } +private: + FileLogger& _logger; +}; + +static FilePersistThread* _persistThreadInstance; + FileLogger::FileLogger(QObject* parent) : AbstractLoggerInterface(parent), _logData("") { - setExtraDebugging(false); + _persistThreadInstance = new FilePersistThread(*this); + _persistThreadInstance->initialize(true); _fileName = FileUtils::standardPath(LOGS_DIRECTORY); QHostAddress clientAddress = getLocalAddress(); @@ -33,16 +57,14 @@ FileLogger::FileLogger(QObject* parent) : _fileName.append(QString(FILENAME_FORMAT).arg(clientAddress.toString(), now.toString(DATETIME_FORMAT))); } -void FileLogger::addMessage(QString message) { - QMutexLocker locker(&_mutex); - emit logReceived(message); - _logData += message; +FileLogger::~FileLogger() { + _persistThreadInstance->terminate(); +} - QFile file(_fileName); - if (file.open(QIODevice::WriteOnly | QIODevice::Append | QIODevice::Text)) { - QTextStream out(&file); - out << message; - } +void FileLogger::addMessage(const QString& message) { + _persistThreadInstance->queueItem(message); + emit logReceived(message); + //_logData += message; } void FileLogger::locateLog() { diff --git a/interface/src/FileLogger.h b/interface/src/FileLogger.h index 3dbbfd26cd..72bffa6445 100644 --- a/interface/src/FileLogger.h +++ b/interface/src/FileLogger.h @@ -13,23 +13,25 @@ #define hifi_FileLogger_h #include "AbstractLoggerInterface.h" -#include +#include class FileLogger : public AbstractLoggerInterface { Q_OBJECT public: FileLogger(QObject* parent = NULL); + virtual ~FileLogger(); - virtual void addMessage(QString); + virtual void addMessage(const QString&); virtual QString getLogData() { return _logData; } virtual void locateLog(); private: QString _logData; QString _fileName; - QMutex _mutex; - + friend class FilePersistThread; }; + + #endif // hifi_FileLogger_h diff --git a/libraries/shared/src/GenericQueueThread.h b/libraries/shared/src/GenericQueueThread.h new file mode 100644 index 0000000000..2a48ff7418 --- /dev/null +++ b/libraries/shared/src/GenericQueueThread.h @@ -0,0 +1,72 @@ +// +// Created by Bradley Austin Davis 2015/07/08. +// Copyright 2015 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#pragma once +#ifndef hifi_GenericQueueThread_h +#define hifi_GenericQueueThread_h + +#include + +#include +#include +#include + +#include "GenericThread.h" +#include "NumericalConstants.h" + +template +class GenericQueueThread : public GenericThread { +public: + using Queue = QQueue; + GenericQueueThread(QObject* parent = nullptr) + : GenericThread(parent) {} + + virtual ~GenericQueueThread() {} + + void queueItem(const T& t) { + lock(); + queueItemInternal(t); + unlock(); + _hasItems.wakeAll(); + } + +protected: + virtual void queueItemInternal(const T& t) { + _items.push_back(t); + } + + virtual uint32_t getMaxWait() { + return MSECS_PER_SECOND; + } + + virtual bool process() { + if (!_items.size()) { + _hasItemsMutex.lock(); + _hasItems.wait(&_hasItemsMutex, getMaxWait()); + _hasItemsMutex.unlock(); + } + + if (!_items.size()) { + return isStillRunning(); + } + + Queue processItems; + lock(); + processItems.swap(_items); + unlock(); + return processQueueItems(processItems); + } + + virtual bool processQueueItems(const Queue& items) = 0; + + Queue _items; + QWaitCondition _hasItems; + QMutex _hasItemsMutex; +}; + +#endif // hifi_GenericQueueThread_h diff --git a/libraries/shared/src/GenericThread.cpp b/libraries/shared/src/GenericThread.cpp index 3068ca2ab6..a7e0f3456b 100644 --- a/libraries/shared/src/GenericThread.cpp +++ b/libraries/shared/src/GenericThread.cpp @@ -14,7 +14,8 @@ #include "GenericThread.h" -GenericThread::GenericThread() : +GenericThread::GenericThread(QObject* parent) : + QObject(parent), _stopThread(false), _isThreaded(false) // assume non-threaded, must call initialize() { diff --git a/libraries/shared/src/GenericThread.h b/libraries/shared/src/GenericThread.h index b2c0eb13db..6fe6b6ccb4 100644 --- a/libraries/shared/src/GenericThread.h +++ b/libraries/shared/src/GenericThread.h @@ -23,7 +23,7 @@ class GenericThread : public QObject { Q_OBJECT public: - GenericThread(); + GenericThread(QObject* parent = nullptr); virtual ~GenericThread(); /// Call to start the thread. From 63dfd570f198e243709bad1a54f0af1821fe3a49 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 8 Jul 2015 22:54:31 -0700 Subject: [PATCH 2/3] Adding priority support to GenericThread --- interface/src/FileLogger.cpp | 6 +++--- libraries/shared/src/GenericThread.cpp | 3 ++- libraries/shared/src/GenericThread.h | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/interface/src/FileLogger.cpp b/interface/src/FileLogger.cpp index bcae96b2ea..508586c23c 100644 --- a/interface/src/FileLogger.cpp +++ b/interface/src/FileLogger.cpp @@ -23,7 +23,7 @@ const QString LOGS_DIRECTORY = "Logs"; class FilePersistThread : public GenericQueueThread < QString > { public: - FilePersistThread(FileLogger& logger) : _logger(logger) { + FilePersistThread(const FileLogger& logger) : _logger(logger) { setObjectName("LogFileWriter"); } @@ -39,7 +39,7 @@ protected: return true; } private: - FileLogger& _logger; + const FileLogger& _logger; }; static FilePersistThread* _persistThreadInstance; @@ -49,7 +49,7 @@ FileLogger::FileLogger(QObject* parent) : _logData("") { _persistThreadInstance = new FilePersistThread(*this); - _persistThreadInstance->initialize(true); + _persistThreadInstance->initialize(true, QThread::LowestPriority); _fileName = FileUtils::standardPath(LOGS_DIRECTORY); QHostAddress clientAddress = getLocalAddress(); diff --git a/libraries/shared/src/GenericThread.cpp b/libraries/shared/src/GenericThread.cpp index a7e0f3456b..9c1c7c590c 100644 --- a/libraries/shared/src/GenericThread.cpp +++ b/libraries/shared/src/GenericThread.cpp @@ -28,13 +28,14 @@ GenericThread::~GenericThread() { } } -void GenericThread::initialize(bool isThreaded) { +void GenericThread::initialize(bool isThreaded, QThread::Priority priority) { _isThreaded = isThreaded; if (_isThreaded) { _thread = new QThread(this); // match the thread name to our object name _thread->setObjectName(objectName()); + _thread->setPriority(priority); // when the worker thread is started, call our engine's run.. connect(_thread, SIGNAL(started()), this, SLOT(threadRoutine())); diff --git a/libraries/shared/src/GenericThread.h b/libraries/shared/src/GenericThread.h index 6fe6b6ccb4..f261dc5b37 100644 --- a/libraries/shared/src/GenericThread.h +++ b/libraries/shared/src/GenericThread.h @@ -28,7 +28,7 @@ public: /// Call to start the thread. /// \param bool isThreaded true by default. false for non-threaded mode and caller must call threadRoutine() regularly. - void initialize(bool isThreaded = true); + void initialize(bool isThreaded = true, QThread::Priority priority = QThread::NormalPriority); /// Call to stop the thread void terminate(); From 2fff560ff9dadfcbb2d8b60eebd881676812cbae Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 9 Jul 2015 09:54:35 -0700 Subject: [PATCH 3/3] Fixing log file viewer --- interface/src/FileLogger.cpp | 13 ++++++++++--- interface/src/FileLogger.h | 7 +++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/interface/src/FileLogger.cpp b/interface/src/FileLogger.cpp index 508586c23c..00da80814b 100644 --- a/interface/src/FileLogger.cpp +++ b/interface/src/FileLogger.cpp @@ -45,8 +45,7 @@ private: static FilePersistThread* _persistThreadInstance; FileLogger::FileLogger(QObject* parent) : - AbstractLoggerInterface(parent), - _logData("") + AbstractLoggerInterface(parent) { _persistThreadInstance = new FilePersistThread(*this); _persistThreadInstance->initialize(true, QThread::LowestPriority); @@ -64,9 +63,17 @@ FileLogger::~FileLogger() { void FileLogger::addMessage(const QString& message) { _persistThreadInstance->queueItem(message); emit logReceived(message); - //_logData += message; } void FileLogger::locateLog() { FileUtils::locateFile(_fileName); } + +QString FileLogger::getLogData() { + QString result; + QFile f(_fileName); + if (f.open(QFile::ReadOnly | QFile::Text)) { + result = QTextStream(&f).readAll(); + } + return result; +} diff --git a/interface/src/FileLogger.h b/interface/src/FileLogger.h index 72bffa6445..549654ca5c 100644 --- a/interface/src/FileLogger.h +++ b/interface/src/FileLogger.h @@ -22,12 +22,11 @@ public: FileLogger(QObject* parent = NULL); virtual ~FileLogger(); - virtual void addMessage(const QString&); - virtual QString getLogData() { return _logData; } - virtual void locateLog(); + virtual void addMessage(const QString&) override; + virtual QString getLogData() override; + virtual void locateLog() override; private: - QString _logData; QString _fileName; friend class FilePersistThread; };