Improve the bottom-up conversion to top-down and caller/callee data

When we encounter broken backtraces, we may not go all the way up
to main. In such cases, a non-leaf frame may actually have a cost
higher than the sum of its children. In these cases, we also have
to hande the difference in cost just like for a normal leaf node.

The manual test I added does not cover this properly, as the
unwinding is too reliable and thus we do not run into the corner case.
In real-world profiles though, this does occur and adds some errors.
This is now fixed and the caller/callee and top-down view become
more reliable.

diff --git a/src/analyze/allocationdata.h b/src/analyze/allocationdata.h
index 891ec50..f00e222 100644 (file)
--- a/src/analyze/allocationdata.h
+++ b/src/analyze/allocationdata.h
@@ -42,6 +42,11 @@ inline bool operator==(const AllocationData& lhs, const AllocationData& rhs)
         && lhs.leaked == rhs.leaked && lhs.peak == rhs.peak;
 }
 
+inline bool operator!=(const AllocationData& lhs, const AllocationData& rhs)
+{
+    return !(lhs == rhs);
+}
+
 inline AllocationData& operator+=(AllocationData& lhs, const AllocationData& rhs)
 {
     lhs.allocations += rhs.allocations;

diff --git a/src/analyze/gui/parser.cpp b/src/analyze/gui/parser.cpp
index 9b0a3cf..f582490 100644 (file)
--- a/src/analyze/gui/parser.cpp
+++ b/src/analyze/gui/parser.cpp
@@ -347,11 +347,17 @@ RowData* findByLocation(const RowData& row, QVector<RowData>* data)
     return nullptr;
 }
 
-void buildTopDown(const TreeData& bottomUpData, TreeData* topDownData)
+AllocationData buildTopDown(const TreeData& bottomUpData, TreeData* topDownData)
 {
-    foreach (const auto& row, bottomUpData) {
-        if (row.children.isEmpty()) {
-            // leaf node found, bubble up the parent chain to build a top-down tree
+    AllocationData totalCost;
+    for (const auto& row : bottomUpData) {
+        // recurse and find the cost attributed to children
+        const auto childCost = buildTopDown(row.children, topDownData);
+        if (childCost != row.cost) {
+            // this row is (partially) a leaf
+            const auto cost = row.cost - childCost;
+
+            // bubble up the parent chain to build a top-down tree
             auto node = &row;
             auto stack = topDownData;
             while (node) {
@@ -363,15 +369,14 @@ void buildTopDown(const TreeData& bottomUpData, TreeData* topDownData)
                 }
                 // always use the leaf node's cost and propagate that one up the chain
                 // otherwise we'd count the cost of some nodes multiple times
-                data->cost += row.cost;
+                data->cost += cost;
                 stack = &data->children;
                 node = node->parent;
             }
-        } else {
-            // recurse to find a leaf
-            buildTopDown(row.children, topDownData);
         }
+        totalCost += row.cost;
     }
+    return totalCost;
 }
 
 QVector<RowData> toTopDownData(const QVector<RowData>& bottomUpData)
@@ -383,8 +388,7 @@ QVector<RowData> toTopDownData(const QVector<RowData>& bottomUpData)
     return topRows;
 }
 
-
-void buildCallerCallee(const TreeData& bottomUpData, CallerCalleeRows* callerCalleeData)
+void buildCallerCallee2(const TreeData& bottomUpData, CallerCalleeRows* callerCalleeData)
 {
     foreach (const auto& row, bottomUpData) {
         if (row.children.isEmpty()) {
@@ -412,9 +416,47 @@ void buildCallerCallee(const TreeData& bottomUpData, CallerCalleeRows* callerCal
             }
         } else {
             // recurse to find a leaf
-            buildCallerCallee(row.children, callerCalleeData);
+            buildCallerCallee2(row.children, callerCalleeData);
+        }
+    }
+}
+
+AllocationData buildCallerCallee(const TreeData& bottomUpData, CallerCalleeRows* callerCalleeData)
+{
+    AllocationData totalCost;
+    for (const auto& row : bottomUpData) {
+        // recurse to find a leaf
+        const auto childCost = buildCallerCallee(row.children, callerCalleeData);
+        if (childCost != row.cost) {
+            // this row is (partially) a leaf
+            const auto cost = row.cost - childCost;
+
+            // leaf node found, bubble up the parent chain to add cost for all frames
+            // to the caller/callee data. this is done top-down since we must not count
+            // symbols more than once in the caller-callee data
+            QSet<LocationData::Ptr> recursionGuard;
+
+            auto node = &row;
+            while (node) {
+                const auto& location = node->location;
+                if (!recursionGuard.contains(location)) { // aggregate caller-callee data
+                    auto it = lower_bound(callerCalleeData->begin(), callerCalleeData->end(), location,
+                        [](const CallerCalleeData& lhs, const LocationData::Ptr& rhs) { return lhs.location < rhs; });
+                    if (it == callerCalleeData->end() || it->location != location) {
+                        it = callerCalleeData->insert(it, {{}, {}, location});
+                    }
+                    it->inclusiveCost += cost;
+                    if (!node->parent) {
+                        it->selfCost += cost;
+                    }
+                    recursionGuard.insert(location);
+                }
+                node = node->parent;
+            }
         }
+        totalCost += row.cost;
     }
+    return totalCost;
 }
 
 CallerCalleeRows toCallerCalleeData(const QVector<RowData>& bottomUpData, bool diffMode)

diff --git a/tests/manual/CMakeLists.txt b/tests/manual/CMakeLists.txt
index 8c10c18..63f9526 100644 (file)
--- a/tests/manual/CMakeLists.txt
+++ b/tests/manual/CMakeLists.txt
@@ -10,3 +10,5 @@ add_executable(callgraph callgraph.cpp)
 add_library(testlib SHARED lib.cpp)
 add_executable(test_lib test_lib.cpp)
 target_link_libraries(test_lib testlib)
+
+add_executable(test_aggregation test_aggregation.cpp)

diff --git a/tests/manual/test_aggregation.cpp b/tests/manual/test_aggregation.cpp
new file mode 100644 (file)
index 0000000..0afbd1c
--- /dev/null
+++ b/tests/manual/test_aggregation.cpp
@@ -0,0 +1,25 @@
+void foo()
+{
+    new char[1];
+}
+
+void bar()
+{
+    foo(); new char[2];
+}
+
+void asdf()
+{
+    bar(); new char[3];
+}
+
+void foobar()
+{
+    asdf(); new char[5];
+}
+
+int main()
+{
+    asdf(); new char[4]; foobar();
+    return 0;
+}