Only invalidate symbol cache when a previously used elf gets invalidated

We previously cleared the cache whenever an mmap event overlapped
a previously encountered mmap event. But often, these mmaps events
where not (yet) used for dwfl. As such, we do not have anything cached
from them which would be invalidated now. So keep the rest of the cache
alive to speed up the process slightly for some scenarios. One of my
files now gets parsed in ~5s instead of ~6s before. I see that the
cache is now only cleared 500 times instead of 2500 times.

Change-Id: If9c29946f6fb12a6b6d64888fe34d8add8eaebb4
Reviewed-by: Milian Wolff <milian.wolff@kdab.com>

4 files changed, 65 insertions, 52 deletions

diff --git a/app/perfelfmap.cpp b/app/perfelfmap.cpp
index 45776f0..85e5f54 100644
--- a/app/perfelfmap.cpp
+++ b/app/perfelfmap.cpp
@@ -59,13 +59,18 @@ struct SortByAddr
 };
 }
 
-bool PerfElfMap::registerElf(quint64 addr, quint64 len, quint64 pgoff,
+PerfElfMap::PerfElfMap(QObject *parent)
+    : QObject(parent)
+{
+}
+
+PerfElfMap::~PerfElfMap() = default;
+
+void PerfElfMap::registerElf(quint64 addr, quint64 len, quint64 pgoff,
                              const QFileInfo &fullPath, const QByteArray &originalFileName,
                              const QByteArray &originalPath)
 {
-    bool cacheInvalid = false;
     quint64 addrEnd = addr + len;
-    const bool isFile = fullPath.isFile();
 
     QVarLengthArray<ElfInfo, 8> newElfs;
     QVarLengthArray<int, 8> removedElfs;
@@ -83,10 +88,9 @@ bool PerfElfMap::registerElf(quint64 addr, quint64 len, quint64 pgoff,
             len = addrEnd - addr;
             if (addr == i->addr && len == i->length) {
                 // New mapping is fully contained in old one: Nothing to do.
-                Q_ASSERT(!cacheInvalid);
                 Q_ASSERT(newElfs.isEmpty());
                 Q_ASSERT(removedElfs.isEmpty());
-                return false;
+                return;
             }
         } else if (iEnd == addr) {
             // Directly adjacent sections of the same file can be merged. Ones of different files
@@ -107,13 +111,8 @@ bool PerfElfMap::registerElf(quint64 addr, quint64 len, quint64 pgoff,
                                       i->originalFileName, i->originalPath));
         }
 
+        aboutToInvalidate(*i);
         removedElfs.push_back(static_cast<int>(std::distance(m_elfs.begin(), i)));
-
-        // Overlapping module. Clear the cache, but only when the section is actually backed by a
-        // file. Otherwise, we will see tons of overlapping heap/anon sections which don't actually
-        // invalidate our caches
-        if (isFile || i->isFile())
-            cacheInvalid = true;
     }
 
     // remove the overwritten elfs, iterate from the back to not invalidate the indices
@@ -134,8 +133,6 @@ bool PerfElfMap::registerElf(quint64 addr, quint64 len, quint64 pgoff,
                                    elf.addr, SortByAddr());
         m_elfs.insert(it, elf);
     }
-
-    return cacheInvalid;
 }
 
 PerfElfMap::ElfInfo PerfElfMap::findElf(quint64 ip) const
diff --git a/app/perfelfmap.h b/app/perfelfmap.h
index e9262e4..9480f61 100644
--- a/app/perfelfmap.h
+++ b/app/perfelfmap.h
@@ -24,8 +24,9 @@
 #include <QVector>
 #include <limits>
 
