Community
Participate
Working Groups
Created attachment 241070 [details] Flight Recording displaying the CSSEngine.applyStyles problem When displaying Flight Recordings, Java Mission Control uses a subclass of FormEditor with quite a few pages added to it (36 in the standard case) (Not all of the pages are available for selection at the same time, we use a toolbar and some tricks to make only some of the pages show) In E3.8.2, switching between two such editors is quite snappy, in E4.x, including E4.4M6, it takes approx. a second. I made a Flight Recorder profiling this behaviour, attached. During the recording, I repeatedly switch between the editors. If you look in the Code/Hot Methods tab at the data from the sampling method profiler, the top three stack traces display this: org.eclipse.e4.ui.css.core.impl.sac.CSSConditionalSelectorImpl.match(Element, String) org.eclipse.e4.ui.css.core.impl.dom.ViewCSSImpl.getComputedStyle(CSSStyleSheet, Element, String) org.eclipse.e4.ui.css.core.impl.dom.ViewCSSImpl.getComputedStyle(Element, String) org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyles(Object, boolean, boolean) org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyles(Object, boolean) org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyles(Object, boolean, boolean) org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyles(Object, boolean) org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngine.applyStyles(Object, boolean, boolean) org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run() org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm, Runnable) org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(MApplicationElement, IEclipseContext) org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(MApplicationElement) org.eclipse.ui.internal.Workbench$5.run() org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm, Runnable) org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) org.eclipse.e4.ui.css.swt.dom.WidgetElement.getWidget() org.eclipse.e4.ui.css.swt.dom.WidgetElement.getCSSClass() org.eclipse.e4.ui.css.core.impl.sac.CSSClassConditionImpl.match(Element, String) org.eclipse.e4.ui.css.core.impl.sac.CSSAndConditionImpl.match(Element, String) org.eclipse.e4.ui.css.core.impl.sac.CSSConditionalSelectorImpl.match(Element, String) org.eclipse.e4.ui.css.core.impl.dom.ViewCSSImpl.getComputedStyle(CSSStyleSheet, Element, String) org.eclipse.e4.ui.css.core.impl.dom.ViewCSSImpl.getComputedStyle(Element, String) It looks to me that the CSSEngine is styling all pages every time you switch editors. Repro with JMC: Install JMC plug-ins from http://download.oracle.com/technology/products/missioncontrol/updatesites/base/5.3.0/eclipse/ Create two copies of the attached Flight Recording files, Open both copies (or any other jfr files you might have). Switch between the two editors and notice the lag.
I think we should generally not stop iteration if something has visibility=false and record that we need to reskin when made visible.
Just as a side note on which version of Eclipse is used by the JMC RCP app, possibly affecting the prio for this bug: In JMC 6.0 (unclear when it will be relased), we have rewritten the Editor, it no includes so many pages. I did a profiling of JMC 6.0 with E4.4M6, and the CSS problem is not showing. With JMC 5.4, we are Feature Complete quite soon, so we will not be able to pick up E4.4 anyway. Depending on the release date for 6.0, there might be another 5.x relase in between, which might pick up E4.4 if this issue is fixed.
My original thought here was to supply a new 'style' NO_CSS that when applied to a particular SWT composite stops the CSS engine from trying to style any of the children. This is more general than a solution aimed particularly at MPEP's...
(In reply to Eric Moffatt from comment #3) > My original thought here was to supply a new 'style' NO_CSS that when > applied to a particular SWT composite stops the CSS engine from trying to > style any of the children. This is more general than a solution aimed > particularly at MPEP's... It's not that they want no styling, it's that it only makes sense to style what's showing (if you have a StackLayout, only the top child Composite is showing but we potentially style all children of the Composite). PW
I'll also note that in this specific case, the MPEP is using a CTabFolder which offers both the ability to know which CTabItem is shown and to know when the selected CTabItem changes. PW
Addition to the note possibly affecting prio for this bug There will most likely be a JMC 5.5 version, in which we would very much like to pick up E4.4, if possible (if this bug is fixed) (We will have a look at other possible issues stopping us from this, and report any we find.)
Doing some performance profiling with YourKit, a few obvious candidates: * EModelService's findElements() is called a surprising amount. Turns out this is from WorkbenchPage#getCurrentPerspective(), called as part of the tool-visibility checks. getCurrentPerspective() calls getPerspectiveStack(), which caches the window's perspective stack but never checks the cached value. * SWT's setForeground(), setBackground(), etc. don't check if the new value is different from the old value and triggers redraws. This may not matter so much when a paint has been scheduled but not yet happened. (HotSpots on CSSPropertyBackgroundSWTHandler#applyCSSProperty and CSSPropertyTextSWTHandler#applyCSSPropertyColor) * On each applyStyles() we continuously recompute the viewCSS for the element. (HotSpot: CSSChildSelectorImpl#match)
WorkbenchPage#getPerspectiveStack() fixed with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8692c807b4dd8722a2bb62a80a8686738134bcdf
My attempts to avoid setting fonts on bug 422894 wasn't quite sufficient on OS X. I've added some logic for colours and fonts to avoid setting values unless required, and the IDE feels snappier now too: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=25f2a7d89e394996fc6bd87f3506ba6a245e1673 I've added some logic to disable the CSS theming via a new theme identifier of "none" (e.g., on the command line: "-cssTheme none"). This should allow people to more easily gauge the impact of CSS. I added some further logic so that CSS can be disabled on a per-widget basis by setting widget data "org.eclipse.e4.ui.css.disabled" = Boolean.TRUE (mistake in the identifier in the commit message). https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=995576e7879f78ad672f9776de376dba96d6fa7f
(In reply to Brian de Alwis from comment #9) > people to more easily gauge the impact of CSS. I added some further logic > so that CSS can be disabled on a per-widget basis by setting widget data > "org.eclipse.e4.ui.css.disabled" = Boolean.TRUE (mistake in the identifier > in the commit message). In the usecase for this bug, though, the problem isn't styling things that shouldn't be styled. It's that all of the pages are styled even though only one of them is visible. We wouldn't want pages in this case not to be styled forever. PW
(In reply to Paul Webster from comment #10) > In the usecase for this bug, though, the problem isn't styling things that > shouldn't be styled. It's that all of the pages are styled even though only > one of them is visible. We wouldn't want pages in this case not to be > styled forever. I doubt JMC will be the only situation where people will hit problems due to CSS. This per-widget switch will allow them to work around possible issues. I have a start at a patch to allow CSS elements to surface information about their visible children. I still have some more work to do on that, but will try to put up a patch for review today.
I've pushed up a change to Gerrit to add allow a CSS element to indicate that it has awareness of the children that are currently visible. It's currently only implemented by CTabFolderElement, which returns the currently selected CTabItem and the CTabItem's control. https://git.eclipse.org/r/23267/
I should note here: this is not the final fix as there are some test failure. But if people want to try playing and see if this helps with some of the performance issues…
Oops, the correct Gerrit patch set is: https://git.eclipse.org/r/24806/ With this latest patch, there are three test failures in CTabFolder tests where the tests assume that CSS is applied to all the CTabItem contents.
Latest patch fixes the CTabFolder tests to only check the selected CTabItem.
I've pushed up the child-visibility changes and will mark this as FIXED. Please re-open if you see other issues. https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=fe229e9094c71968f94ac0e44a5ebe796b5a5933
Verified in I20140428-2000 using the CSS Spy and CSS Scratchpad: 1. Open the Java perspective. 2. Click on the Problems and the Javadoc views to cause their contents to be realized (the hosting CTabFolder/PartStack will create a host Composite for their contents). 3. Open the CSS Scratchpad and apply the following snippet; this should only colour the hosting composite (the .MPlaceholder) for the currently visible view. CTabFolder > .MPlaceholder { background: green; } 4. Open the CSS Spy and examine the backgrounds of "CTabFolder > .MPlaceholder": only the visible .MPlaceholder in each part stack should have its background set to green (#008000).
This issue seems fixed when I look at it. Unfortunatly it did not solve the ui performance problem for Java Mission Control, 416650 seems to be the main problem still.
Please consider reopening this issue, the performance issue is not fixed and the root cause must be from https://bugs.eclipse.org/bugs/show_bug.cgi?id=422894 if some linux system is having some issue, i don't think its a good idea to change something it was working before.
(In reply to Brian de Alwis from comment #16) > I've pushed up the child-visibility changes and will mark this as FIXED. > Please re-open if you see other issues. > > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=fe229e9094c71968f94ac0e44a5ebe796b5a5933 Brian, Can you point me at some instructions or video for building and testing the changes in org.eclipse.e4.ui.css.swt project
Chidveer, these changes were committed 3 years ago. Please open a new bug if you're seeing performance issues with Photon (4.8).
