Skip to content

Commit

Permalink
Fix resolution of callsites in differential flame graphs
Browse files Browse the repository at this point in the history 
does not merge conflicting symbols, this is step 1

Signed-off-by: Matthew Khouzam <matthew.khouzam@ericsson.com>
Change-Id: Icbdc655b69a5e2606da457b348b36107dfae1573
  • Loading branch information
@MatthewKhouzam
MatthewKhouzam committed Aug 22, 2024
1 parent a518cec commit f30c215
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 51 deletions.
28 changes: 14 additions & 14 deletions ...acecompass/incubator/internal/executioncomparison/core/DifferentialCallGraphProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,35 +31,32 @@
*/
public class DifferentialCallGraphProvider extends DifferentialWeightedTreeProvider<ICallStackSymbol> {

private final ICallGraphProvider2 fOriginalTree;

/**
* Constructor
*
* @param instrumentedCallStackAnalysis
* @param instrumentedCallStackAnalyses
* the original tree
* @param trees
* the other trees to compare to
*/
public DifferentialCallGraphProvider(IWeightedTreeProvider<ICallStackSymbol, ICallStackElement, AggregatedCallSite> instrumentedCallStackAnalysis,
public DifferentialCallGraphProvider(Collection<ICallGraphProvider2> instrumentedCallStackAnalyses,
Collection<DifferentialWeightedTree<ICallStackSymbol>> trees) {
this(instrumentedCallStackAnalysis, DifferentialWeightedTreeSet.<ICallStackSymbol> create(trees));
this(instrumentedCallStackAnalyses, DifferentialWeightedTreeSet.<ICallStackSymbol> 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<ICallStackSymbol, ICallStackElement, ? extends WeightedTree<ICallStackSymbol>> originalTree,
Collection<ICallGraphProvider2> instrumentedCallStackAnalyses,
DifferentialWeightedTreeSet<ICallStackSymbol> treeSet) {
super((IWeightedTreeProvider<ICallStackSymbol, ICallStackElement, WeightedTree<ICallStackSymbol>>) originalTree, treeSet);
fOriginalTree = (ICallGraphProvider2) originalTree;
super(instrumentedCallStackAnalyses, treeSet);
}

@Override
Expand All @@ -75,7 +70,12 @@ public String toDisplayString(DifferentialWeightedTree<ICallStackSymbol> tree) {
WeightedTree<ICallStackSymbol> 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<>()));
}
Expand Down
10 changes: 4 additions & 6 deletions ...compass/incubator/internal/executioncomparison/core/DifferentialSeqCallGraphAnalysis.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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$
}

