Community
Participate
Working Groups
I have absolutely no idea as to which component I should raise this bug against, took the general "platform ui" instead. We have an (GMF-based) application making heavy use of the tabbed properties view in order to display something more appealing than the classical table for our elements. All goes well except ... it can become really expensive to refresh (when the user selects a new element or when anything changes in the diagram) when text is shown. A profiler (Yourkit) shows that most (20%!) of the update time is spent in org.eclipse.ui.internal.views.properties.tabbed.view.TabbedPropertyList.getTextDimension(String) . private Point getTextDimension(String text) { Shell shell = new Shell(); GC gc = new GC(shell); gc.setFont(JFaceResources.getFontRegistry().getBold( JFaceResources.DEFAULT_FONT)); Point point = gc.textExtent(text); point.x++; gc.dispose(); shell.dispose(); return point; } Why? Because of the very first line of this method : the SWT Shell is really time consumng to instantiate ... especially when it is done so many times. Since this Shell's one and only purpose is to instantiate a GC, why didn't you use something like "new GC(Display.getDefault())" or if you really want to use a Shell, "new GC(Display.getDefault().getActiveShell())" ? That would save us all the initialisation time for a rough 18% performance gain (as measured on our application).
Since the TabbedPropertyList is itself a subclass of Composite, 'this' could probably just be used.
Created attachment 122556 [details] patch for the 3.4 branch Hi, Please find attached a patch for 3.4 maintenance branch. I ran a simple performance test with YourKit, before and after the patch, using the following code : public Object execute(ExecutionEvent event) throws ExecutionException { IWorkbenchWindow window = HandlerUtil.getActiveWorkbenchWindowChecked(event); TabbedPropertyList list = new TabbedPropertyList(window.getShell(),null); for (int i = 0; i < 10000; i++) { list.getTextDimension("eclipseworld"); } return null; } (I needed to comment TabbedPopertyList#initColours() call and change TabbedPropertyList#getTextDimension(String s) visibility) Below the Yourkit time results for getTextDimension method : before patch -> 4756 ms after patch -> 360 ms My test configuration was Windows XP, Eclipse 3.4.1, Java 1.5.0.06.
Created attachment 122557 [details] Before patch time results
Created attachment 122558 [details] After patch time results
Cool find. You are requesting this for Galileo SR2 (Eclipse 3.4.2)?
Created attachment 122655 [details] patch for head Yes if it's possible. I added a patch for head. Thanks.
Request PMC approval to commit this minor performance fix to 3.4.2.
Anthony, at this stage PMC requests for 3.4.2 have to be sent to the eclipse-pmc@eclipse.org mailing list (to make them more visible). At the technical level, the patch looks good and the performance gain looks like this fix should be put into 3.4.2 - we just need to follow the process.
Are we certain that this can never be called when the widget is disposed?
(In reply to comment #9) > Are we certain that this can never be called when the widget is disposed? > Correct, this is just a private method to figure out the size of the text and is only used by the TabbedPropertyList.
Committed the patch to R3_4_maintenance and HEAD. IPLog the 3.4 patch and merged to HEAD. Note that I corrected the date in the patch (see http://www.eclipse.org/legal/copyrightandlicensenotice.php )
Nice fix on this one! I just ran into the same issue and discovered the same problem with a profiler. After retrieving source from CVS I discovered it was already fixed... thanks!
Verified by source inspection that the fix is in M20090211-1700.
