From 04fd6e9d0491ae266ad2fb7c8f366afaa12be7fa Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Thu, 28 Feb 2019 14:17:43 +0800 Subject: [PATCH 1/5] Avoid performance penalty when logger is full --- src/base/logger.cpp | 37 +++++++++++++++++++++---------------- src/base/logger.h | 12 +++++++----- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/base/logger.cpp b/src/base/logger.cpp index 46d980686..c27ef1784 100644 --- a/src/base/logger.cpp +++ b/src/base/logger.cpp @@ -28,20 +28,31 @@ #include "logger.h" +#include + #include -#include "base/utils/string.h" + +namespace +{ + template + QVector loadFromBuffer(const boost::circular_buffer_space_optimized &src, const int offset = 0) + { + QVector ret; + ret.reserve(src.size() - offset); + std::copy((src.begin() + offset), src.end(), std::back_inserter(ret)); + return ret; + } +} Logger *Logger::m_instance = nullptr; Logger::Logger() - : m_lock(QReadWriteLock::Recursive) - , m_msgCounter(0) - , m_peerCounter(0) + : m_messages(MAX_LOG_MESSAGES) + , m_peers(MAX_LOG_MESSAGES) + , m_lock(QReadWriteLock::Recursive) { } -Logger::~Logger() {} - Logger *Logger::instance() { return m_instance; @@ -68,9 +79,6 @@ void Logger::addMessage(const QString &message, const Log::MsgType &type) const Log::Msg temp = {m_msgCounter++, QDateTime::currentMSecsSinceEpoch(), type, message.toHtmlEscaped()}; m_messages.push_back(temp); - if (m_messages.size() >= MAX_LOG_MESSAGES) - m_messages.pop_front(); - emit newLogMessage(temp); } @@ -81,9 +89,6 @@ void Logger::addPeer(const QString &ip, const bool blocked, const QString &reaso const Log::Peer temp = {m_peerCounter++, QDateTime::currentMSecsSinceEpoch(), ip.toHtmlEscaped(), blocked, reason.toHtmlEscaped()}; m_peers.push_back(temp); - if (m_peers.size() >= MAX_LOG_MESSAGES) - m_peers.pop_front(); - emit newLogPeer(temp); } @@ -95,12 +100,12 @@ QVector Logger::getMessages(const int lastKnownId) const const int size = m_messages.size(); if ((lastKnownId == -1) || (diff >= size)) - return m_messages; + return loadFromBuffer(m_messages); if (diff <= 0) return {}; - return m_messages.mid(size - diff); + return loadFromBuffer(m_messages, (size - diff)); } QVector Logger::getPeers(const int lastKnownId) const @@ -111,12 +116,12 @@ QVector Logger::getPeers(const int lastKnownId) const const int size = m_peers.size(); if ((lastKnownId == -1) || (diff >= size)) - return m_peers; + return loadFromBuffer(m_peers); if (diff <= 0) return {}; - return m_peers.mid(size - diff); + return loadFromBuffer(m_peers, (size - diff)); } void LogMsg(const QString &message, const Log::MsgType &type) diff --git a/src/base/logger.h b/src/base/logger.h index f6be24b4f..0bd6aa441 100644 --- a/src/base/logger.h +++ b/src/base/logger.h @@ -29,6 +29,8 @@ #ifndef LOGGER_H #define LOGGER_H +#include + #include #include #include @@ -89,14 +91,14 @@ signals: private: Logger(); - ~Logger(); + ~Logger() = default; static Logger *m_instance; - QVector m_messages; - QVector m_peers; + boost::circular_buffer_space_optimized m_messages; + boost::circular_buffer_space_optimized m_peers; mutable QReadWriteLock m_lock; - int m_msgCounter; - int m_peerCounter; + int m_msgCounter = 0; + int m_peerCounter = 0; }; // Helper function From a3019f56b073e4260ef272f5a950f70b6c4bcfa6 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Fri, 8 Mar 2019 17:14:20 +0800 Subject: [PATCH 2/5] Clean up code --- src/app/filelogger.cpp | 15 ++++++++------- src/gui/executionlogwidget.cpp | 33 +++++++++++++++------------------ src/gui/executionlogwidget.ui | 3 --- src/gui/loglistwidget.cpp | 17 ++++++++++------- src/gui/loglistwidget.h | 2 +- 5 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/app/filelogger.cpp b/src/app/filelogger.cpp index 4a44f57f6..45d160b9f 100644 --- a/src/app/filelogger.cpp +++ b/src/app/filelogger.cpp @@ -66,10 +66,9 @@ FileLogger::~FileLogger() void FileLogger::changePath(const QString &newPath) { - QString tmpPath = Utils::Fs::fromNativePath(newPath); - QDir dir(tmpPath); - dir.mkpath(tmpPath); - tmpPath = dir.absoluteFilePath("qbittorrent.log"); + const QDir dir(newPath); + dir.mkpath(newPath); + const QString tmpPath = dir.absoluteFilePath("qbittorrent.log"); if (tmpPath != m_path) { m_path = tmpPath; @@ -87,8 +86,10 @@ void FileLogger::deleteOld(const int age, const FileLogAgeType ageType) { const QDateTime date = QDateTime::currentDateTime(); const QDir dir(Utils::Fs::branchPath(m_path)); + const QFileInfoList fileList = dir.entryInfoList(QStringList("qbittorrent.log.bak*") + , (QDir::Files | QDir::Writable), (QDir::Time | QDir::Reversed)); - for (const QFileInfo &file : asConst(dir.entryInfoList(QStringList("qbittorrent.log.bak*"), QDir::Files | QDir::Writable, QDir::Time | QDir::Reversed))) { + for (const QFileInfo &file : fileList) { QDateTime modificationDate = file.lastModified(); switch (ageType) { case DAYS: @@ -106,7 +107,7 @@ void FileLogger::deleteOld(const int age, const FileLogAgeType ageType) } } -void FileLogger::setBackup(bool value) +void FileLogger::setBackup(const bool value) { m_backup = value; } @@ -168,7 +169,7 @@ void FileLogger::openLogFile() || !m_logFile->setPermissions(QFile::ReadOwner | QFile::WriteOwner)) { delete m_logFile; m_logFile = nullptr; - Logger::instance()->addMessage(tr("An error occurred while trying to open the log file. Logging to file is disabled."), Log::CRITICAL); + LogMsg(tr("An error occurred while trying to open the log file. Logging to file is disabled."), Log::CRITICAL); } } diff --git a/src/gui/executionlogwidget.cpp b/src/gui/executionlogwidget.cpp index 8e0980c67..45c845976 100644 --- a/src/gui/executionlogwidget.cpp +++ b/src/gui/executionlogwidget.cpp @@ -40,7 +40,7 @@ ExecutionLogWidget::ExecutionLogWidget(QWidget *parent, const Log::MsgTypes &types) : QWidget(parent) , m_ui(new Ui::ExecutionLogWidget) - , m_msgList(new LogListWidget(MAX_LOG_MESSAGES, Log::MsgTypes(types))) + , m_msgList(new LogListWidget(MAX_LOG_MESSAGES, types)) , m_peerList(new LogListWidget(MAX_LOG_MESSAGES)) { m_ui->setupUi(this); @@ -75,38 +75,35 @@ void ExecutionLogWidget::showMsgTypes(const Log::MsgTypes &types) void ExecutionLogWidget::addLogMessage(const Log::Msg &msg) { - QString text; - QDateTime time = QDateTime::fromMSecsSinceEpoch(msg.timestamp); - QColor color; - + QString colorName; switch (msg.type) { case Log::INFO: - color.setNamedColor("blue"); + colorName = QLatin1String("blue"); break; case Log::WARNING: - color.setNamedColor("orange"); + colorName = QLatin1String("orange"); break; case Log::CRITICAL: - color.setNamedColor("red"); + colorName = QLatin1String("red"); break; default: - color = QApplication::palette().color(QPalette::WindowText); + colorName = QApplication::palette().color(QPalette::WindowText).name(); } - text = "" + time.toString(Qt::SystemLocaleShortDate) + " - " + msg.message + ""; + const QDateTime time = QDateTime::fromMSecsSinceEpoch(msg.timestamp); + const QString text = QString(QLatin1String("%1 - %3")) + .arg(time.toString(Qt::SystemLocaleShortDate), colorName, msg.message); m_msgList->appendLine(text, msg.type); } void ExecutionLogWidget::addPeerMessage(const Log::Peer &peer) { - QString text; - QDateTime time = QDateTime::fromMSecsSinceEpoch(peer.timestamp); - - if (peer.blocked) - text = "" + time.toString(Qt::SystemLocaleShortDate) + " - " - + tr("%1 was blocked %2", "x.y.z.w was blocked").arg(peer.ip, peer.reason); - else - text = "" + time.toString(Qt::SystemLocaleShortDate) + " - " + tr("%1 was banned", "x.y.z.w was banned").arg(peer.ip); + const QDateTime time = QDateTime::fromMSecsSinceEpoch(peer.timestamp); + const QString msg = QString(QLatin1String("%1 - %2")) + .arg(time.toString(Qt::SystemLocaleShortDate), peer.ip); + const QString text = peer.blocked + ? tr("%1 was blocked %2", "0.0.0.0 was blocked due to reason").arg(msg, peer.reason) + : tr("%1 was banned", "0.0.0.0 was banned").arg(msg); m_peerList->appendLine(text, Log::NORMAL); } diff --git a/src/gui/executionlogwidget.ui b/src/gui/executionlogwidget.ui index 1814e19ac..9de7af044 100644 --- a/src/gui/executionlogwidget.ui +++ b/src/gui/executionlogwidget.ui @@ -10,9 +10,6 @@ 300 - - Form - 0 diff --git a/src/gui/loglistwidget.cpp b/src/gui/loglistwidget.cpp index 6f02e42e8..cdb8172a2 100644 --- a/src/gui/loglistwidget.cpp +++ b/src/gui/loglistwidget.cpp @@ -39,7 +39,7 @@ #include "base/global.h" #include "guiiconprovider.h" -LogListWidget::LogListWidget(int maxLines, const Log::MsgTypes &types, QWidget *parent) +LogListWidget::LogListWidget(const int maxLines, const Log::MsgTypes &types, QWidget *parent) : QListWidget(parent) , m_maxLines(maxLines) , m_types(types) @@ -47,8 +47,8 @@ LogListWidget::LogListWidget(int maxLines, const Log::MsgTypes &types, QWidget * // Allow multiple selections setSelectionMode(QAbstractItemView::ExtendedSelection); // Context menu - QAction *copyAct = new QAction(GuiIconProvider::instance()->getIcon("edit-copy"), tr("Copy"), this); - QAction *clearAct = new QAction(GuiIconProvider::instance()->getIcon("edit-clear"), tr("Clear"), this); + auto *copyAct = new QAction(GuiIconProvider::instance()->getIcon("edit-copy"), tr("Copy"), this); + auto *clearAct = new QAction(GuiIconProvider::instance()->getIcon("edit-clear"), tr("Clear"), this); connect(copyAct, &QAction::triggered, this, &LogListWidget::copySelection); connect(clearAct, &QAction::triggered, this, &LogListWidget::clear); addAction(copyAct); @@ -78,10 +78,12 @@ void LogListWidget::keyPressEvent(QKeyEvent *event) void LogListWidget::appendLine(const QString &line, const Log::MsgType &type) { - auto *item = new QListWidgetItem; // We need to use QLabel here to support rich text - QLabel *lbl = new QLabel(line); + auto *lbl = new QLabel(line); + lbl->setTextFormat(Qt::RichText); lbl->setContentsMargins(4, 2, 4, 2); + + auto *item = new QListWidgetItem; item->setSizeHint(lbl->sizeHint()); item->setData(Qt::UserRole, type); insertItem(0, item); @@ -96,9 +98,10 @@ void LogListWidget::appendLine(const QString &line, const Log::MsgType &type) void LogListWidget::copySelection() { - static const QRegularExpression htmlTag("<[^>]+>"); + const QRegularExpression htmlTag("<[^>]+>"); + QStringList strings; - for (QListWidgetItem* it : asConst(selectedItems())) + for (QListWidgetItem *it : asConst(selectedItems())) strings << static_cast(itemWidget(it))->text().remove(htmlTag); QApplication::clipboard()->setText(strings.join('\n')); diff --git a/src/gui/loglistwidget.h b/src/gui/loglistwidget.h index c289ccc30..6dcb21d26 100644 --- a/src/gui/loglistwidget.h +++ b/src/gui/loglistwidget.h @@ -53,7 +53,7 @@ protected: void keyPressEvent(QKeyEvent *event) override; private: - int m_maxLines; + const int m_maxLines; Log::MsgTypes m_types; }; From 874bc84efc7949dea8b0a451fc588b4dffcb4b44 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Mon, 11 Mar 2019 01:00:47 +0800 Subject: [PATCH 3/5] Remove excessive usage of pointer --- src/app/filelogger.cpp | 30 +++++++++++------------------- src/app/filelogger.h | 5 ++--- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/app/filelogger.cpp b/src/app/filelogger.cpp index 45d160b9f..54b8ed8e6 100644 --- a/src/app/filelogger.cpp +++ b/src/app/filelogger.cpp @@ -30,7 +30,6 @@ #include #include -#include #include #include "base/global.h" @@ -40,7 +39,6 @@ FileLogger::FileLogger(const QString &path, const bool backup, const int maxSize, const bool deleteOld, const int age, const FileLogAgeType ageType) : m_backup(backup) , m_maxSize(maxSize) - , m_logFile(nullptr) { m_flusher.setInterval(0); m_flusher.setSingleShot(true); @@ -59,9 +57,7 @@ FileLogger::FileLogger(const QString &path, const bool backup, const int maxSize FileLogger::~FileLogger() { - if (!m_logFile) return; closeLogFile(); - delete m_logFile; } void FileLogger::changePath(const QString &newPath) @@ -73,11 +69,8 @@ void FileLogger::changePath(const QString &newPath) if (tmpPath != m_path) { m_path = tmpPath; - if (m_logFile) { - closeLogFile(); - delete m_logFile; - } - m_logFile = new QFile(m_path); + closeLogFile(); + m_logFile.setFileName(m_path); openLogFile(); } } @@ -119,9 +112,9 @@ void FileLogger::setMaxSize(const int value) void FileLogger::addLogMessage(const Log::Msg &msg) { - if (!m_logFile) return; + if (!m_logFile.isOpen()) return; - QTextStream str(m_logFile); + QTextStream str(&m_logFile); switch (msg.type) { case Log::INFO: @@ -139,7 +132,7 @@ void FileLogger::addLogMessage(const Log::Msg &msg) str << QDateTime::fromMSecsSinceEpoch(msg.timestamp).toString(Qt::ISODate) << " - " << msg.message << endl; - if (m_backup && (m_logFile->size() >= m_maxSize)) { + if (m_backup && (m_logFile.size() >= m_maxSize)) { closeLogFile(); int counter = 0; QString backupLogFilename = m_path + ".bak"; @@ -159,16 +152,15 @@ void FileLogger::addLogMessage(const Log::Msg &msg) void FileLogger::flushLog() { - if (m_logFile) - m_logFile->flush(); + if (m_logFile.isOpen()) + m_logFile.flush(); } void FileLogger::openLogFile() { - if (!m_logFile->open(QIODevice::WriteOnly | QIODevice::Append | QIODevice::Text) - || !m_logFile->setPermissions(QFile::ReadOwner | QFile::WriteOwner)) { - delete m_logFile; - m_logFile = nullptr; + if (!m_logFile.open(QIODevice::WriteOnly | QIODevice::Append | QIODevice::Text) + || !m_logFile.setPermissions(QFile::ReadOwner | QFile::WriteOwner)) { + m_logFile.close(); LogMsg(tr("An error occurred while trying to open the log file. Logging to file is disabled."), Log::CRITICAL); } } @@ -176,5 +168,5 @@ void FileLogger::openLogFile() void FileLogger::closeLogFile() { m_flusher.stop(); - m_logFile->close(); + m_logFile.close(); } diff --git a/src/app/filelogger.h b/src/app/filelogger.h index 5eefb3d90..78c8a2d5b 100644 --- a/src/app/filelogger.h +++ b/src/app/filelogger.h @@ -29,11 +29,10 @@ #ifndef FILELOGGER_H #define FILELOGGER_H +#include #include #include -class QFile; - namespace Log { struct Msg; @@ -71,7 +70,7 @@ private: QString m_path; bool m_backup; int m_maxSize; - QFile *m_logFile; + QFile m_logFile; QTimer m_flusher; }; From cb9a1603660ca2c92cbca2f6402c773c210ccbbf Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Mon, 11 Mar 2019 01:11:18 +0800 Subject: [PATCH 4/5] Utilize parent pointer appropriately --- src/gui/executionlogwidget.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/gui/executionlogwidget.cpp b/src/gui/executionlogwidget.cpp index 45c845976..a561a53ca 100644 --- a/src/gui/executionlogwidget.cpp +++ b/src/gui/executionlogwidget.cpp @@ -40,8 +40,8 @@ ExecutionLogWidget::ExecutionLogWidget(QWidget *parent, const Log::MsgTypes &types) : QWidget(parent) , m_ui(new Ui::ExecutionLogWidget) - , m_msgList(new LogListWidget(MAX_LOG_MESSAGES, types)) - , m_peerList(new LogListWidget(MAX_LOG_MESSAGES)) + , m_msgList(new LogListWidget(MAX_LOG_MESSAGES, types, this)) + , m_peerList(new LogListWidget(MAX_LOG_MESSAGES, Log::ALL, this)) { m_ui->setupUi(this); @@ -63,8 +63,6 @@ ExecutionLogWidget::ExecutionLogWidget(QWidget *parent, const Log::MsgTypes &typ ExecutionLogWidget::~ExecutionLogWidget() { - delete m_msgList; - delete m_peerList; delete m_ui; } From b7d739ab3f8471fa5b67f2610568a873caf20823 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Fri, 15 Mar 2019 14:29:54 +0800 Subject: [PATCH 5/5] Fix build error Apparently the function is not available on Windows platforms. --- src/app/application.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/app/application.cpp b/src/app/application.cpp index 124ed7942..7eef6bb3a 100644 --- a/src/app/application.cpp +++ b/src/app/application.cpp @@ -149,8 +149,10 @@ Application::Application(const QString &id, int &argc, char **argv) #if !defined(DISABLE_GUI) setAttribute(Qt::AA_UseHighDpiPixmaps, true); // opt-in to the high DPI pixmap support setQuitOnLastWindowClosed(false); +#if !defined(Q_OS_WIN) setDesktopFileName("org.qbittorrent.qBittorrent"); #endif +#endif #if defined(Q_OS_WIN) && !defined(DISABLE_GUI) connect(this, &QGuiApplication::commitDataRequest, this, &Application::shutdownCleanup, Qt::DirectConnection);