Community
Participate
Working Groups
Created attachment 281200 [details] here you see a yourkit profile where 6 seconds is just there.. https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.e4.ui.css.core/src/org/eclipse/e4/ui/css/core/impl/engine/AbstractCSSEngine.java#n426 that line calls in a for loop constantly getLength() that results in very bad behavior on windows, this is my stack: Thread [main] (Suspended) LessPropertiesComposite$1(Composite)._getChildren() line: 103 LessPropertiesComposite$1(Composite).getChildren() line: 470 LessPropertiesComposite$1.getChildren() line: 159 CompositeElement.getLength() line: 37 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 428 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean) line: 358 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 429 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean) line: 358 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 429 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean) line: 358 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 429 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean) line: 358 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 429 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean) line: 358 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean, boolean) line: 429 CSSSWTEngineImpl(AbstractCSSEngine).applyStyles(Object, boolean) line: 358 ThemeEngine.applyStyles(Object, boolean) line: 543 that LessPropertiesComposite$1.getChildren() line: 159 is my code that does nothing more then a super call to see how often that is called (and the numbers are enormous) Problem is that in the end _getChildren() is constantly creating a array with all the children and for windows it calls also quite a bit of native code. here you can see that more have this problem: https://stackoverflow.com/questions/58451881/eclipse-4-ui-slow-css-engine-called-too-often-windows when i do this in my overwritten getChildren() method: public Control[] getChildren() { if (children == null) { children = super.getChildren(); } return children; } it suddenly way way more faster.. Problem is that that composite has many components (up to 400 or so) because it is a special editor with a lot of label, textfield, button combinations
Cool, Johan, could you please provide a Gerrit patch for that change? See https://www.vogella.com/tutorials/EclipsePlatformDevelopment/article.html#exericse-eclipse-user-creation-and-gerrit-server-configuration Gerrit setup usually take ~10-15 min. Lets me know if you have questions.
Johan, a issue that sounds similar is handled by https://git.eclipse.org/r/c/152619
dont think it is touched in that specific gerrit patch at least i don't see it as a change because the change is quite simple instead of doing: for (int k = 0; k < nodes.getLength(); k++) { applyStyles(nodes.item(k), applyStylesToChildNodes); } do int length = nodes.getLength(); for (int k = 0; k < length; k++) { applyStyles(nodes.item(k), applyStylesToChildNodes); } or go from length to 0, no idea if that is even better or not... We don't expect that nodes.getLength() suddenly changes right? That would be very weird..
(In reply to Johan Compagner from comment #3) > We don't expect that nodes.getLength() suddenly changes right? That would be > very weird.. I would also not expect that, please upload a Gerrit so that we can test and integrate it. Thanks for helping here Johan!
hmm i need to check a bit more Because the problem is also a bit in CompositeElement ... because if i just adjust the calls to getLength() in 3 places in AbstractCSSEngine then that is a nice thing to do but then the next line: nodes.item(k) does it again So i think the CompositeElement needs to cache the children, let me look if that is possible.
New Gerrit change created: https://git.eclipse.org/r/154336
That was a bit more tricky then first thought.. So now NodeList implementors like CompositeElement can implement a new interface IStreamingNodeList For now i let only CompositeElement do that. but i think for example CTabFolderElement could also benefit from that.
there is a problem at jenkins/hudson i guess: Caused by: java.io.IOException: No space left on device
Thanks Johan for submitting the patch to Gerrit. The build server problem will be solved soon by the EFdn staff. We'll look at your patch soon.
Gerrit change https://git.eclipse.org/r/154336 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=f56ccbf8fc4a0172b170e6a7dc2b288ddab21524
Thanks Johan for the patch. I hope we see more patches from you.
(In reply to Lars Vogel from comment #11) > Thanks Johan for the patch. I hope we see more patches from you. i still have one ;) https://bugs.eclipse.org/bugs/show_bug.cgi?id=533828 Now i constantly just patch that plugin and with mvn i inject it into the local repo over the one that is already there, so get a patched version in our product: https://github.com/Servoy/servoy-eclipse/blob/master/org.eclipse.help.ui/pom.xml#L44
(In reply to Johan Compagner from comment #12) > (In reply to Lars Vogel from comment #11) > > Thanks Johan for the patch. I hope we see more patches from you. > > i still have one ;) > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=533828 > > Now i constantly just patch that plugin and with mvn i inject it into the > local repo over the one that is already there, so get a patched version in > our product: > https://github.com/Servoy/servoy-eclipse/blob/master/org.eclipse.help.ui/pom. > xml#L44 Thanks, will merge today. Please provide more patches if you can. We love cleanup, performance and API improvement patches as well as new functionality and hopefully your product can benefit from a better platform.