-class PerfElfMap
+class PerfElfMap : public QObject
 {
+    Q_OBJECT
 public:
     struct ElfInfo {
         enum {
@@ -83,7 +84,10 @@ public:
         quint64 dwflEnd = 0;
     };
 
-    bool registerElf(quint64 addr, quint64 len, quint64 pgoff,
+    explicit PerfElfMap(QObject *parent = nullptr);
+    ~PerfElfMap();
+
+    void registerElf(quint64 addr, quint64 len, quint64 pgoff,
                      const QFileInfo &fullPath,
                      const QByteArray &originalFileName = {},
                      const QByteArray &originalPath = {});
@@ -97,6 +101,9 @@ public:
 
     bool isAddressInRange(quint64 addr) const;
 
+signals:
+    void aboutToInvalidate(const ElfInfo &elf);
+
 private:
     // elf sorted by start address
     QVector<ElfInfo> m_elfs;
diff --git a/app/perfsymboltable.cpp b/app/perfsymboltable.cpp
index 6db4fe5..0c1a7b3 100644
--- a/app/perfsymboltable.cpp
+++ b/app/perfsymboltable.cpp
@@ -57,7 +57,15 @@ PerfSymbolTable::PerfSymbolTable(qint32 pid, Dwfl_Callbacks *callbacks, PerfUnwi
     m_callbacks(callbacks),
     m_pid(pid)
 {
+    QObject::connect(&m_elfs, &PerfElfMap::aboutToInvalidate,
+                     &m_elfs, [this](const PerfElfMap::ElfInfo &elf) {
+                        if (m_dwfl && !m_cacheIsDirty && dwfl_addrmodule(m_dwfl, elf.addr)) {
+                            m_cacheIsDirty = true;
+                        }
+                     });
+
     m_dwfl = dwfl_begin(m_callbacks);
+
     dwfl_report_begin(m_dwfl);
 
     // "DWFL can not be used until this function returns 0"
@@ -310,13 +318,13 @@ void PerfSymbolTable::registerElf(const PerfRecordMmap &mmap, const QByteArray &
             fullPath = QFileInfo();
     }
 
-    bool cacheInvalid = m_elfs.registerElf(mmap.addr(), mmap.len(), mmap.pgoff(), fullPath,
-                                           fileName.toUtf8(), mmap.filename());
+    m_elfs.registerElf(mmap.addr(), mmap.len(), mmap.pgoff(), fullPath,
+                       fileName.toUtf8(), mmap.filename());
 
     // There is no need to clear the symbol or location caches in PerfUnwind. Some locations become
     // stale this way, but we still need to keep their IDs, as the receiver might still use them for
     // past frames.
-    if (cacheInvalid)
+    if (m_cacheIsDirty)
         clearCache();
 }
 
diff --git a/tests/auto/elfmap/tst_elfmap.cpp b/tests/auto/elfmap/tst_elfmap.cpp
index 6a38d09..c961b58 100644
--- a/tests/auto/elfmap/tst_elfmap.cpp
+++ b/tests/auto/elfmap/tst_elfmap.cpp
@@ -24,14 +24,6 @@
 #include <QTemporaryFile>
 #include <QTest>
 
-namespace {
-bool registerElf(PerfElfMap *map, const PerfElfMap::ElfInfo &info)
-{
-    return map->registerElf(info.addr, info.length, info.pgoff, info.localFile,
-                            info.originalFileName, info.originalPath);
-}
-}
-
 QT_BEGIN_NAMESPACE
 namespace QTest {
 template<>
@@ -58,7 +50,7 @@ private slots:
 
         const PerfElfMap::ElfInfo first({}, 100, 10, 0);
 
-        QVERIFY(!registerElf(&map, first));
+        QVERIFY(registerElf(&map, first).isEmpty());
         QVERIFY(!map.isEmpty());
 
         QCOMPARE(map.findElf(99), invalid);
@@ -68,7 +60,7 @@ private slots:
         QCOMPARE(map.findElf(110), invalid);
 
         const PerfElfMap::ElfInfo second({}, 0, 10, 0);
-        QVERIFY(!registerElf(&map, second));
+        QVERIFY(registerElf(&map, second).isEmpty());
 
         QCOMPARE(map.findElf(0), second);
         QCOMPARE(map.findElf(5), second);
@@ -103,32 +95,27 @@ private slots:
 
         PerfElfMap map;
 
-        {
-            const PerfElfMap::ElfInfo first(file1, 95, 20, 0);
-            QCOMPARE(registerElf(&map, first), false);
-            QCOMPARE(map.findElf(110), first);
-        }
+        const PerfElfMap::ElfInfo first(file1, 95, 20, 0);
+        QVERIFY(registerElf(&map, first).isEmpty());
+        QCOMPARE(map.findElf(110), first);
 
-        {
-            const PerfElfMap::ElfInfo second(file1, 105, 20, 0);
-            QCOMPARE(registerElf(&map, second), firstIsFile);
-            QCOMPARE(map.findElf(110), second);
+        const PerfElfMap::ElfInfo second(file1, 105, 20, 0);
+        QCOMPARE(registerElf(&map, second), QVector<PerfElfMap::ElfInfo>{first});
+        QCOMPARE(map.findElf(110), second);
 
-            const PerfElfMap::ElfInfo fragment1(file1, 95, 10, 0);
-            QCOMPARE(map.findElf(97), fragment1);
-        }
+        const PerfElfMap::ElfInfo fragment1(file1, 95, 10, 0);
+        QCOMPARE(map.findElf(97), fragment1);
 
-        {
-            const PerfElfMap::ElfInfo third(file2, 100, 20, 0);
-            QCOMPARE(registerElf(&map, third), firstIsFile || secondIsFile);
-            QCOMPARE(map.findElf(110), third);
-            QCOMPARE(map.findElf(110), third);
+        const PerfElfMap::ElfInfo third(file2, 100, 20, 0);
+        QVector<PerfElfMap::ElfInfo> invalidatedByThird = {fragment1, second};
+        QCOMPARE(registerElf(&map, third), invalidatedByThird);
+        QCOMPARE(map.findElf(110), third);
+        QCOMPARE(map.findElf(110), third);
 
-            const PerfElfMap::ElfInfo fragment2(file1, 120, 5, 15);
-            const PerfElfMap::ElfInfo fragment3(file1, 95, 5, 0);
-            QCOMPARE(map.findElf(122), fragment2);
-            QCOMPARE(map.findElf(97), fragment3);
-        }
+        const PerfElfMap::ElfInfo fragment2(file1, 120, 5, 15);
+        const PerfElfMap::ElfInfo fragment3(file1, 95, 5, 0);
+        QCOMPARE(map.findElf(122), fragment2);
+        QCOMPARE(map.findElf(97), fragment3);
     }
 
     void testOverwrite_data()
@@ -148,14 +135,14 @@ private slots:
         QVERIFY(!map.isAddressInRange(10));
 
         const PerfElfMap::ElfInfo first({}, 10, 10, 0);
-        QVERIFY(!registerElf(&map, first));
+        QVERIFY(registerElf(&map, first).isEmpty());
         QVERIFY(!map.isAddressInRange(9));
         QVERIFY(map.isAddressInRange(10));
         QVERIFY(map.isAddressInRange(19));
         QVERIFY(!map.isAddressInRange(20));
 
         const PerfElfMap::ElfInfo second({}, 30, 10, 0);
-        QVERIFY(!registerElf(&map, second));
+        QVERIFY(registerElf(&map, second).isEmpty());
         QVERIFY(!map.isAddressInRange(9));
         QVERIFY(map.isAddressInRange(10));
         QVERIFY(map.isAddressInRange(19));
@@ -350,6 +337,20 @@ private slots:
     {
         benchRegisterElfDisjunct_data();
     }
+
+private:
+    QVector<PerfElfMap::ElfInfo> registerElf(PerfElfMap *map, const PerfElfMap::ElfInfo &info)
+    {
+        QVector<PerfElfMap::ElfInfo> invalidated;
+        auto connection = connect(map, &PerfElfMap::aboutToInvalidate,
+                                  this, [&](const PerfElfMap::ElfInfo &other) {
+                                    invalidated.push_back(other);
+                                  });
+        map->registerElf(info.addr, info.length, info.pgoff, info.localFile,
+                         info.originalFileName, info.originalPath);
+        disconnect(connection);
+        return invalidated;
+    }
 };
 
 QTEST_GUILESS_MAIN(TestElfMap)