Revert back to using a mutex for process readings

Get rid of wanring "load() is deprecated". Besides, one mutex should
perform better than ten atomic ints. On my machine it's roughly twice
as fast.

Change-Id: I35254604b7739f44011b3799c5400a5d0140949a
Reviewed-by: Robert Griebl <robert.griebl@qt.io>

5 files changed, 129 insertions, 140 deletions

diff --git a/src/manager-lib/processstatus.cpp b/src/manager-lib/processstatus.cpp
index bc05f834..cb8e3cd1 100644
--- a/src/manager-lib/processstatus.cpp
+++ b/src/manager-lib/processstatus.cpp
@@ -43,6 +43,7 @@
 #include "processstatus.h"
 
 #include <QCoreApplication>
+#include <QMutexLocker>
 #include <QtQml/qqmlinfo.h>
 
 
@@ -152,8 +153,8 @@ ProcessStatus::ProcessStatus(QObject *parent)
     m_reader->moveToThread(m_workerThread);
 
     connect(m_reader, &ProcessReader::updated, this, [this]() {
+        fetchReadings();
         emit cpuLoadChanged();
-        fetchMemoryReadings();
         emit memoryReportingChanged(m_memoryVirtual, m_memoryRss, m_memoryPss);
         m_pendingUpdate = false;
     });
@@ -278,21 +279,25 @@ qint64 ProcessStatus::processId() const
 */
 qreal ProcessStatus::cpuLoad()
 {
-    return m_reader->cpuLoad.load() / ProcessReader::cpuLoadFactor;
+    return m_cpuLoad;
 }
 
-void ProcessStatus::fetchMemoryReadings()
+void ProcessStatus::fetchReadings()
 {
+    QMutexLocker locker(&m_reader->mutex);
+
+    m_cpuLoad = m_reader->cpuLoad;
+
     // Although smaps claims to report kB it's actually KiB (2^10 = 1024 Bytes)
-    m_memoryVirtual[qSL("total")] = quint64(m_reader->totalVm.load()) << 10;
-    m_memoryVirtual[qSL("text")] = quint64(m_reader->textVm.load()) << 10;
-    m_memoryVirtual[qSL("heap")] = quint64(m_reader->heapVm.load()) << 10;
-    m_memoryRss[qSL("total")] = quint64(m_reader->totalRss.load()) << 10;
-    m_memoryRss[qSL("text")] = quint64(m_reader->textRss.load()) << 10;
-    m_memoryRss[qSL("heap")] = quint64(m_reader->heapRss.load()) << 10;
-    m_memoryPss[qSL("total")] = quint64(m_reader->totalPss.load()) << 10;
-    m_memoryPss[qSL("text")] = quint64(m_reader->textPss.load()) << 10;
-    m_memoryPss[qSL("heap")] = quint64(m_reader->heapPss.load()) << 10;
+    m_memoryVirtual[qSL("total")] = static_cast<quint64>(m_reader->memory.totalVm) << 10;
+    m_memoryVirtual[qSL("text")] = static_cast<quint64>(m_reader->memory.textVm) << 10;
+    m_memoryVirtual[qSL("heap")] = static_cast<quint64>(m_reader->memory.heapVm) << 10;
+    m_memoryRss[qSL("total")] = static_cast<quint64>(m_reader->memory.totalRss) << 10;
+    m_memoryRss[qSL("text")] = static_cast<quint64>(m_reader->memory.textRss) << 10;
+    m_memoryRss[qSL("heap")] = static_cast<quint64>(m_reader->memory.heapRss) << 10;
+    m_memoryPss[qSL("total")] = static_cast<quint64>(m_reader->memory.totalPss) << 10;
+    m_memoryPss[qSL("text")] = static_cast<quint64>(m_reader->memory.textPss) << 10;
+    m_memoryPss[qSL("heap")] = static_cast<quint64>(m_reader->memory.heapPss) << 10;
 }
 
 /*!
diff --git a/src/manager-lib/processstatus.h b/src/manager-lib/processstatus.h
index 282ad13f..a2f4f879 100644
--- a/src/manager-lib/processstatus.h
+++ b/src/manager-lib/processstatus.h
@@ -102,12 +102,13 @@ private slots:
     void onRunStateChanged(Am::RunState state);
 
 private:
-    void fetchMemoryReadings();
+    void fetchReadings();
     void determinePid();
 
     QString m_appId;
     qint64 m_pid = 0;
 
+    qreal m_cpuLoad = 0;
     QVariantMap m_memoryVirtual;
     QVariantMap m_memoryRss;
     QVariantMap m_memoryPss;
diff --git a/src/monitor-lib/processreader.cpp b/src/monitor-lib/processreader.cpp
index e8b760a9..bbd966f4 100644
--- a/src/monitor-lib/processreader.cpp
+++ b/src/monitor-lib/processreader.cpp
@@ -40,6 +40,7 @@
 **
 ****************************************************************************/
 
