Community
Participate
Working Groups
I sampled the eclipse startup and found the main hotspot on my windows computer is: org.eclipse.swt.internal.win32.OS.SetWindowPos(long, long, int, int, int, int, int) org.eclipse.swt.widgets.ToolBar.computeSizeInPixels(int, int, boolean) It feels so wrong that a compute() actually sets the windows size (and resets it afterwards). Probably the callers did not expect that either and call it a thousand times during eclipse startup. May be there is a way to: a) compute the size without actually resizing b) cache the result c) do not call it that often I do not plan a contribution about this myself and only wanted to inform you about my analysis.
Created attachment 286725 [details] Screenshot Eclipse Startup Native calls.png
In my test with Eclipse self hosted mode: - Noticed ToolBar#computeSizeInPixels() method is called 667 times which is quite a big number and in this method SetWindowPos native call is made twice for Horizontal ToolBar layout(which is the case mostly with Eclipse) so roughly the total number of calls to SetWindowPos is 1000+ for every Eclipse Launch. Tried out a potential patch which caches the computeSizeInPixels() result, at specific hWidth, hHeight & count entries in the toolbar. - So every time a new entry is made made to a toolbar the cache gets refreshed. With this approach the ToolBar#computeSizeInPixels() method is called 48 times so SetWindowPos native call is less than 100 in my case. Will share a gerrit shortly.
(In reply to Niraj Modi from comment #2) > In my test with Eclipse self hosted mode: > - Noticed ToolBar#computeSizeInPixels() method is called 667 times which is > quite a big number and in this method SetWindowPos native call is made twice > for Horizontal ToolBar layout(which is the case mostly with Eclipse) so > roughly the total number of calls to SetWindowPos is 1000+ for every Eclipse > Launch. > > Tried out a potential patch which caches the computeSizeInPixels() result, > at specific hWidth, hHeight & count entries in the toolbar. > - So every time a new entry is made made to a toolbar the cache gets > refreshed. > > With this approach the ToolBar#computeSizeInPixels() method is called 48 > times so SetWindowPos native call is less than 100 in my case. I mean to say out of 667 calls, in 48 time the complete calculation will be done, rest will return the caches values.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/183361
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/183361 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=bbec86cfb4a3ffdab8e7b4c095580128113dbd20
Resolving now.
Thanks, Jörg for the report and Niraj for the fix. Looking forward to see, if that makes a real difference in startup time.
(In reply to Lars Vogel from comment #7) > Thanks, Jörg for the report and Niraj for the fix. Looking forward to see, > if that makes a real difference in startup time. Using the -debug flag on my windows machine does not result in a significant difference. Jörg or Niraj, do you see a big impact with this change?
(In reply to Lars Vogel from comment #8) > (In reply to Lars Vogel from comment #7) > > Thanks, Jörg for the report and Niraj for the fix. Looking forward to see, > > if that makes a real difference in startup time. > > Using the -debug flag on my windows machine does not result in a significant > difference. Jörg or Niraj, do you see a big impact with this change? Only ~70 of ~1000 calls at start are not cached now. Also when i hover a Tab of a Tabfolder the cached result is used. Or when i resize the eclipse window almost only cached values are used. Also i see in the native samplings of eclipse start (I20210730) ToolBar.computeSizeInPixels far less. Looks like "good job" to me. I have no numbers how time was saved.
Created attachment 286876 [details] Screenshot Eclipse Startup Native calls I20210703.png Added Screenshot of Sampling after change. Sampling taken on an empty workspace with eclipse.exe -vmargs -XX:+FlightRecorder -XX:StartFlightRecording=dumponexit=true,filename=myrecording.jfr,settings="C:\java\JDKmc_Data\Sampling2ms.jfc" -XX:FlightRecorderOptions=stackdepth=1000
(In reply to Jörg Kubitz from comment #9) > (In reply to Lars Vogel from comment #8) > > (In reply to Lars Vogel from comment #7) > > > Thanks, Jörg for the report and Niraj for the fix. Looking forward to see, > > > if that makes a real difference in startup time. > > > > Using the -debug flag on my windows machine does not result in a significant > > difference. Jörg or Niraj, do you see a big impact with this change? > > Only ~70 of ~1000 calls at start are not cached now. Also when i hover a Tab > of a Tabfolder the cached result is used. Or when i resize the eclipse > window almost only cached values are used. Also i see in the native > samplings of eclipse start (I20210730) ToolBar.computeSizeInPixels far less. > Looks like "good job" to me. > I have no numbers how time was saved. Thanks, Jörg. Eclipse "feels" much snappier with this change under Window. A maximize window used to be sluggish and feels faster with the latest I-build.
This causes a serve regression for us (and for everyone not using a 'fixed' size toolbar) as the toolbar is no longer resizing if its *content* changed (in contrast to the item count). The problem is, that if the layout manager (e.g. Gridlayout) calls Control#computeSize with the usual -1,-1 hints and the count of course has not changed it always returns the previous computed values. This then leads to the problem that items "magically" disappear if e.g. the Text of an action or the size of a tool control changes under windows and the user is unable to recover from this state.
If I clear the count field (setting it to -1) the Toolbar resizes but under Win 7 menus are then broken (clicking on the arrow simply does nothing).
Maybe computeSizeInPixels should simply take the "changed" flag into consideration? It seems LayoutMangers consider the changed=true as a way to tell the control the want to flush any saved state and get a fresh size.
(In reply to Christoph Laeubrich from comment #12) > no longer resizing if its *content* changed > (in contrast to the item count). Please explain which method you call to change the content. The current implementations calls ToolBar.clearSizeCache() whenever for example ToolItem.setImage() is called. Probably that is missing somewhere.
Thanks for the hint, sadly this was just added after the 2021-09 release :-\ I haven't checked with the current ibuilds, but still think that honor the changed flag would be a good to consider...
> I haven't checked with the current ibuilds, Please create a new bug if you can reproduce with newest builds.
(In reply to Jörg Kubitz from comment #15) > (In reply to Christoph Laeubrich from comment #12) > > no longer resizing if its *content* changed > > (in contrast to the item count). > > Please explain which method you call to change the content. The current > implementations calls ToolBar.clearSizeCache() whenever for example > ToolItem.setImage() is called. Probably that is missing somewhere. I have checked the code an think a layout(changed,all) might be a better place to clear the cache?
I have created Bug 577140 for latest ibuild.
