callstack.core: check monitor more often for callgraph analysis

This commit adds the possibility of canceling a very long callgraph analysis. Without this, the callgraph analysis prevents Trace Compass from exiting correctly and would not cancel properly. Signed-off-by: Arnaud Fiorini <fiorini.arnaud@gmail.com> Change-Id: Ib805a4aed35e9d4218b8f8b6a8ac7f245fff34c3 Reviewed-on: https://git.eclipse.org/r/c/tracecompass/org.eclipse.tracecompass/+/205185 Tested-by: Trace Compass Bot <tracecompass-bot@eclipse.org> Tested-by: Bernd Hufmann <bernd.hufmann@ericsson.com> Tested-by: Matthew Khouzam <matthew.khouzam@ericsson.com> Reviewed-by: Bernd Hufmann <bernd.hufmann@ericsson.com> Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
eclipse-tracecompass · Nov 20, 2023 · ee914eb · ee914eb
1 parent 2d56f43
commit ee914eb
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 23 deletions.
diff --git a/...mpass/analysis/callstack/core/tests/callgraph/AggregatedCalledFunctionStatisticsTest.java b/...mpass/analysis/callstack/core/tests/callgraph/AggregatedCalledFunctionStatisticsTest.java
@@ -13,7 +13,6 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -146,7 +145,7 @@ public void treeStatisticsTest() {
         // Execute the CallGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         // Test the threads generated by the analysis
         assertNotNull(threads);
@@ -232,7 +231,7 @@ public void mergeFirstLevelCalleesStatisticsTest() {
         // Execute the CallGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         assertNotNull(threads);
         ICallStackElement thread = threads.get(0);
@@ -328,7 +327,7 @@ public void multiFunctionRootsTest() {
         // Execute the callGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         // Test the threads generated by the analysis
         assertNotNull(threads);
@@ -443,7 +442,7 @@ public void mergeSecondLevelCalleesTest() {
         // Execute the CallGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         // Test the threads generated by the analysis
         assertNotNull(threads);

diff --git a/...org/eclipse/tracecompass/analysis/callstack/core/tests/callgraph/AggregationTreeTest.java b/...org/eclipse/tracecompass/analysis/callstack/core/tests/callgraph/AggregationTreeTest.java
@@ -13,7 +13,6 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.util.ArrayList;
@@ -172,7 +171,7 @@ public void emptyStateSystemTest() {
         fixture.closeHistory(1002);
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         Collection<ICallStackElement> threads = getLeafElements(cga);
         assertNotNull(threads);
         assertEquals("Number of threads found", 0, threads.size());
@@ -211,7 +210,7 @@ public void cascadeTest() {
 
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         // Test the threads generated by the analysis
         assertNotNull(threads);
@@ -282,7 +281,7 @@ public void treeTest() {
         // Execute the CallGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         @NonNull
         List<ICallStackElement> threads = getLeafElements(cga);
         // Test the threads generated by the analysis
@@ -358,7 +357,7 @@ public void mergeFirstLevelCalleesTest() {
         String @NonNull [] pp = { CallGraphAnalysisStub.PROCESS_PATH };
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture, ImmutableList.of(pp, tp));
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
 
         CallGraph cg = cga.getCallGraph();
@@ -467,7 +466,7 @@ public void mergeSecondLevelCalleesTest() {
         // Execute the CallGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         CallGraph cg = cga.getCallGraph();
         // Test the threads generated by the analysis
@@ -539,7 +538,7 @@ public void largeTest() {
         // Execute the callGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         // Test the threads generated by the analysis
         assertNotNull(threads);
@@ -591,7 +590,7 @@ public void multiFunctionRootsTest() {
         // Execute the callGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         CallGraph cg = cga.getCallGraph();
         // Test the threads generated by the analysis
@@ -661,7 +660,7 @@ public void multiFunctionRootsSecondTest() {
         // Execute the callGraphAnalysis
         CallGraphAnalysisStub cga = new CallGraphAnalysisStub(fixture);
         setCga(cga);
-        assertTrue(cga.iterate());
+        cga.iterate();
         List<ICallStackElement> threads = getLeafElements(cga);
         CallGraph cg = cga.getCallGraph();
         // Test the threads generated by the analysis

diff --git a/...s/org/eclipse/tracecompass/analysis/callstack/core/tests/stubs/CallGraphAnalysisStub.java b/...s/org/eclipse/tracecompass/analysis/callstack/core/tests/stubs/CallGraphAnalysisStub.java
@@ -123,15 +123,13 @@ private CallGraphAnalysisStub(IFlameChartProvider csp) {
 
     /**
      * Will trigger the iteration over the callstack series
-     *
-     * @return The return value of the iteration
      */
-    public boolean iterate() {
+    public void iterate() {
         CallStackSeries callStackSeries = fCsProvider.getCallStackSeries();
         if (callStackSeries == null) {
             throw new IllegalArgumentException();
         }
-        return iterateOverCallstackSerie(callStackSeries, ModelManager.getModelFor(""), getCallGraph(), 0, Long.MAX_VALUE, new NullProgressMonitor());
+        iterateOverCallstackSerie(callStackSeries, ModelManager.getModelFor(""), getCallGraph(), 0, Long.MAX_VALUE, new NullProgressMonitor());
     }
 
     @Override

diff --git a/...rg/eclipse/tracecompass/internal/analysis/callstack/core/callgraph/CallGraphAnalysis.java b/...rg/eclipse/tracecompass/internal/analysis/callstack/core/callgraph/CallGraphAnalysis.java
@@ -166,7 +166,8 @@ private boolean executeForRange(CallGraph callgraph, TmfTimeRange range, @Nullab
             long time1 = range.getEndTime().toNanos();
             long start = Math.min(time0, time1);
             long end = Math.max(time0, time1);
-            if (!iterateOverCallstackSerie(callstack, model, callgraph, start, end, monitor)) {
+            iterateOverCallstackSerie(callstack, model, callgraph, start, end, monitor);
+            if (monitor.isCanceled()) {
                 return false;
             }
         }
@@ -191,19 +192,17 @@ private boolean executeForRange(CallGraph callgraph, TmfTimeRange range, @Nullab
      *            The end time of the request
      * @param monitor
      *            A progress monitor
-     * @return Whether the series was successfully iterated over
      */
     @VisibleForTesting
-    protected boolean iterateOverCallstackSerie(CallStackSeries callstackSerie, IHostModel model, CallGraph callgraph, long start, long end, IProgressMonitor monitor) {
+    protected void iterateOverCallstackSerie(CallStackSeries callstackSerie, IHostModel model, CallGraph callgraph, long start, long end, IProgressMonitor monitor) {
         // The root elements are the same as the one from the callstack series
         Collection<ICallStackElement> rootElements = callstackSerie.getRootElements();
         for (ICallStackElement element : rootElements) {
             if (monitor.isCanceled()) {
-                return false;
+                return;
             }
             iterateOverElement(element, model, callgraph, start, end, monitor);
         }
-        return true;
     }
 
     private void iterateOverElement(ICallStackElement element, IHostModel model, CallGraph callgraph, long start, long end, IProgressMonitor monitor) {
@@ -215,6 +214,9 @@ private void iterateOverElement(ICallStackElement element, IHostModel model, Cal
             }
         }
         for (ICallStackElement child : element.getChildrenElements()) {
+            if (monitor.isCanceled()) {
+                return;
+            }
             iterateOverElement(child, model, callgraph, start, end, monitor);
         }
     }
@@ -234,6 +236,9 @@ private void iterateOverLeafElement(ICallStackElement element, IHostModel model,
         // Start with the first function
         AbstractCalledFunction nextFunction = (AbstractCalledFunction) callStack.getNextFunction(callStack.getStartTime(), 1, null, model, start, end);
         while (nextFunction != null) {
+            if (monitor.isCanceled()) {
+                return;
+            }
             AggregatedCalledFunction aggregatedChild = createCallSite(CallStackSymbolFactory.createSymbol(nextFunction.getSymbol(), element, nextFunction.getStart()));
             iterateOverCallstack(element, callStack, nextFunction, 2, aggregatedChild, model, start, end, monitor);
             aggregatedChild.addFunctionCall(nextFunction);
@@ -256,6 +261,9 @@ private void iterateOverCallstack(ICallStackElement element, CallStack callstack
 
         AbstractCalledFunction nextFunction = (AbstractCalledFunction) callstack.getNextFunction(function.getStart(), nextLevel, function, model, Math.max(function.getStart(), start), Math.min(function.getEnd(), end));
         while (nextFunction != null) {
+            if (monitor.isCanceled()) {
+                return;
+            }
             // Add sampling data of the time between next function and beginning
             // of next level
             if (threadId > 0) {