Hi Geneviève,
Apologies for not thanking you earlier, your answer has been on point and very helpful to me.
Just to give some additional context before diving in further, the reason I wanted a selection-based pie chart was to
be able to display timing-based data related to the selection.
More precisely, I wanted to be able to display time spent in various states for multiple actors in a system. Since these
states can be somewhat layered (Stopped can mean idle, interrupted, preempted, etc.), the hierarchical tree model was a perfect fit for my needs. With an interactive pie chart to easily break down that information, and I couldn’t see how the Weighted Tree/Pie
Chart Viewer could be better suited.
I managed to make a working prototype of the behavior I have been looking for, that is having a WeightedTreeSelView which
displays the a tree/pie chart based on the current selection, and not on the whole trace.
However I encountered some issues which I don’t know how to address without having to refactor some classes above, and
I’d be grateful to have your input on them.
I leveraged the existing getSelection(start,end) method of the IWeightedTreeProvider interface. I made a module implementing
that interface, with an implementation of getSelection() and getTreeSet() (which is just a specific case of getSelection with the whole trace selected).
After enabling the correct use of getSelection() in the WeightedTreeViewer’s updateElements() method, I observed that
the getTree() method of IWeightedTreeProvider is still being uses at multiple locations, outside of the updateElements() method. That is to be expected, if that was the only way of accessing the data from the provider, which used to be static. This leads to
some issues where the wrong data (from getTreeSet) overwrites the data from getSelection in some funny ways. Currently, in order to achieve correct behavior (for me), I had to force the use of getSelection, which breaks the use of getTreeSet, and comment out
some UI refreshes which didn’t take the selection into account.
Obviously this is not wanted, but it allowed me to work on my provider.
To proceed, I need to find a way to fix my issues with the existing getTree() calls.
The goal is to allow both use cases to coexist, that is:
-
Have a viewer that ONLY show the global data, and doesn’t update on selection. I don’t want to break anything
existing.
-
Have a viewer that (only?) shows the selection when there is a selection.
I guess ideally this logic would be in the viewer, which would know how to handle the selection signal, and refresh everything
as required. But other classes may not necessarily be aware of the selection/non-selection preference of the viewer, thus not knowing whether to call getTreeSet() vs getSelection().
Should I:
1)
Remove getSelection(), and use a getTreeSet() method that checks the current selection in the trace context?
This removes the possibility to have both a global and selection-based data in one provider, and required two providers if the need for both types of data arises again.
2)
Go over all invocations of getTreeSet(), and add a more generic getData() method instead, which chooses between
getTreeSet() or getSelection(). (or keep the getTreeSet name for this purpose)
How would I go about knowing which data to return ? Querying the provider kind of goes back to solution 1).
I apologize for the rambly e-mail, let me know how If there is anything that needs clarifying.
Once again I hope that there is an easy solution that you’ll help me see :)
Denis
PS. I attempted to push some code to Gerrit to simplify the discussion, but it seems that I need some approval to be able
to do so? My Eclipse account is created, but I can’t access the Gerrit web-UI. Is there some step I’m missing? Do I need an approval from one of the Trace Compass developers to do so?
From: tracecompass-dev <tracecompass-dev-bounces@xxxxxxxxxxx>
On Behalf Of Geneviève Bastien
Sent: mardi 1 juin 2021 21:50
To: tracecompass-dev@xxxxxxxxxxx
Subject: [EXTERNAL] Re: [tracecompass-dev] Selection-based WeightedTreeView
[EXTERNAL EMAIL]
Hi Denis,
Thank you for your interest in Trace Compass. It's great to hear about your needs in the WeightedTreeView. That was implemented many years ago and not really revisited since... Any ideas or contributions for improvements are more than
welcome!
I'll add some comments inline below:
On 2021-05-31 10:31 a.m., tracecompass developer discussions via tracecompass-dev wrote:
I however have two issues:
1) WeightedTreeView as it is provides a global pie chart presenting data from the whole trace. I need to have it to trigger a refresh on a range selection.
I was thinking of making a new WeightedTreeViewer class, and overriding the updateElements method. But doing this doesn't really seem possible without also modifying the WeightedTreeView class, and maybe a few others.
Is there an easier way to achieve this that I don't see? The way things look, I see two options:
- Modify WeightedTreeView/Viewer (and classes using them) to accept a template, so behaviour can be overridden.
- Implement my own view, potentially reusing code from the existing classes.
When we implemented the WeightedTreePieChartViewer, we took the TmfPieChartViewer as a base, which uses a statistics model. At the time (and still), it was too much work to make one single view that takes a common model, so we copied it
to display a weighted tree. But the original statistics pie chart had 2 pies, a global one and one for the selected time range. I recall starting with pies too, hoping to see a selection one, but it turned out confusing mentally to process/understand the
results for the user, so we just kept the global one.
But if you look at the flame graphs, we have 2 views, one of which is global and the other for a selected range. The second one inherits from the first and just adds a listener to the TmfSelectionRangeUpdatedSignal to rebuild the view
using the graph from a time range instead of entire trace.
That may be another option, to add a WeightedTreeViewSel for a selection that inherits the one that is there now. You will probably need to add a method, similar to the FlameGraphView's '#buildFlameGraph' to get the weighted tree for the
selected. The IWeightedTreeProvider already has a method to getSelection(start, end), so that should be fairly easy-ish to refactor and that could definitely be a great contribution to the current feature.
2) When implementing a IWeightedTreeProvider so it can be used by the WeightedTreeView, the CallStackAanalysisListener adds the Callstack/FlameGraph views which are not really interesting to my use case. I've tried to see if I could de-register
an output to an analysis, however it doesn't look like this API exists.
Here, except renaming the interface to avoid the automatic output registration, I don't really see how I could do this.
Indeed, the Flame Graphs are automatically added to all IWeightedTreeProvider. Both views show the same data, with flame graphs giving a nice quick visual representation of the whole trees, for entire trace and selection. But if really
you don't want those views, your analysis module could simply override the registerOutput method and not call super.registerOutput for the outputs that you don't want!
Both of these issues says to me that maybe the handy-dandy WeightedTreeView is not completely suited to my use case, and maybe there is another path for me to take.
What advice could those familiar with those interfaces give me?
Without more details on your actual use case, I can't judge if the WeightedTreeView is suited or not for it. I hope the hints I'm giving you will help you figure it out! Don't hesitate to ask further questions or comment on the issue!
:D
Cheers,
Geneviève
