Community
Participate
Working Groups
Since some builds, the step over action in the debugger is slow on GTK (see bug 25516). Other actions (step in/out) work fine, step over works fine on motif and win32. I did some profiling with OptimizeIt. The result is, when we refresh the debug (launch) view after the step, one call to Tree.setSelection() takes > 1 second (see the screenshots below). For my test, I use the following compilation unit : public class Test { public static void main(String[] args) { new Test().recursif(20); } void recursif(int i) { if (i == 0) { String s= null; s= null; // <-- breakpoint here s= null; System.out.println(s); } else { recursif(i - 1); } } }
Created attachment 2687 [details] optimizeit1.png profiling result for a step over
Created attachment 2688 [details] optimizeit2.png - profiling for a step in
Chrix to investigate and advise Adding Silenio to the CC list Need to check: - nbr of calls to Tree.setSelection (e.g. by setting OptimizeIt in 'instrumentation' mode) - Characteristics of the Tree, selection, and impact on performance
Luc: Tested on GTK with I20021204 and on Windows with I20021127 On Step Over in the example given above, Tree.setSelection is called 27 times. The first 25 times seem to be exactly identical (same tree, same selection). Step Into/Step Return don't have that problem. If setting the level of recursion to 30, SWT gets 35 identical calls. You can check this by either - importing org.eclipse.swt to your project and adding a println to Tree.setSelection - run OptimizeIt and select 'instrumentation' instead of the default 'sampling' (ask for more details on this if needed) Can you confirm that this is happening? Are all these Tree.setSelection required? Or the symptom of a problem somewhere else? I attach a log of the stack traces next. Thanks Chris
Created attachment 2737 [details] Tree.setSelection called 27 times, 25 times on same tree with same parameters
We're calling StructuredViewer.refresh(Object). Why is this causing the selection to be set? The API on this method doesn't say anything about setting the selection.
Nick here. The viewer refresh tries to preserve the selection across the refresh. It gets the selection before doing the refresh work, then sets it back after. This is to ensure that the user's selection is unaffected (or as minimally affected as possible) by the refresh. For example, if a refresh causes a reordering of elements, you don't want your selection to be lost. Looking at the stack traces captured above, it looks like refresh is being called many more times than needed. A comment in the code suggests that Debug is calling this to force population of the items so that setSelection works. However, AbstractTreeViewer.setSelection already does this for you (the call to internalExpand). This relies on ITreeContentProvider.getParent actually returning a parent element though, which may be problematic in your case. Why can't you just call setSelection(element, true), or expandToLevel(element, 1)?
I'm looking at making the code on our end do less. We were trying to be clever and only refresh the children of a tree node instead of just refreshing the node itself. I've found that by gutting our code of all wit, stepping is much faster. :)
I'd like to understand why you need to do the refresh at all. Refreshes are expensive, and I don't think it should be necessary just to select and reveal an element.
In our case, we're responding to suspend events after a Step in the debugger. We need to refresh the thread to update the stack frames. When the number of frames changes (StepIn, StepReturn) we need to add/remove frames. When the number of frames stays the same (StepOver), we need to update the labels of the frames (at least the top frame and the thread label).
Note that if you only want to update labels, and not the structure, you should use the update methods on the individual elements rather than refresh on the parent. Update does not recurse.
Thanks for the info. Regarding the bug report, it turns out that calling refresh on all of the items is just as slow on Win32. We fixed our code (stepping is *much* faster now). We found that refreshing ~40 tree items made a step take about 1 second on a fast machine. Unless this is considered poor performance, this report can be closed.
As found in this PR, Tree.setSelection/StructuredViewer.refresh are not cheap and may be overkill in some situations. Cheaper alternative APIs have been found, fixing the original performance problem. Closing PR. BTW, continue to report 'slow SWT calls' you may find impacting some Eclipse use case. This helps everybody understand the chain of calls and work it out. Thanks.
Sorry, but 1 second to step on a fast machine still sounds pretty slow. Are you still using refresh? I still don't understand why a refresh is needed.
I don't think I was clear. :-) We're only calling refresh 1 time now in our code. Stepping is blazingly fast. However, the fact remains that when we did call refresh 40 times (back in the long, long ago), it took about 1 second for those 40 refreshes to occur. In CPU-time, 1 second to do something 40 times would usually be considered bad so there might still be some performance work to be done here.
I'm still not clear on why you need to do the refresh at all. If it's to show new stack frames, is there any way you could use add() and remove() instead? If it's just to update the label for the topmost one, you should just use the update method.
