Community
Participate
Working Groups
When a cheat sheet is shown, resizing the workbench is very sluggish. The workbench redrawing is not smooth. Close the cheat sheet and resizing the workbench becomes smooth again.
Do we understand what is going on here? Is this a problem with the cheat sheet, SWT, or it just an example of a case where we should do less, even at the cost of it not looking as good?
I have seen a similar problem in a simple SWT Text widget that contains a lot of code. When resizing the parent shell, the widget is calculating so much that the contents of the shell gets redrawn maybe once per 3/4 seconds. Cheat sheets redraws faster than that, but is still not acceptable. Weird guess: Is GEF being used?
I don't think we can do something radical here. The content of the cheat sheet is a number of composites and layouts and as you resize the workbench, they are constantly resized and layouts recomputed. A dedicated control (browser) would be better in the same sense as StyledText had to be written anew to only redraw areas that are absolutely necessary when dirty. We can analyze if we can elliminate unneeded 'layout' calls but it is fairly hard. Resizing changes the width of the view. This changes the wrapping of the individual text widgets, which in turn changes the height. Changes in the height affect the ScrolledComposite widget that need to constantly recompute scroll bar visibility. If we find a way to achive the same effect with less repaints and layouts, the performance will be better.
No, GEF has not been used. The cheat sheet uses ScrolledComposite that hosts an Eclipse Form. A combination of TableWrapLayout and GridLayout is used to control placement of the cheat sheet parts. These layouts are costly to compute when called in a rapid succession.
Some UIs circumvent the problem of the need of showing/hiding scrollbars by always showing them. When they are not needed, use disabled scrollbar, but do not hide it. This makes the UI more stable, and I bet, double-bet, relayout and redraw will be much faster.
I think the disabled scrollbars would not have as nice an appearance. Q: If most of the content repositions/resizes is it better to turn off redraw first?
We can experiment and fine-tune the performance. However, just for the record, cheat sheet is actually snappier now since we ported it to Eclipse Forms than it was before. In other words, we are not talking about a regression here, rather a long-standing problem that actually got better but apparently not good enough yet.
Laffra's Law 1: Performance of any software program inevitable becomes a space-time tradeoff. -- Chris Laffra Laffra's Law 2: Performance of any software program is inevitable related to strings. -- Chris Laffra In this case, Law 2 applies. I had a quick look at Cheatsheet. Zooming in on 'strings', I ended up at ViewItem. In method addItem, I replaced this line: bodyText = new Label(bodyWrapperComposite, SWT.WRAP); with the following bodyText = new Label(bodyWrapperComposite, SWT.NONE); and redraw performance looks 3x to 4x better(needs actual measurement). Without blaming SWT too quickly, lots of time is spent in simply laying out the text in the individual view item blocks. Similar behavior happens in the Welcome Editor and in the new Eclipse Intro screen. Resizing the workbench with one of those shown is very slow also. SWT.Label should be investigated to see how a wrapped label computes the width of a string. How often do we call StringWidth?
Also note that both GridLayout and TableWrapLayout (the later being the novelty contributed by Eclipse Forms) are complex and not very fast layout algorithms that are powerful but do a lot of computing when layout out controls. Being the author of TableWrapLayout, I know there is a lot of room for improvement - it is definitely not the best and fastest as it is now (the focus was on getting it to work first). When you switch from SWT.WRAP to SWT.NULL, the label is always computed on one long line. That in turn makes the entire layout less 'jumpy' becuase the height does not change with the width. It does not help much - we need it to wrap, so this is just an academic exercise. Note that if you are looking at the new welcome on Windows, you are complaining about Internet Explorer - I doubt we can do much about it :-).
I downloaded the May 25 integration build. If you start with an empty resource workbench and add the "Hello World" cheatsheet, the following happen: method calls nested calls ----------------------------------------------------------- TableWrapLayout.layout 37 2.3 million TableWrapLayout.computeSize 127 2 million CheatSheetViewer.checkSavedState 1 1 million CheatSheetViewer.clearBackgrounds 1 0.7 million TextSegment.advanceLocator 819 2.5 million ViewItem.setBold 14 1.2 million GC.textExtent 31,884 0.4 million StringCharacterIterator.getIndex 405K String.charAt 275K String.substring 54K ----------------------------------------------------------- CheatSheet demonstrates how bad it can be to rely on third-party components. TextSegment.advanceLocator is *very* wasteful. It uses a breakIterator - an expensive way to find spaces in a string. It asks for textExtent as long the string is not wide enough, adds a word to it, and keeps trying, computing the size of the existing string over and over and over. I instrumented the calls to gc.textExtent(). Do the same, and you be shocked too. I remember doing something very similar MIDlet text layout on the Palm (you know the old ones with 16Mhz CPUs). We ended up sizing the string character by character and used a local cache to remember characters and their widths. A 1000 chars cache is not a bad space/time tradeoff here. There is something really bad going on in the layout here. It seems the text is sized over and over. When resizing the workbench a scrollbar briefly appears. Makes me suspect things are laid out twice, perhaps even more times, too many. In the current state, CheatSheets are embarrasing Eclipse 3.0. I hope performance-oriented work has been done since April 8. If a workspace contains an open CheatSheet, Eclipse startup time is *doubled*. The CheatSheet uses the same amount of resources as the entire platform needs to startup.
Please don't use priority field - it is for the component owner to prioritize defects, not for the originators (use severity for that purpose). Cheat sheet used to use StyledText. It was SLOWER when it did. The current implementation of FormText can be made faster, but it is currently functionally correct. There is life after 3.0, and cheat sheets are not used in daily Eclipse development like Java editor, for example. Starting cheat sheet on Eclipse startup is not a mainstream scenario. It only happens if you close with the cheat sheet showing. BreakIterator is important because word breaking is NL-sensitive - it is NOT as simple as locating white space in strings (I know that well enough - I worked with DB2 team on text breaking for a wizard and they shipped on every locale known to man; word breaking in Chinese is very different). I need to check your numbers. Knowing how cheat sheets are implemented, it seems very strange that showing a cheat sheet takes a much time as showing the entire Eclipse. I am almost positive that it does not take that much time to RENDER it. There must be more than meets the eye here - some chain of events where cheat sheet reaches into Eclipse and fetches data that is lengthy.
Did you enter a problem report against StyledText? Did the slow down come between 2.1.x and 3.0?
Dejan, I get the feeling that you seem to marginalize my performance complaints. Whenever a CheatSheet is involved, the entire workbench is UNACCEPTABLY slow. Resizing, restarting, switching perspectives. People will not notice that this is due to CheatSheet and will blaim Eclipse 3.0 for being slow. There... I will say it, in its current implementation, CheatSheet is a stop-ship for me.
Chris, I am just trying to pinpoint the reason. Lorne, please do the following: 1) Add the text normalization code that you had before 2) Replace FormText with a Label with SWT.WRAP style 3) Make the code switchable i.e. make it easy to toggle between Label and FormText We need to take FormText out of the equation and see if we need to focus our attention to it or there are other places in cheat sheet code that needs performance tweaking.
BTW, Chris, since you have a lot of practical suggestion, would you go into FormText and play a bit? I have no objections to people sending improvements - this is open source, after all :-).
What are your system properties? OS, platform, machine type, memory, etc.
Nice try :-) I wrote a profiler to profile your code. That should be enough :-) Starting an empty workbench with the HelloWorld CheatSheet: stopwatch time: 8328 milliseconds CPU time: 6960 milliseconds (83%) gc frequency: 13 times gc cost: 530 milliseconds (7%) disk read I/O: 10664909 bytes disk write I/O: 3539 bytes classes loaded: 3226 classes The same workspace with the CheatSheet closed: stopwatch time: 4805 milliseconds CPU time: 4466 milliseconds (92%) gc frequency: 9 times gc cost: 420 milliseconds (9%) disk read I/O: 9937283 bytes disk write I/O: 3539 bytes classes loaded: 3084 classes The 142 extra classes don't explain the double startup time. For the record, I think CheatSheets are a smarvelous idea, they look cool and they work really well. I just think they are too slow. If we can improve the performance a bit, things look rozy.
Windows XP, 1GB memory, warmed up disk cache, ThinkPad T40
Chris, please pitch in and fix/hack the code to find the problem. Then rerun your tools to verify that you are making the code better. This will help us find out what we are doing wrong and validate your tools in the process. It's a win/win situation. Thanks.
As said before, let us take FormText out of the picture first and rerun the tests. If it turns out that FormText is the bottleneck, we can focus on it. FormText is a cool custom widget that has limited tagged text processing capabilites (<b>, <img>, <span>, <p>, <br/>, <a>). However, it is written by an amateur (myself) and a seasoned SWT developer will probably pinch his/her nose while reading the code in question. I welcome all suggestions from SWT developers that know the real price of certain method calls. The interesting code starts in FormText.paint() and goes into Paragraph.paint() and finally into TextSegment.paint() Any suggestion that will make this code faster is welcome.
And by 'seasoned SWT developer that knows how to write good custom widgets', I usually mean 'Veronika', hence I am adding her to the CC list :-). Veronika, the word 'seasoned' is by no means an implication of your age, just your SWT experience :-).
After a few code fixes here are the numbers I'm getting to compare FormText to Label with SWT.WRAP: FormText | Label with SWT.WRAP ---------------------------------- 821 | 581 611 | 390 601 | 390 531 | 321 511 | 281 491 | 280 481 | 280 481 | 300 The times listed above are in milliseconds. The scenario is to start Eclipse self-hosted in debug-mode with a fresh workspace. Close the Welcome and launch the "Hello, World Application" cheat sheet from the Help menu. Close the Cheat Sheet view and repeat previous step. From what I can tell, over half the time is spent in the control.layout(); call of CheatSheetViewer.initCheatSheetView().
From what I can tell, FormText is about 50% slower than the native Label. This is not bad at all, considering that it does more and is also Java code i.e. not native. It is quite possible than TableWrapLayout (also from Eclipse Forms) that is far from optimal could benefit from some performance tune up. It is quite complex and is also written with focus on function, less on performance. Right now, the 'dropCache' flag is not used at all i.e. each time the layout is performed, grid structure is built from scratch. This is wasteful and there are opportunities for gain here if the grid structure is reused. For example, once the items are created, the grid structure remains unchanged, so it can be reused for the lifetime of the cheat sheet content.
Found this bug while scanning through the performance bugs... interesting discussion with good findings. However, the current priority (P3) is a fair assignment for this bug and I suggest that it is treated as such.
OK. I give up :-) I was just rattling the cage a bit when I changed the priority number. Sorry.
Chris, I have released some changes to HEAD. Could you check if there is any improvements on your end.
I came across major performance problem using Forms API while creating relatively complex forms. The problem is not only with the cheat sheet but with any Form containing certain number of complex controls like ExpandableComposite, Section or FormText. I partially fixed problem with ExpandableComposite. The fix is rough but it can help you to understand the reason of the problem. I tried to find appropriate open bug in eclipse’s bugzilla and found the cheat sheet redraw issue. Attached are two sample plug-in projects with the same view before and after the fix. The view contains ScrolledForm with one Section and 20 ExpandableComposite controls. It redraws very slow, 5 - 7 seconds to expand/collapse expandable composite and causes even worse problem than the cheat sheet when resizing the workbench. If you run the sample in profiler, it will show about 800 calls of ExpandableComposite$ExpandableLayout.computeSize() for each redraw or mouse click to expand/collapse a composite. But the number of ExpandableComposites in the view is only 20. Since the ExpandableLayout.computeSize() is expensive these calls take about 90% of the redraw time. This enormous number of computeSize() calls starts from ScrolledForm.reflow() that does the following: 1. calls content.getBody().layout() method that computes the body size by calling computeSize() of its children 2. content.layout() - guess does the same 3. then calls super.reflow() -> SharedScrolledComponent.reflow(): a. and again getContent().layout() method that calls computeSize() of its children b. after this it calls computeSize() directly c. then getContent().setSize() that does content re-layout again with computeSize() for all the children. d. then setMinSize() which causes widget event SWT.Resize (listener is in the SharedScrolledComposite constructor). The listener calls reflow() second time that repeats steps from 1 to 3.c … I guess there are other places where computeSize() is also called. For example, layout() method of Composite computes sizes of children by calling their computeSize(). Then resizes child controls by Control.setBounds() method and if a child is Composite then setBounds() causes its layout() call which consequently, again calls computeSize() of its children as well as computeMinimumSize() and computeMaximumSize(). I might be mistaken in some details listed above but not in general and the number of computeSize() calls is a fact. What I tried is to cache computed sizes in ExpandableLayout class. You can import both sample projects in eclipse, launch runtime workspace and compare two versions of the view - “Sample Form (Original)” and “Sample Form (Fixed)”. The same happens in the cheat sheet - computeSize() of FormText is expensive and in addition is called multiple, unnecessary times each time the form resizes or expandable composites expand/collapse. My sample fix affected only one class – org.eclipse.ui.forms.widgets.ExpandableComposite. I did not update the original class but created its copy in my sample plug-in and modified it (ModifiedExpandableComposite). I also created copy of FormToolkit class, which actually does not have any relation with the problem fix, and just added a method for ModifiedExpandableComposite creation. You can do diff between the original ExpandableComposite and the modified one to see the changes.
Created attachment 14408 [details] sample project demonstrating forms api performance problem
Created attachment 14409 [details] modified project demonstrating possible resolution of the performance problem
The patch I've attached to bug 73716 will fix this. Note: that part of the problem is due to SWT's ScrolledComposite: 1. Each time ScrolledComposite calls setVisible on its scroll bars, it fires a resize event (which can happen 2x per layout). 2. ScrolledComposite attaches a resize listener to itself and its content area... each listener will call resize(), which triggers an additional layout(true). Using layout(true) flushes all the layout caches, ensuring the worst possible cache reuse. This means that each layout can trigger up to 4 additional layouts. This would cause infinite recursion if it wasn't for the flags inside ScrolledComposite.resize().
The performance improvement is such that we can consider this defect fixed.
Verified in I20050214-0927. Redraw is very fast now. Nice work.