+#include <QMutexLocker>
 #include "processreader.h"
 
 #include "logging.h"
@@ -63,32 +64,27 @@ void ProcessReader::enableMemoryReporting(bool enabled)
 {
     m_memoryReportingEnabled = enabled;
     if (!m_memoryReportingEnabled)
-        zeroMemory();
+        memory = Memory();
 }
 
 void ProcessReader::update()
 {
-    cpuLoad.store(static_cast<quint32>(cpuLoadFactor * readCpuLoad()));
-
-    if (m_memoryReportingEnabled && !readMemory())
-        zeroMemory();
+    qreal load = readCpuLoad();
+
+    if (m_memoryReportingEnabled) {
+        Memory mem;
+        bool memRead = readMemory(mem);
+        QMutexLocker locker(&mutex);
+        memory = memRead ? mem : Memory();
+        cpuLoad = load;
+    } else {
+        QMutexLocker locker(&mutex);
+        cpuLoad = load;
+    }
 
     emit updated();
 }
 
-void ProcessReader::zeroMemory()
-{
-    totalVm.store(0);
-    totalRss.store(0);
-    totalPss.store(0);
-    textVm.store(0);
-    textRss.store(0);
-    textPss.store(0);
-    heapVm.store(0);
-    heapRss.store(0);
-    heapPss.store(0);
-}
-
 #if defined(Q_OS_LINUX)
 
 void ProcessReader::openCpuLoad()
@@ -134,10 +130,10 @@ qreal ProcessReader::readCpuLoad()
 }
 
 
-bool ProcessReader::readMemory()
+bool ProcessReader::readMemory(Memory &mem)
 {
-    QByteArray smapsFile = "/proc/" + QByteArray::number(m_pid) + "/smaps";
-    return readSmaps(smapsFile);
+    const QByteArray smapsFile = "/proc/" + QByteArray::number(m_pid) + "/smaps";
+    return readSmaps(smapsFile, mem);
 }
 
 static uint parseValue(const char *pl) {
@@ -146,18 +142,8 @@ static uint parseValue(const char *pl) {
     return static_cast<uint>(strtoul(pl, nullptr, 10));
 }
 
-bool ProcessReader::readSmaps(const QByteArray &smapsFile)
+bool ProcessReader::readSmaps(const QByteArray &smapsFile, Memory &mem)
 {
-    quint32 _totalVm = 0;
-    quint32 _totalRss = 0;
-    quint32 _totalPss = 0;
-    quint32 _textVm = 0;
-    quint32 _textRss = 0;
-    quint32 _textPss = 0;
-    quint32 _heapVm = 0;
-    quint32 _heapRss = 0;
-    quint32 _heapPss = 0;
-
     struct ScopedFile {
         ~ScopedFile() { if (file) fclose(file); }
         FILE *file = nullptr;
@@ -288,22 +274,22 @@ bool ProcessReader::readSmaps(const QByteArray &smapsFile)
         if (foundTags < allTags)
             break;
 
-        _totalVm += vm;
-        _totalRss += rss;
-        _totalPss += pss;
+        mem.totalVm += vm;
+        mem.totalRss += rss;
+        mem.totalPss += pss;
 
         static const char permRXP[] = { 'r', '-', 'x', 'p' };
         static const char permRWP[] = { 'r', 'w', '-', 'p' };
         if (!memcmp(permissions, permRXP, sizeof(permissions))) {
-            _textVm += vm;
-            _textRss += rss;
-            _textPss += pss;
+            mem.textVm += vm;
+            mem.textRss += rss;
+            mem.textPss += pss;
         } else if (!memcmp(permissions, permRWP, sizeof(permissions))
                    && !isMainStack && (vm != 8192 || hasInode || !wasPrivateOnly) // try to exclude stack
                    && !hasInode) {
-            _heapVm += vm;
-            _heapRss += rss;
-            _heapPss += pss;
+            mem.heapVm += vm;
+            mem.heapRss += rss;
+            mem.heapPss += pss;
         }
 
         static const char permP[] = { '-', '-', '-', 'p' };
@@ -315,22 +301,15 @@ bool ProcessReader::readSmaps(const QByteArray &smapsFile)
         }
     }
 
-    if (ok) {
-        // publish the readings
-        totalVm.store(_totalVm);
-        totalRss.store(_totalRss);
-        totalPss.store(_totalPss);
-        textVm.store(_textVm);
-        textRss.store(_textRss);
-        textPss.store(_textPss);
-        heapVm.store(_heapVm);
-        heapRss.store(_heapRss);
-        heapPss.store(_heapPss);
-    }
-
     return ok;
 }
 
