Simplify PerfElfMap::registerElf now that the input is sorted by time

Because the mmap events are now added in time order, we can simplify
the implementation of registerElf as we do not longer need to account
for older events getting added.

Change-Id: I131ca75fcb52e6e1f4238470f276f34a13bea537
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

2 files changed, 24 insertions, 59 deletions

diff --git a/app/perfelfmap.cpp b/app/perfelfmap.cpp
index 13273dd..e300de4 100644
--- a/app/perfelfmap.cpp
+++ b/app/perfelfmap.cpp
@@ -45,35 +45,25 @@ bool PerfElfMap::registerElf(const quint64 addr, const quint64 len, quint64 pgof
     const bool isFile = fullPath.isFile();
 
     QMultiMap<quint64, ElfInfo> fragments;
-    QMultiMap<quint64, ElfInfo>::iterator firstOverwrite = m_elfs.end();
     for (auto i = m_elfs.begin(), end = m_elfs.end(); i != end && i.key() < addrEnd; ++i) {
         const quint64 iEnd = i.key() + i->length;
         if (iEnd <= addr)
             continue;
 
-        if (time >= i->timeAdded) {
-            if (i->timeOverwritten > time) {
-                // Newly added elf overwrites existing one. Mark the existing one as overwritten and
-                // reinsert any fragments of it that remain.
-
-                if (i.key() < addr) {
-                    fragments.insertMulti(i.key(), ElfInfo(i->file, i.key(), addr - i.key(), i->pgoff,
-                                                           time, i->timeOverwritten));
-                }
-                if (iEnd > addrEnd) {
-                    fragments.insertMulti(addrEnd, ElfInfo(i->file, addrEnd, iEnd - addrEnd,
-                                                           i->pgoff + addrEnd - i.key(), time,
-                                                           i->timeOverwritten));
-                }
-                i->timeOverwritten = time;
+        if (i->timeOverwritten > time) {
+            // Newly added elf overwrites existing one. Mark the existing one as overwritten and
+            // reinsert any fragments of it that remain.
+
+            if (i.key() < addr) {
+                fragments.insertMulti(i.key(), ElfInfo(i->file, i.key(), addr - i.key(), i->pgoff,
+                                                        time, i->timeOverwritten));
             }
-        } else {
-            // Newly added elf is overwritten by existing one. Note the first overwrite and insert
-            // the remaining fragments in further passes.
-            if (i->timeAdded < overwritten) {
-                overwritten = i->timeAdded;
-                firstOverwrite = i;
+            if (iEnd > addrEnd) {
+                fragments.insertMulti(addrEnd, ElfInfo(i->file, addrEnd, iEnd - addrEnd,
+                                                        i->pgoff + addrEnd - i.key(), time,
+                                                        i->timeOverwritten));
             }
+            i->timeOverwritten = time;
         }
 
         // Overlapping module. Clear the cache, but only when the section is actually backed by a
@@ -83,16 +73,6 @@ bool PerfElfMap::registerElf(const quint64 addr, const quint64 len, quint64 pgof
             cacheInvalid = true;
     }
 
-    if (firstOverwrite != m_elfs.end()) {
-        const quint64 overwriteEnd = firstOverwrite.key() + firstOverwrite->length;
-        if (addr < firstOverwrite.key())
-            registerElf(addr, firstOverwrite.key() - addr, pgoff, overwritten, fullPath);
-        if (addrEnd > overwriteEnd) {
-            registerElf(overwriteEnd, addrEnd - overwriteEnd, pgoff + overwriteEnd - addr,
-                        overwritten, fullPath);
-        }
-    }
-
     m_elfs.unite(fragments);
     m_elfs.insertMulti(addr, ElfInfo(fullPath, addr, len, pgoff, time, overwritten));
 
diff --git a/tests/auto/elfmap/tst_elfmap.cpp b/tests/auto/elfmap/tst_elfmap.cpp
index 68e0c5a..f16aafe 100644
--- a/tests/auto/elfmap/tst_elfmap.cpp
+++ b/tests/auto/elfmap/tst_elfmap.cpp
@@ -79,19 +79,18 @@ private slots:
         QCOMPARE(map.findElf(0, 1), second);
         QCOMPARE(map.findElf(5, 1), second);
         QCOMPARE(map.findElf(9, 1), second);
-        QCOMPARE(map.findElf(10, 0), invalid);
+        QCOMPARE(map.findElf(10, 1), invalid);
         QCOMPARE(map.findElf(5, 2), second);
 
-        QCOMPARE(map.findElf(99, 0), invalid);
-        QCOMPARE(map.findElf(100, 0), first);
-        QCOMPARE(map.findElf(109, 0), first);
-        QCOMPARE(map.findElf(110, 0), invalid);
+        QCOMPARE(map.findElf(99, 1), invalid);
+        QCOMPARE(map.findElf(100, 1), first);
         QCOMPARE(map.findElf(105, 1), first);
+        QCOMPARE(map.findElf(109, 1), first);
+        QCOMPARE(map.findElf(110, 1), invalid);
     }
 
     void testOverwrite()
     {
-        QFETCH(bool, reversed);
         QFETCH(bool, firstIsFile);
         QFETCH(bool, secondIsFile);
 
@@ -114,15 +113,9 @@ private slots:
         const PerfElfMap::ElfInfo third(file2, 100, 20, 0, 2);
 
         PerfElfMap map;
-        if (!reversed) {
-            QCOMPARE(registerElf(&map, first), false);
-            QCOMPARE(registerElf(&map, second), firstIsFile);
-            QCOMPARE(registerElf(&map, third), firstIsFile || secondIsFile);
-        } else {
-            QCOMPARE(registerElf(&map, third), false);
-            QCOMPARE(registerElf(&map, second), firstIsFile || secondIsFile);
-            QCOMPARE(registerElf(&map, first), firstIsFile || secondIsFile);
-        }
+        QCOMPARE(registerElf(&map, first), false);
+        QCOMPARE(registerElf(&map, second), firstIsFile);
+        QCOMPARE(registerElf(&map, third), firstIsFile || secondIsFile);
 
         QCOMPARE(map.findElf(110, 0), first);
 
@@ -143,21 +136,13 @@ private slots:
 
     void testOverwrite_data()
     {
-        QTest::addColumn<bool>("reversed");
         QTest::addColumn<bool>("firstIsFile");
         QTest::addColumn<bool>("secondIsFile");
 
-        QTest::newRow("normal") << false << true << true;
-        QTest::newRow("reversed") << true << true << true;
-
-        QTest::newRow("normal-one-file-A") << false << false << true;
-        QTest::newRow("reversed-one-file-A") << true << false << true;
-
-        QTest::newRow("normal-one-file-B") << false << true << false;
-        QTest::newRow("reversed-one-file-B") << true << true << false;
-
-        QTest::newRow("normal-no-files") << false << false << false;
-        QTest::newRow("reversed-no-files") << true << false << false;
+        QTest::newRow("both-files") << true << true;
+        QTest::newRow("one-file-A") << false << true;
+        QTest::newRow("one-file-B") << true << false;
+        QTest::newRow("no-files") << false << false;
     }
 
     void testIsAddressInRange()