From: Matthew Khouzam Date: Wed, 19 Oct 2016 13:42:05 +0000 (-0400) Subject: timing.ui: Remove dependency on trace with FlameGraphContentProvider X-Git-Url: http://git.efficios.com/?p=deliverable%2Ftracecompass.git;a=commitdiff_plain;h=c827e96ffeb7fa750da95fcce6b3d06bc3d4864a timing.ui: Remove dependency on trace with FlameGraphContentProvider This decouples the flamegraph from the notion of traces. Also removes dependencies on trace with FlameGraphView and adds synchronization to refresh so tests will not query in the middle of a draw. Change-Id: I58f072cba473a74641b8ca8c364c85bfc0ef5cee Signed-off-by: Matthew Khouzam Reviewed-on: https://git.eclipse.org/r/83598 Reviewed-by: Hudson CI Reviewed-by: Genevieve Bastien Tested-by: Genevieve Bastien --- diff --git a/analysis/org.eclipse.tracecompass.analysis.timing.ui/src/org/eclipse/tracecompass/internal/analysis/timing/ui/flamegraph/FlameGraphContentProvider.java b/analysis/org.eclipse.tracecompass.analysis.timing.ui/src/org/eclipse/tracecompass/internal/analysis/timing/ui/flamegraph/FlameGraphContentProvider.java index 02a6c70ca7..35a952d55e 100644 --- a/analysis/org.eclipse.tracecompass.analysis.timing.ui/src/org/eclipse/tracecompass/internal/analysis/timing/ui/flamegraph/FlameGraphContentProvider.java +++ b/analysis/org.eclipse.tracecompass.analysis.timing.ui/src/org/eclipse/tracecompass/internal/analysis/timing/ui/flamegraph/FlameGraphContentProvider.java @@ -13,17 +13,15 @@ import static org.eclipse.tracecompass.common.core.NonNullUtils.checkNotNull; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Collection; import java.util.Comparator; import java.util.Deque; import java.util.List; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jface.viewers.Viewer; - import org.eclipse.tracecompass.internal.analysis.timing.core.callgraph.AggregatedCalledFunction; import org.eclipse.tracecompass.internal.analysis.timing.core.callgraph.ThreadNode; -import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace; -import org.eclipse.tracecompass.tmf.core.trace.TmfTraceManager; import org.eclipse.tracecompass.tmf.ui.widgets.timegraph.ITimeGraphContentProvider; import org.eclipse.tracecompass.tmf.ui.widgets.timegraph.model.ITimeGraphEntry; import org.eclipse.tracecompass.tmf.ui.widgets.timegraph.model.TimeGraphEntry; @@ -37,11 +35,9 @@ import org.eclipse.tracecompass.tmf.ui.widgets.timegraph.model.TimeGraphEntry; public class FlameGraphContentProvider implements ITimeGraphContentProvider { private final List fFlameGraphEntries = new ArrayList<>(); - private ITmfTrace fActiveTrace; private SortOption fSortOption = SortOption.BY_NAME; private @NonNull Comparator fThreadComparator = new ThreadNameComparator(); - /** * Parse the aggregated tree created by the callGraphAnalysis and creates * the event list (functions) for each entry (depth) @@ -110,19 +106,17 @@ public class FlameGraphContentProvider implements ITimeGraphContentProvider { public ITimeGraphEntry[] getElements(Object inputElement) { fFlameGraphEntries.clear(); // Get the root of each thread - fActiveTrace = TmfTraceManager.getInstance().getActiveTrace(); - if (fActiveTrace == null) { - return new ITimeGraphEntry[0]; - } - - if (inputElement instanceof List) { - List threadNodes = (List) inputElement; + if (inputElement instanceof Collection) { + Collection threadNodes = (Collection) inputElement; for (Object object : threadNodes) { if (object instanceof ThreadNode) { buildChildrenEntries((ThreadNode) object); } } + } else { + return new ITimeGraphEntry[0]; } + // Sort the threads fFlameGraphEntries.sort(fThreadComparator); return fFlameGraphEntries.toArray(new ITimeGraphEntry[fFlameGraphEntries.size()]); @@ -178,7 +172,6 @@ public class FlameGraphContentProvider implements ITimeGraphContentProvider { // Do nothing } - /** * Get the sort option * @@ -192,7 +185,7 @@ public class FlameGraphContentProvider implements ITimeGraphContentProvider { * Set the sort option for sorting the thread entries * * @param sortOption - * the sort option to set + * the sort option to set * */ public void setSortOption(SortOption sortOption) { diff --git a/analysis/org.eclipse.tracecompass.analysis.timing.ui/src/org/eclipse/tracecompass/internal/analysis/timing/ui/flamegraph/FlameGraphView.java b/analysis/org.eclipse.tracecompass.analysis.timing.ui/src/org/eclipse/tracecompass/internal/analysis/timing/ui/flamegraph/FlameGraphView.java index 87c9773c96..309f06f25d 100644 --- a/analysis/org.eclipse.tracecompass.analysis.timing.ui/src/org/eclipse/tracecompass/internal/analysis/timing/ui/flamegraph/FlameGraphView.java +++ b/analysis/org.eclipse.tracecompass.analysis.timing.ui/src/org/eclipse/tracecompass/internal/analysis/timing/ui/flamegraph/FlameGraphView.java @@ -11,6 +11,8 @@ *******************************************************************************/ package org.eclipse.tracecompass.internal.analysis.timing.ui.flamegraph; +import java.util.concurrent.Semaphore; + import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; @@ -43,7 +45,6 @@ import org.eclipse.tracecompass.segmentstore.core.ISegment; import org.eclipse.tracecompass.tmf.core.signal.TmfSelectionRangeUpdatedSignal; import org.eclipse.tracecompass.tmf.core.signal.TmfSignalHandler; import org.eclipse.tracecompass.tmf.core.signal.TmfTraceClosedSignal; -import org.eclipse.tracecompass.tmf.core.signal.TmfTraceOpenedSignal; import org.eclipse.tracecompass.tmf.core.signal.TmfTraceSelectedSignal; import org.eclipse.tracecompass.tmf.core.timestamp.TmfTimestamp; import org.eclipse.tracecompass.tmf.core.trace.ITmfTrace; @@ -57,6 +58,8 @@ import org.eclipse.ui.IActionBars; import org.eclipse.ui.IEditorPart; import org.eclipse.ui.IWorkbenchActionConstants; +import com.google.common.annotations.VisibleForTesting; + /** * View to display the flame graph .This uses the flameGraphNode tree generated * by CallGraphAnalysisUI. @@ -87,6 +90,11 @@ public class FlameGraphView extends TmfView { private final @NonNull MenuManager fEventMenuManager = new MenuManager(); private Action fSortByNameAction; private Action fSortByIdAction; + /** + * A plain old semaphore is used since different threads will be competing + * for the same resource. + */ + private final Semaphore fLock = new Semaphore(1); /** * Constructor @@ -135,22 +143,14 @@ public class FlameGraphView extends TmfView { }); } - private TimeGraphViewer getTimeGraphViewer() { - return fTimeGraphViewer; - } /** - * Handler for the trace opened signal + * Get the time graph viewer * - * @param signal - * The incoming signal + * @return the time graph viewer */ - @TmfSignalHandler - public void TraceOpened(TmfTraceOpenedSignal signal) { - fTrace = signal.getTrace(); - if (fTrace != null) { - CallGraphAnalysis flamegraphModule = TmfTraceUtils.getAnalysisModuleOfClass(fTrace, CallGraphAnalysis.class, CallGraphAnalysisUI.ID); - buildFlameGraph(flamegraphModule); - } + @VisibleForTesting + public TimeGraphViewer getTimeGraphViewer() { + return fTimeGraphViewer; } /** @@ -171,23 +171,50 @@ public class FlameGraphView extends TmfView { /** * Get the necessary data for the flame graph and display it * - * @param flamegraphModule + * @param callGraphAnalysis * the callGraphAnalysis */ - private void buildFlameGraph(CallGraphAnalysis callGraphAnalysis) { - fTimeGraphViewer.setInput(null); + @VisibleForTesting + public void buildFlameGraph(CallGraphAnalysis callGraphAnalysis) { + /* + * Note for synchronization: + * + * Acquire the lock at entry. then we have 4 places to release it + * + * 1- if the lock failed + * + * 2- if the data is null and we have no UI to update + * + * 3- if the request is cancelled before it gets to the display + * + * 4- on a clean execution + */ + try { + fLock.acquire(); + } catch (InterruptedException e) { + Activator.getDefault().logError(e.getMessage(), e); + fLock.release(); + } + if (callGraphAnalysis == null) { + fTimeGraphViewer.setInput(null); + fLock.release(); + return; + } + fTimeGraphViewer.setInput(callGraphAnalysis.getSegmentStore()); callGraphAnalysis.schedule(); Job j = new Job(Messages.CallGraphAnalysis_Execution) { @Override protected IStatus run(IProgressMonitor monitor) { if (monitor.isCanceled()) { + fLock.release(); return Status.CANCEL_STATUS; } callGraphAnalysis.waitForCompletion(monitor); Display.getDefault().asyncExec(() -> { fTimeGraphViewer.setInput(callGraphAnalysis.getThreadNodes()); fTimeGraphViewer.resetStartFinishTime(); + fLock.release(); }); return Status.OK_STATUS; } @@ -195,6 +222,21 @@ public class FlameGraphView extends TmfView { j.schedule(); } + /** + * Await the next refresh + * + * @throws InterruptedException + * something took too long + */ + @VisibleForTesting + public void waitForUpdate() throws InterruptedException { + /* + * wait for the semaphore to be available, then release it immediately + */ + fLock.acquire(); + fLock.release(); + } + /** * Trace is closed: clear the data structures and the view *