+bool ProcessReader::testReadSmaps(const QByteArray &smapsFile)
+{
+    memory = Memory();
+    return readSmaps(smapsFile, memory);
+}
+
 #elif defined(Q_OS_MACOS)
 
 void ProcessReader::openCpuLoad()
@@ -342,7 +321,7 @@ qreal ProcessReader::readCpuLoad()
     return 0.0;
 }
 
-bool ProcessReader::readMemory()
+bool ProcessReader::readMemory(Memory &mem)
 {
     struct task_basic_info t_info;
     mach_msg_type_number_t t_info_count = TASK_BASIC_INFO_COUNT;
@@ -352,8 +331,8 @@ bool ProcessReader::readMemory()
         return false;
     }
 
-    totalRss.store(t_info.resident_size);
-    totalVm.store(t_info.virtual_size);
+    mem.totalRss = t_info.resident_size;
+    mem.totalVm = t_info.virtual_size;
 
     return true;
 }
@@ -369,8 +348,9 @@ qreal ProcessReader::readCpuLoad()
     return 0.0;
 }
 
-bool ProcessReader::readMemory()
+bool ProcessReader::readMemory(Memory &mem)
 {
+    Q_UNUSED(mem)
     return false;
 }
 
diff --git a/src/monitor-lib/processreader.h b/src/monitor-lib/processreader.h
index 9d63790f..d1eace4a 100644
--- a/src/monitor-lib/processreader.h
+++ b/src/monitor-lib/processreader.h
@@ -42,7 +42,7 @@
 
 #pragma once
 
-#include <QAtomicInteger>
+#include <QMutex>
 #include <QElapsedTimer>
 #include <QObject>
 