/**
Expand All @@ -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<WeightedTree<ICallStackSymbol>> originalTree = new ArrayList<>();
Collection<WeightedTree<ICallStackSymbol>> diffTree = new ArrayList<>();
WeightedTreeSet<ICallStackSymbol, Object> callGraphA = mergeCallGraph(fStartA, fEndA, getTraceListA());
Expand All @@ -120,8 +119,8 @@ public DifferentialCallGraphProvider refreshDiffCG(@Nullable IProgressMonitor mo
Collection<DifferentialWeightedTree<ICallStackSymbol>> trees;
trees = WeightedTreeUtils.diffTrees(originalTree, diffTree, fStatistic);

IWeightedTreeProvider<ICallStackSymbol, ICallStackElement, AggregatedCallSite> instrumentedCallStackAnalysis = Iterables.get(fTraceCallGraphRegistry.values(), 0);
fDifferentialCallGraphProvider = new DifferentialCallGraphProvider(instrumentedCallStackAnalysis, trees);
Collection<ICallGraphProvider2> instrumentedCallStackAnalyses = fTraceCallGraphRegistry.values();
fDifferentialCallGraphProvider = new DifferentialCallGraphProvider(instrumentedCallStackAnalyses, trees);
return fDifferentialCallGraphProvider;
}
}
Expand Down Expand Up @@ -220,7 +219,6 @@ private static List<CallGraph> addToCallGraph(ITmfTimestamp start, ITmfTimestamp
}
}
}

refreshDiffCG(monitor);
return fDifferentialCallGraphProvider;

Expand Down
45 changes: 32 additions & 13 deletions ...ecompass/incubator/analysis/core/weighted/tree/diff/DifferentialWeightedTreeProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -76,34 +77,39 @@ public class DifferentialWeightedTreeProvider<@NonNull N> implements IWeightedTr

private final IWeightedTreeSet<N, Object, DifferentialWeightedTree<N>> fTreeSet;

private final IWeightedTreeProvider<N, ?, WeightedTree<N>> fOriginalTree;
private final List<MetricType> fAdditionalMetrics = new ArrayList<>(WEIGHT_TYPES);
private @Nullable IDataPalette fPalette = null;

protected final Collection<ICallGraphProvider2> 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<N, ?, WeightedTree<N>> originalTree, Collection<DifferentialWeightedTree<N>> trees) {
public DifferentialWeightedTreeProvider(Collection<ICallGraphProvider2> originalTree, Collection<DifferentialWeightedTree<N>> trees) {
this(originalTree, DifferentialWeightedTreeSet.create(trees));
}

/**
* Constructor
*
* @param originalTree
* The original tree provider, used to get information for texts and metrics.
* The original tree provider, used to get information for texts
* and metrics.
* @param treeSet
* The differential tree set
*/
public DifferentialWeightedTreeProvider(IWeightedTreeProvider<N, ?, WeightedTree<N>> originalTree, DifferentialWeightedTreeSet<N> treeSet) {
fOriginalTree = originalTree;
public DifferentialWeightedTreeProvider(Collection<ICallGraphProvider2> originalTrees, DifferentialWeightedTreeSet<N> treeSet) {
fOriginalTrees = originalTrees;
fTreeSet = treeSet;
fAdditionalMetrics.addAll(fOriginalTree.getAdditionalMetrics());
for (ICallGraphProvider2 tree : fOriginalTrees) {
fAdditionalMetrics.addAll(tree.getAdditionalMetrics());
}
}

/**
Expand Down Expand Up @@ -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<N> 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
Expand All @@ -156,7 +172,10 @@ public Object getAdditionalMetric(DifferentialWeightedTree<N> 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
Expand Down
57 changes: 39 additions & 18 deletions ...race.core/src/org/eclipse/tracecompass/incubator/internal/uftrace/core/trace/Uftrace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -222,10 +222,10 @@ public void initTrace(IResource resource, String path, Class<? extends ITmfEvent
} else if (name.endsWith(".map")) { //$NON-NLS-1$
MapParser create = MapParser.create(child);
if (create != null) {
fMap.put(create.getSessionId(), create);
getMap().put(create.getSessionId(), create);
}
} else if (name.endsWith(".sym")) { //$NON-NLS-1$
fSyms.put(name.substring(0, name.length() - 4), SymParser.parse(child));
getSyms().put(name.substring(0, name.length() - 4), SymParser.parse(child));
} else if (name.equals("task.txt")) { //$NON-NLS-1$
fTasks = new TaskParser(child);
} else if (name.equals("info")) { //$NON-NLS-1$
Expand Down Expand Up @@ -372,7 +372,7 @@ public final class PidAspect extends LinuxPidAspect {
if (event.getContent().getValue() instanceof DatEvent) {
DatEvent datEvent = (DatEvent) event.getContent().getValue();
int tid = datEvent.getTid();
return fTasks.getPid(tid);
return getTasks().getPid(tid);
}
return null;
}
Expand All @@ -390,7 +390,7 @@ public final class ExecAspect extends ProcessNameAspect {
if (event.getContent().getValue() instanceof DatEvent) {
DatEvent datEvent = (DatEvent) event.getContent().getValue();
int tid = datEvent.getTid();
return fTasks.getExecName(tid);
return getTasks().getExecName(tid);
}
return null;
}
Expand Down Expand Up @@ -468,37 +468,58 @@ public TaskParser getTasks() {
return fTidAspect;
}

/**
* @return the syms
*/
private Map<String, SymParser> getSyms() {
return fSyms;
}

/**
* @return the map
*/
private Map<Long, MapParser> 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<Long, MapEntry> 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<Long, Symbol> floorEntry = sym.getMap().floorEntry(offset);
if (floorEntry != null) {
Expand All @@ -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
Expand All @@ -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);
}

}
Expand Down

0 comments on commit f30c215

Please sign in to comment.