From 35f968d3c422cc5f7d90f31877bce07ada8b3f17 Mon Sep 17 00:00:00 2001 From: Matthew Khouzam Date: Thu, 22 Aug 2024 16:04:48 -0400 Subject: [PATCH] Fix resolution of callsites in differential flame graphs does not merge conflicting symbols, this is step 1 Signed-off-by: Matthew Khouzam Change-Id: Icbdc655b69a5e2606da457b348b36107dfae1573 --- .../core/DifferentialCallGraphProvider.java | 28 ++++----- .../DifferentialSeqCallGraphAnalysis.java | 10 ++-- .../DifferentialWeightedTreeProvider.java | 45 ++++++++++----- .../weighted/tree/diff/WeightedTreeUtils.java | 3 +- .../internal/uftrace/core/trace/Uftrace.java | 57 +++++++++++++------ 5 files changed, 91 insertions(+), 52 deletions(-) diff --git a/analyses/org.eclipse.tracecompass.incubator.executioncomparison.core/src/org/eclipse/tracecompass/incubator/internal/executioncomparison/core/DifferentialCallGraphProvider.java b/analyses/org.eclipse.tracecompass.incubator.executioncomparison.core/src/org/eclipse/tracecompass/incubator/internal/executioncomparison/core/DifferentialCallGraphProvider.java index 375357921..4e4185243 100644 --- a/analyses/org.eclipse.tracecompass.incubator.executioncomparison.core/src/org/eclipse/tracecompass/incubator/internal/executioncomparison/core/DifferentialCallGraphProvider.java +++ b/analyses/org.eclipse.tracecompass.incubator.executioncomparison.core/src/org/eclipse/tracecompass/incubator/internal/executioncomparison/core/DifferentialCallGraphProvider.java @@ -14,13 +14,11 @@ import java.util.ArrayList; import java.util.Collection; -import org.eclipse.tracecompass.analysis.profiling.core.callgraph.AggregatedCallSite; -import org.eclipse.tracecompass.analysis.profiling.core.callgraph.ICallGraphProvider2; import org.eclipse.tracecompass.analysis.profiling.core.base.ICallStackSymbol; import org.eclipse.tracecompass.analysis.profiling.core.base.IDataPalette; -import org.eclipse.tracecompass.analysis.profiling.core.tree.IWeightedTreeProvider; +import org.eclipse.tracecompass.analysis.profiling.core.callgraph.AggregatedCallSite; +import org.eclipse.tracecompass.analysis.profiling.core.callgraph.ICallGraphProvider2; import org.eclipse.tracecompass.analysis.profiling.core.tree.WeightedTree; -import org.eclipse.tracecompass.analysis.profiling.core.base.ICallStackElement; import org.eclipse.tracecompass.incubator.analysis.core.weighted.tree.diff.DifferentialWeightedTree; import org.eclipse.tracecompass.incubator.analysis.core.weighted.tree.diff.DifferentialWeightedTreeProvider; import org.eclipse.tracecompass.incubator.analysis.core.weighted.tree.diff.DifferentialWeightedTreeSet; @@ -33,35 +31,32 @@ */ public class DifferentialCallGraphProvider extends DifferentialWeightedTreeProvider { - private final ICallGraphProvider2 fOriginalTree; - /** * Constructor * - * @param instrumentedCallStackAnalysis + * @param instrumentedCallStackAnalyses * the original tree * @param trees * the other trees to compare to */ - public DifferentialCallGraphProvider(IWeightedTreeProvider instrumentedCallStackAnalysis, + public DifferentialCallGraphProvider(Collection instrumentedCallStackAnalyses, Collection> trees) { - this(instrumentedCallStackAnalysis, DifferentialWeightedTreeSet. create(trees)); + this(instrumentedCallStackAnalyses, DifferentialWeightedTreeSet. create(trees)); } /** * Constructor * - * @param originalTree + * @param instrumentedCallStackAnalyses * The original tree provider, used to get information for texts * and metrics. * @param treeSet * The differential tree set */ public DifferentialCallGraphProvider( - IWeightedTreeProvider> originalTree, + Collection instrumentedCallStackAnalyses, DifferentialWeightedTreeSet treeSet) { - super((IWeightedTreeProvider>) originalTree, treeSet); - fOriginalTree = (ICallGraphProvider2) originalTree; + super(instrumentedCallStackAnalyses, treeSet); } @Override @@ -75,7 +70,12 @@ public String toDisplayString(DifferentialWeightedTree tree) { WeightedTree originalTree = tree.getOriginalTree(); String label = ""; //$NON-NLS-1$ if (originalTree instanceof AggregatedCallSite) { - label = fOriginalTree.toDisplayString((AggregatedCallSite) originalTree); + for (ICallGraphProvider2 provider : fOriginalTrees) { + label = provider.toDisplayString((AggregatedCallSite) originalTree); + if (!label.startsWith("0x")) { //$NON-NLS-1$ + break; + } + } } else { label = String.valueOf(originalTree.getObject().resolve(new ArrayList<>())); } diff --git a/analyses/org.eclipse.tracecompass.incubator.executioncomparison.core/src/org/eclipse/tracecompass/incubator/internal/executioncomparison/core/DifferentialSeqCallGraphAnalysis.java b/analyses/org.eclipse.tracecompass.incubator.executioncomparison.core/src/org/eclipse/tracecompass/incubator/internal/executioncomparison/core/DifferentialSeqCallGraphAnalysis.java index 40e73283c..5b94651d9 100644 --- a/analyses/org.eclipse.tracecompass.incubator.executioncomparison.core/src/org/eclipse/tracecompass/incubator/internal/executioncomparison/core/DifferentialSeqCallGraphAnalysis.java +++ b/analyses/org.eclipse.tracecompass.incubator.executioncomparison.core/src/org/eclipse/tracecompass/incubator/internal/executioncomparison/core/DifferentialSeqCallGraphAnalysis.java @@ -34,7 +34,6 @@ import org.eclipse.tracecompass.analysis.profiling.core.callgraph.ICallGraphProvider2; import org.eclipse.tracecompass.analysis.profiling.core.instrumented.InstrumentedCallStackAnalysis; import org.eclipse.tracecompass.analysis.profiling.core.tree.ITree; -import org.eclipse.tracecompass.analysis.profiling.core.tree.IWeightedTreeProvider; import org.eclipse.tracecompass.analysis.profiling.core.tree.WeightedTree; import org.eclipse.tracecompass.analysis.profiling.core.tree.WeightedTreeSet; import org.eclipse.tracecompass.common.core.log.TraceCompassLog; @@ -54,8 +53,6 @@ import org.eclipse.tracecompass.tmf.core.trace.TmfTraceUtils; import org.eclipse.tracecompass.tmf.core.trace.experiment.TmfExperiment; -import com.google.common.collect.Iterables; - /** * Builds a differential call graph using the differentialWeightedTreeSet from * two sets of call graphs. @@ -92,6 +89,7 @@ public DifferentialSeqCallGraphAnalysis() { // TODO: Make a way to register tracetype->callstack IDs. fCallStackAnalysisMap.put("org.eclipse.tracecompass.incubator.traceevent.core.trace", "org.eclipse.tracecompass.incubator.traceevent.analysis.callstack"); //$NON-NLS-1$ //$NON-NLS-2$ fCallStackAnalysisMap.put("org.eclipse.linuxtools.lttng2.ust.tracetype", "org.eclipse.tracecompass.lttng2.ust.core.analysis.callstack"); //$NON-NLS-1$ //$NON-NLS-2$ + fCallStackAnalysisMap.put("org.eclipse.tracecompass.incubator.uftrace.trace", "org.eclipse.tracecompass.incubator.uftrace.analysis.callstack"); //$NON-NLS-1$ //$NON-NLS-2$ } /** @@ -103,6 +101,7 @@ public DifferentialSeqCallGraphAnalysis() { */ public DifferentialCallGraphProvider refreshDiffCG(@Nullable IProgressMonitor monitor) { try (ScopeLog sl = new ScopeLog(LOGGER, Level.CONFIG, "DifferentialSequenceCGA::refresh()")) { //$NON-NLS-1$ + Collection> originalTree = new ArrayList<>(); Collection> diffTree = new ArrayList<>(); WeightedTreeSet callGraphA = mergeCallGraph(fStartA, fEndA, getTraceListA()); @@ -120,8 +119,8 @@ public DifferentialCallGraphProvider refreshDiffCG(@Nullable IProgressMonitor mo Collection> trees; trees = WeightedTreeUtils.diffTrees(originalTree, diffTree, fStatistic); - IWeightedTreeProvider instrumentedCallStackAnalysis = Iterables.get(fTraceCallGraphRegistry.values(), 0); - fDifferentialCallGraphProvider = new DifferentialCallGraphProvider(instrumentedCallStackAnalysis, trees); + Collection instrumentedCallStackAnalyses = fTraceCallGraphRegistry.values(); + fDifferentialCallGraphProvider = new DifferentialCallGraphProvider(instrumentedCallStackAnalyses, trees); return fDifferentialCallGraphProvider; } } @@ -220,7 +219,6 @@ private static List addToCallGraph(ITmfTimestamp start, ITmfTimestamp } } } - refreshDiffCG(monitor); return fDifferentialCallGraphProvider; diff --git a/callstack/org.eclipse.tracecompass.incubator.analysis.core/src/org/eclipse/tracecompass/incubator/analysis/core/weighted/tree/diff/DifferentialWeightedTreeProvider.java b/callstack/org.eclipse.tracecompass.incubator.analysis.core/src/org/eclipse/tracecompass/incubator/analysis/core/weighted/tree/diff/DifferentialWeightedTreeProvider.java index 06dd36400..51bd3f3f1 100644 --- a/callstack/org.eclipse.tracecompass.incubator.analysis.core/src/org/eclipse/tracecompass/incubator/analysis/core/weighted/tree/diff/DifferentialWeightedTreeProvider.java +++ b/callstack/org.eclipse.tracecompass.incubator.analysis.core/src/org/eclipse/tracecompass/incubator/analysis/core/weighted/tree/diff/DifferentialWeightedTreeProvider.java @@ -23,9 +23,10 @@ import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; import org.eclipse.tracecompass.analysis.profiling.core.base.IDataPalette; +import org.eclipse.tracecompass.analysis.profiling.core.callgraph.AggregatedCallSite; +import org.eclipse.tracecompass.analysis.profiling.core.callgraph.ICallGraphProvider2; import org.eclipse.tracecompass.analysis.profiling.core.tree.IWeightedTreeProvider; import org.eclipse.tracecompass.analysis.profiling.core.tree.IWeightedTreeSet; -import org.eclipse.tracecompass.analysis.profiling.core.tree.WeightedTree; import org.eclipse.tracecompass.incubator.internal.analysis.core.weighted.tree.DifferentialPalette; /** @@ -76,19 +77,21 @@ public class DifferentialWeightedTreeProvider<@NonNull N> implements IWeightedTr private final IWeightedTreeSet> fTreeSet; - private final IWeightedTreeProvider> fOriginalTree; private final List fAdditionalMetrics = new ArrayList<>(WEIGHT_TYPES); private @Nullable IDataPalette fPalette = null; + protected final Collection fOriginalTrees; + /** * Constructor * - * @param originalTree - * The original tree provider, used to get information for texts and metrics. - * @param trees + * @param instrumentedCallStackAnalyses + * The original tree provider, used to get information for texts + * and metrics. + * @param treeSet * The differential tree */ - public DifferentialWeightedTreeProvider(IWeightedTreeProvider> originalTree, Collection> trees) { + public DifferentialWeightedTreeProvider(Collection originalTree, Collection> trees) { this(originalTree, DifferentialWeightedTreeSet.create(trees)); } @@ -96,14 +99,17 @@ public DifferentialWeightedTreeProvider(IWeightedTreeProvider> originalTree, DifferentialWeightedTreeSet treeSet) { - fOriginalTree = originalTree; + public DifferentialWeightedTreeProvider(Collection originalTrees, DifferentialWeightedTreeSet treeSet) { + fOriginalTrees = originalTrees; fTreeSet = treeSet; - fAdditionalMetrics.addAll(fOriginalTree.getAdditionalMetrics()); + for (ICallGraphProvider2 tree : fOriginalTrees) { + fAdditionalMetrics.addAll(tree.getAdditionalMetrics()); + } } /** @@ -138,12 +144,22 @@ public String getTitle() { @Override public @NonNull MetricType getWeightType() { - return fOriginalTree.getWeightType(); + for(ICallGraphProvider2 tree : fOriginalTrees) { + return tree.getWeightType(); + } + throw new IllegalStateException(); } @Override public String toDisplayString(DifferentialWeightedTree tree) { - return fOriginalTree.toDisplayString(tree.getOriginalTree()); + String returnValue = ""; + for (ICallGraphProvider2 provider: fOriginalTrees) { + returnValue = provider.toDisplayString((AggregatedCallSite) tree.getOriginalTree()); + if (!returnValue.startsWith("0x")) { + break; + } + } + return returnValue; } @Override @@ -156,7 +172,10 @@ public Object getAdditionalMetric(DifferentialWeightedTree object, int metric if (metricIndex == 0) { return object.getDifference(); } - return fOriginalTree.getAdditionalMetric(object.getOriginalTree(), metricIndex - 1); + for (ICallGraphProvider2 tree : fOriginalTrees) { + return tree.getAdditionalMetric((AggregatedCallSite) object.getOriginalTree(), metricIndex - 1); + } + throw new IllegalStateException(); } @Override diff --git a/callstack/org.eclipse.tracecompass.incubator.analysis.core/src/org/eclipse/tracecompass/incubator/analysis/core/weighted/tree/diff/WeightedTreeUtils.java b/callstack/org.eclipse.tracecompass.incubator.analysis.core/src/org/eclipse/tracecompass/incubator/analysis/core/weighted/tree/diff/WeightedTreeUtils.java index 52819dd86..be4ac8ad2 100644 --- a/callstack/org.eclipse.tracecompass.incubator.analysis.core/src/org/eclipse/tracecompass/incubator/analysis/core/weighted/tree/diff/WeightedTreeUtils.java +++ b/callstack/org.eclipse.tracecompass.incubator.analysis.core/src/org/eclipse/tracecompass/incubator/analysis/core/weighted/tree/diff/WeightedTreeUtils.java @@ -18,6 +18,7 @@ import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; +import org.eclipse.tracecompass.analysis.profiling.core.callgraph.ICallGraphProvider2; import org.eclipse.tracecompass.analysis.profiling.core.tree.ITree; import org.eclipse.tracecompass.analysis.profiling.core.tree.IWeightedTreeProvider; import org.eclipse.tracecompass.analysis.profiling.core.tree.IWeightedTreeSet; @@ -172,7 +173,7 @@ private static long[] calculateWeights(WeightedTree<@NonNull T> base, @Nulla * set, or null if the 2 treesets have no elements in * common */ - public static <@NonNull N> @Nullable DifferentialWeightedTreeProvider diffTreeSets(IWeightedTreeProvider> provider, + public static <@NonNull N> @Nullable DifferentialWeightedTreeProvider diffTreeSets(Collection provider, IWeightedTreeSet> first, IWeightedTreeSet> second) { Collection> pairedElements = pairElementsFromTrees(first, second); diff --git a/tracetypes/org.eclipse.tracecompass.incubator.uftrace.core/src/org/eclipse/tracecompass/incubator/internal/uftrace/core/trace/Uftrace.java b/tracetypes/org.eclipse.tracecompass.incubator.uftrace.core/src/org/eclipse/tracecompass/incubator/internal/uftrace/core/trace/Uftrace.java index 02acecce2..83b594e12 100644 --- a/tracetypes/org.eclipse.tracecompass.incubator.uftrace.core/src/org/eclipse/tracecompass/incubator/internal/uftrace/core/trace/Uftrace.java +++ b/tracetypes/org.eclipse.tracecompass.incubator.uftrace.core/src/org/eclipse/tracecompass/incubator/internal/uftrace/core/trace/Uftrace.java @@ -87,7 +87,7 @@ public class Uftrace extends TmfTrace implements ITmfPropertiesProvider, private long fSize; - private final ISymbolProvider fSymbolProvider = new UfTraceSymbolProvider(); + private final ISymbolProvider fSymbolProvider = new UfTraceSymbolProvider(this); private final @NonNull TidAspect fTidAspect = new TidAspect(); private final @NonNull PidAspect fPidAspect = new PidAspect(); @@ -222,10 +222,10 @@ public void initTrace(IResource resource, String path, Class getSyms() { + return fSyms; + } + + /** + * @return the map + */ + private Map getMap() { + return fMap; + } + /** * overly complicated, should clean up * * @author Matthew Khouzam * */ - private class UfTraceSymbolProvider implements ISymbolProvider { + private static class UfTraceSymbolProvider implements ISymbolProvider { + + private Uftrace fTrace; + + public UfTraceSymbolProvider(Uftrace trace) { + fTrace = trace; + } + @Override public TmfResolvedSymbol getSymbol(int tid, long timestamp, long address) { - String execName = fTasks.getExecName(tid); + String execName = fTrace.getTasks().getExecName(tid); if (execName == null) { - return new TmfResolvedSymbol(address, "0x" + Long.toHexString(address)); //$NON-NLS-1$ + return null; } - Long session = fTasks.getSessName(tid); + Long session = fTrace.getTasks().getSessName(tid); if (session == null) { - return new TmfResolvedSymbol(address, "0x" + Long.toHexString(address)); //$NON-NLS-1$ + return null; } - MapParser mapParser = fMap.get(session); + MapParser mapParser = fTrace.getMap().get(session); if (mapParser == null) { - return new TmfResolvedSymbol(address, "0x" + Long.toHexString(address)); //$NON-NLS-1$ + return null; } Entry key = mapParser.getData().floorEntry(address); if (key == null) { - return new TmfResolvedSymbol(address, "0x" + Long.toHexString(address)); //$NON-NLS-1$ + return null; } long offset = address - key.getValue().getAddrLow(); String pathName = key.getValue().getPathName(); String substring = pathName.substring(pathName.lastIndexOf(File.separator) + 1); - SymParser sym = fSyms.get(substring); + SymParser sym = fTrace.getSyms().get(substring); if (sym == null) { - return new TmfResolvedSymbol(address, pathName + ":0x" + Long.toHexString(address)); //$NON-NLS-1$ + return null; } Entry floorEntry = sym.getMap().floorEntry(offset); if (floorEntry != null) { @@ -508,13 +529,13 @@ public TmfResolvedSymbol getSymbol(int tid, long timestamp, long address) { return new TmfResolvedSymbol(address, name); } } - return new TmfResolvedSymbol(address, "0x" + Long.toHexString(address)); //$NON-NLS-1$ + return null; } /* needed for ISymbolProvider */ @Override public @NonNull ITmfTrace getTrace() { - return (ITmfTrace) this; + return fTrace; } @Override @@ -524,7 +545,7 @@ public void loadConfiguration(@Nullable IProgressMonitor monitor) { @Override public @Nullable TmfResolvedSymbol getSymbol(long address) { - return getSymbol(Iterables.getFirst(fTasks.getTids(), -1), 0, address); + return getSymbol(Iterables.getFirst(fTrace.getTasks().getTids(), -1), 0, address); } }