@@ -57,6 +57,27 @@ QT_BEGIN_NAMESPACE_AM
 
 class ProcessReader : public QObject {
     Q_OBJECT
+
+public:
+    QMutex mutex;
+    qreal cpuLoad;
+    struct Memory {
+        quint32 totalVm = 0;
+        quint32 totalRss = 0;
+        quint32 totalPss = 0;
+        quint32 textVm = 0;
+        quint32 textRss = 0;
+        quint32 textPss = 0;
+        quint32 heapVm = 0;
+        quint32 heapRss = 0;
+        quint32 heapPss = 0;
+    } memory;
+
+#if defined(Q_OS_LINUX)
+    // solely for testing purposes
+    bool testReadSmaps(const QByteArray &smapsFile);
+#endif
+
 public slots:
     void update();
     void setProcessId(qint64 pid);
@@ -65,32 +86,14 @@ public slots:
 signals:
     void updated();
 
-public:
-    QAtomicInteger<quint32> cpuLoad;
-
-    QAtomicInteger<quint32> totalVm;
-    QAtomicInteger<quint32> totalRss;
-    QAtomicInteger<quint32> totalPss;
-    QAtomicInteger<quint32> textVm;
-    QAtomicInteger<quint32> textRss;
-    QAtomicInteger<quint32> textPss;
-    QAtomicInteger<quint32> heapVm;
-    QAtomicInteger<quint32> heapRss;
-    QAtomicInteger<quint32> heapPss;
-
-#if defined(Q_OS_LINUX)
-    // it's public solely for testing purposes
-    bool readSmaps(const QByteArray &smapsFile);
-#endif
-    static constexpr qreal cpuLoadFactor = 1000000.0;
-
 private:
     void openCpuLoad();
     qreal readCpuLoad();
-    bool readMemory();
-    void zeroMemory();
+    bool readMemory(Memory &mem);
 
 #if defined(Q_OS_LINUX)
+    bool readSmaps(const QByteArray &smapsFile, Memory &mem);
+
     QScopedPointer<SysFsReader> m_statReader;
 #endif
     QElapsedTimer m_elapsedTime;
diff --git a/tests/processreader/tst_processreader.cpp b/tests/processreader/tst_processreader.cpp
index 978770a0..da5d61ac 100644
--- a/tests/processreader/tst_processreader.cpp
+++ b/tests/processreader/tst_processreader.cpp
@@ -68,74 +68,74 @@ void tst_ProcessReader::memInvalid()
 {
     QFETCH(QString, file);
 
-    reader.readSmaps(file.toLocal8Bit());
-
-    QCOMPARE(reader.totalVm.load(), 0u);
-    QCOMPARE(reader.totalRss.load(), 0u);
-    QCOMPARE(reader.totalPss.load(), 0u);
-    QCOMPARE(reader.textVm.load(), 0u);
-    QCOMPARE(reader.textRss.load(), 0u);
-    QCOMPARE(reader.textPss.load(), 0u);
-    QCOMPARE(reader.heapVm.load(), 0u);
-    QCOMPARE(reader.heapRss.load(), 0u);
-    QCOMPARE(reader.heapPss.load(), 0u);
+    reader.testReadSmaps(file.toLocal8Bit());
+
+    QCOMPARE(reader.memory.totalVm, 0u);
+    QCOMPARE(reader.memory.totalRss, 0u);
+    QCOMPARE(reader.memory.totalPss, 0u);
+    QCOMPARE(reader.memory.textVm, 0u);
+    QCOMPARE(reader.memory.textRss, 0u);
+    QCOMPARE(reader.memory.textPss, 0u);
+    QCOMPARE(reader.memory.heapVm, 0u);
+    QCOMPARE(reader.memory.heapRss, 0u);
+    QCOMPARE(reader.memory.heapPss, 0u);
 }
 
 void tst_ProcessReader::memTestProcess()
 {
     const QByteArray file = "/proc/" + QByteArray::number(QCoreApplication::applicationPid()) + "/smaps";
 
-    QVERIFY(reader.readSmaps(file));
+    QVERIFY(reader.testReadSmaps(file));
     //printMem(reader);
-    QVERIFY(reader.totalVm.load() >= reader.totalRss.load());
-    QVERIFY(reader.totalRss.load() >= reader.totalPss.load());
-    QVERIFY(reader.textVm.load() >= reader.textRss.load());
-    QVERIFY(reader.textRss.load() >= reader.textPss.load());
-    QVERIFY(reader.heapVm.load() >= reader.heapRss.load());
-    QVERIFY(reader.heapRss.load() >= reader.heapPss.load());
+    QVERIFY(reader.memory.totalVm >= reader.memory.totalRss);
+    QVERIFY(reader.memory.totalRss >= reader.memory.totalPss);
+    QVERIFY(reader.memory.textVm >= reader.memory.textRss);
+    QVERIFY(reader.memory.textRss >= reader.memory.textPss);
+    QVERIFY(reader.memory.heapVm >= reader.memory.heapRss);
+    QVERIFY(reader.memory.heapRss >= reader.memory.heapPss);
 }
 
 void tst_ProcessReader::memBasic()
 {
-    QVERIFY(reader.readSmaps(QFINDTESTDATA("basic.smaps").toLocal8Bit()));
+    QVERIFY(reader.testReadSmaps(QFINDTESTDATA("basic.smaps").toLocal8Bit()));
     //printMem(reader);
-    QCOMPARE(reader.totalVm.load(), 107384u);
-    QCOMPARE(reader.totalRss.load(), 20352u);
-    QCOMPARE(reader.totalPss.load(), 13814u);
-    QCOMPARE(reader.textVm.load(), 7800u);
-    QCOMPARE(reader.textRss.load(), 5884u);
-    QCOMPARE(reader.textPss.load(), 2318u);
-    QCOMPARE(reader.heapVm.load(), 24376u);
-    QCOMPARE(reader.heapRss.load(), 7556u);
-    QCOMPARE(reader.heapPss.load(), 7556u);
+    QCOMPARE(reader.memory.totalVm, 107384u);
+    QCOMPARE(reader.memory.totalRss, 20352u);
+    QCOMPARE(reader.memory.totalPss, 13814u);
+    QCOMPARE(reader.memory.textVm, 7800u);
+    QCOMPARE(reader.memory.textRss, 5884u);
+    QCOMPARE(reader.memory.textPss, 2318u);
+    QCOMPARE(reader.memory.heapVm, 24376u);
+    QCOMPARE(reader.memory.heapRss, 7556u);
+    QCOMPARE(reader.memory.heapPss, 7556u);
 }
 
 void tst_ProcessReader::memAdvanced()
 {
-    QVERIFY(reader.readSmaps(QFINDTESTDATA("advanced.smaps").toLocal8Bit()));
+    QVERIFY(reader.testReadSmaps(QFINDTESTDATA("advanced.smaps").toLocal8Bit()));
     //printMem(reader);
-    QCOMPARE(reader.totalVm.load(), 77728u);
-    QCOMPARE(reader.totalRss.load(), 17612u);
-    QCOMPARE(reader.totalPss.load(), 17547u);
-    QCOMPARE(reader.textVm.load(), 2104u);
-    QCOMPARE(reader.textRss.load(), 1772u);
-    QCOMPARE(reader.textPss.load(), 1707u);
-    QCOMPARE(reader.heapVm.load(), 16032u);
-    QCOMPARE(reader.heapRss.load(), 15740u);
-    QCOMPARE(reader.heapPss.load(), 15740u);
+    QCOMPARE(reader.memory.totalVm, 77728u);
+    QCOMPARE(reader.memory.totalRss, 17612u);
+    QCOMPARE(reader.memory.totalPss, 17547u);
+    QCOMPARE(reader.memory.textVm, 2104u);
+    QCOMPARE(reader.memory.textRss, 1772u);
+    QCOMPARE(reader.memory.textPss, 1707u);
+    QCOMPARE(reader.memory.heapVm, 16032u);
+    QCOMPARE(reader.memory.heapRss, 15740u);
+    QCOMPARE(reader.memory.heapPss, 15740u);
 }
 
 void tst_ProcessReader::printMem(const ProcessReader &reader)
 {
-    qDebug() << "totalVm:" << reader.totalVm.load();
-    qDebug() << "totalRss:" << reader.totalRss.load();
-    qDebug() << "totalPss:" << reader.totalPss.load();
-    qDebug() << "textVm:" << reader.textVm.load();
-    qDebug() << "textRss:" << reader.textRss.load();
-    qDebug() << "textPss:" << reader.textPss.load();
-    qDebug() << "heapVm:" << reader.heapVm.load();
-    qDebug() << "heapRss:" << reader.heapRss.load();
-    qDebug() << "heapPss:" << reader.heapPss.load();
+    qDebug() << "totalVm:" << reader.memory.totalVm;
+    qDebug() << "totalRss:" << reader.memory.totalRss;
+    qDebug() << "totalPss:" << reader.memory.totalPss;
+    qDebug() << "textVm:" << reader.memory.textVm;
+    qDebug() << "textRss:" << reader.memory.textRss;
+    qDebug() << "textPss:" << reader.memory.textPss;
+    qDebug() << "heapVm:" << reader.memory.heapVm;
+    qDebug() << "heapRss:" << reader.memory.heapRss;
+    qDebug() << "heapPss:" << reader.memory.heapPss;
 }
 
 QTEST_APPLESS_MAIN(tst_ProcessReader)