Merge pull request #10366 from Chocobo1/logger

Avoid performance penalty when logger is full

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);

src/app/filelogger.cpp

@@ -30,7 +30,6 @@
 
 #include <QDateTime>
 #include <QDir>
-#include <QFile>
 #include <QTextStream>
 
 #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,26 +57,20 @@ 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)
 {
-    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;
 
-        if (m_logFile) {
         closeLogFile();
-            delete m_logFile;
-        }
-        m_logFile = new QFile(m_path);
+        m_logFile.setFileName(m_path);
         openLogFile();
     }
 }
@@ -87,8 +79,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 +100,7 @@ void FileLogger::deleteOld(const int age, const FileLogAgeType ageType)
     }
 }
 
-void FileLogger::setBackup(bool value)
+void FileLogger::setBackup(const bool value)
 {
     m_backup = value;
 }
@@ -118,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:
@@ -138,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";
@@ -158,22 +152,21 @@ 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;
-        Logger::instance()->addMessage(tr("An error occurred while trying to open the log file. Logging to file is disabled."), Log::CRITICAL);
+    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);
     }
 }
 
 void FileLogger::closeLogFile()
 {
     m_flusher.stop();
-    m_logFile->close();
+    m_logFile.close();
 }

src/app/filelogger.h

@@ -29,11 +29,10 @@
 #ifndef FILELOGGER_H
 #define FILELOGGER_H
 
+#include <QFile>
 #include <QObject>
 #include <QTimer>
 
-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;
 };
 

src/base/logger.cpp

@@ -28,20 +28,31 @@
 
 #include "logger.h"
 
+#include <algorithm>
+
 #include <QDateTime>
-#include "base/utils/string.h"
+
+namespace
+{
+    template <typename T>
+    QVector<T> loadFromBuffer(const boost::circular_buffer_space_optimized<T> &src, const int offset = 0)
+    {
+        QVector<T> 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<Log::Msg> 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<Log::Peer> Logger::getPeers(const int lastKnownId) const
@@ -111,12 +116,12 @@ QVector<Log::Peer> 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)

src/base/logger.h

@@ -29,6 +29,8 @@
 #ifndef LOGGER_H
 #define LOGGER_H
 
+#include <boost/circular_buffer.hpp>
+
 #include <QObject>
 #include <QReadWriteLock>
 #include <QString>
@@ -89,14 +91,14 @@ signals:
 
 private:
     Logger();
-    ~Logger();
+    ~Logger() = default;
 
     static Logger *m_instance;
-    QVector<Log::Msg> m_messages;
-    QVector<Log::Peer> m_peers;
+    boost::circular_buffer_space_optimized<Log::Msg> m_messages;
+    boost::circular_buffer_space_optimized<Log::Peer> m_peers;
     mutable QReadWriteLock m_lock;
-    int m_msgCounter;
-    int m_peerCounter;
+    int m_msgCounter = 0;
+    int m_peerCounter = 0;
 };
 
 // Helper function

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, Log::MsgTypes(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;
 }
 
@@ -75,38 +73,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 = "<font color='grey'>" + time.toString(Qt::SystemLocaleShortDate) + "</font> - <font color='" + color.name() + "'>" + msg.message + "</font>";
+    const QDateTime time = QDateTime::fromMSecsSinceEpoch(msg.timestamp);
+    const QString text = QString(QLatin1String("<font color='grey'>%1</font> - <font color='%2'>%3</font>"))
+        .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 = "<font color='grey'>" + time.toString(Qt::SystemLocaleShortDate) + "</font> - "
-            + tr("<font color='red'>%1</font> was blocked %2", "x.y.z.w was blocked").arg(peer.ip, peer.reason);
-    else
-        text = "<font color='grey'>" + time.toString(Qt::SystemLocaleShortDate) + "</font> - " + tr("<font color='red'>%1</font> was banned", "x.y.z.w was banned").arg(peer.ip);
+    const QDateTime time = QDateTime::fromMSecsSinceEpoch(peer.timestamp);
+    const QString msg = QString(QLatin1String("<font color='grey'>%1</font> - <font color='red'>%2</font>"))
+        .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);
 }

src/gui/executionlogwidget.ui

@@ -10,9 +10,6 @@
     <height>300</height>
    </rect>
   </property>
-  <property name="windowTitle">
-   <string notr="true">Form</string>
-  </property>
   <layout class="QVBoxLayout" name="verticalLayout">
    <property name="leftMargin">
     <number>0</number>

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<QLabel*>(itemWidget(it))->text().remove(htmlTag);
 
     QApplication::clipboard()->setText(strings.join('\n'));

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;
 };