Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [mdt-papyrus.dev] [Performance] Css/stereotypes element creation

Hi,

I will try to answer to questions from both performance issues threads. While the CSS Engine makes some (often unnecessary) brute-force refreshes in diagrams, this is not the main cause of performance degradation. The reason why SysML 1.4 is especially affected is because it uses sub-packages (rather than sub-profiles), and last time I checked [1], Eclipse/UML2 didn't optimize the performances for this scenario. The reason is that Sub-packages in profiles have been introduced with UML 2.4, whereas the API to retrieve a Definition from a Profile is much older (And it retrieves a Definition from a *Profile*, not from a *Package*).

So if you look at the results from a profiling tool (deep enough, i.e. deeper than what is displayed in the screenshot here), you will probably find "UMLUtil.getStereotype(EObject)". This method uses several strategies to retrieve the Profile <-> EPackage matching (Using static definitions, dynamic definitions, extension points... and ultimately, when everything else fails, iterating on the entire resource set contents)

This method doesn't expect the scenario where an Ecore EPackage corresponds to a UML Package, simply because this wasn't possible until UML 2.4 (And most profiles in Papyrus are still using nested *profiles*, so they are not affected)

Fixing this method in Eclipse/UML2 should solve your performance issues

Regarding the CSS Engine itself, creating an element may affect the style of another element. For example:

Class[ownedAttributes~=id] { /* A class containing a property named "id" will be displayed in red */
    fontColor: red;
}

If you create a new property named "ID" in this class, then the class' figure needs to be refreshed (Even if the creation happens in the semantic model)

There are some ways to optimize the performances of the CSS Engine (To reduce the cost of interpreting Matchers, and to do fine-grain refresh, without actually breaking any feature), but they require rewriting a good part of the engine. This was scheduled along with the support of CSS3, but this work has never been completed.

Anyway, improving UMLUtil to support Package Definitions (UML Packages containing Stereotypes) should be a better candidate for fixing SysML 1.4 performance issues

[1] I didn't verify whether https://bugs.eclipse.org/bugs/show_bug.cgi?id=497359 fixed or improved this issue

Regards,
Camille

2016-09-06 15:18 GMT+02:00 MAGGI Benoit <Benoit.MAGGI@xxxxxx>:

Hi,

 

It seems  that improving the perf will require to fix the css engine.

Am I the only one having this kind of performances issues?

 

So I could change the API and also change the API to inject my own css motor for SysML 1.4 that don’t refresh everything for a creation.

But I would rather have a solution in the Papyrus core css motor.

 

Is there a list of the usecases that required to refresh more than just the created element?

 

Regards,

Benoit

 

De : mdt-papyrus.dev-bounces@eclipse.org [mailto:mdt-papyrus.dev-bounces@xxxxxxxxxxx] De la part de Christian Damus
Envoyé : vendredi 29 juillet 2016 20:37
À : Papyrus Project list <mdt-papyrus.dev@xxxxxxxxxxx>
Objet : Re: [mdt-papyrus.dev] [Performance] Css/stereotypes element creation

 

Hi, Benoit,

 

The UML2 project does not use Gerrit.  You can find a patch attached to the bugzilla that I referenced.

 

Yes, it could be useful for future enhancements in refresh granularity, or for extensibility in the short term, to include the original Notification in the hook for recalculation of the styles.  It should be simple to do that in a compatible way: a new notifyChange(Element, Notification) API would just delegate to the current notifyChange(Element) API, dropping the notification.

 

cW

 

On 29 July, 2016 at 11:07:20, MAGGI Benoit (benoit.maggi@xxxxxx) wrote:

Hi Christian,

 

Thanks for your answer, I made some comment inline.

I merged small patches this morning that will improve the performance

I will try to improve the perf without changing the css engine.

 

Regards,

Benoit

 

De : mdt-papyrus.dev-bounces@eclipse.org [mailto:mdt-papyrus.dev-bounces@xxxxxxxxxxx] De la part de Christian Damus
Envoyé : mercredi 27 juillet 2016 16:21
À : Papyrus Project list <mdt-papyrus.dev@xxxxxxxxxxx>
Objet : Re: [mdt-papyrus.dev] [Performance] Css/stereotypes element creation

 

Hi, Benoit,

 

See some replies in-line, below.

 

HTH,

 

Christian

 

 

 

On 27 July, 2016 at 09:47:31, MAGGI Benoit (benoit.maggi@xxxxxx) wrote:

Hi Christian,

 

Thx for the answer. I wasn’t really going to remove everything anyway J

 

 

I made some small improvement (still in my draft) on Utility and EditPart but  the problem remains pretty much the same.

 

My performance problem is coming from the combination of our css engine and the calls to UML2 stereotype methods

-        Is there any way to cached the stereotypes application?

 

They are cached in the CacheAdapter.  If they appear not to be cached, it is because some changes are being made in the resource set that cause the cache to be cleared.  But the Element::getValue(…) operation is not cached (I doubt that it would help if it were).

In UML2 5.3, I contributed an API that provides for caching within the UMLUtil methods that look up profile definitions.  It significantly improves the performance of all of these stereotype-related APIs for statically-generated profiles (dynamic profiles don’t have nearly such performance problem).  But, this API will only be available in the UML2 Oxygen release, so you wouldn’t have it for Neon.1.  Have a look at http://eclip.se/497359 for details.

  I will look at it. Is there a gerrit for the patch?


-
        There is probably some works todo in StereotypeDisplayUtil to have less call to UML2 getstereotypeapplication (I hope to catch @Celine tomorrow to get some ideas from her)

-        The main problem is still in the css engine

o   I found a way to override the default one for SysML 1.4 (so it shouldn’t impact other DSML)

o   I can also pass the notification from StyleListener to CSSEngine handler

 

What does this mean?

Currently the notification has only elementAdapter in parameter, I believe that the GMF notification will be required to make a fine refresh. Did I missed something?

public void notifyChange(Element elementAdapter) {

 


§  Is there a way to differentiate an “open diagram” notification from  a simple “create element” (both seems to use ADD event) ?

 

I don’t understand.  Opening a diagram is not a modification of anything.  It doesn’t change the notation model.  What is this ADD event?

My bad, it’s when opening a model. I got notification in the SyleListener


o
   It should also be possible to use my own diagramHelper to customize the refresh (refresh only the created element)

§  Is there a nice way to do it?

 

Perhaps the DiagramHelper could be installed in the editor as an IService?  But as of Neon release, it isn’t an object; it’s a suite of static utilities.  Changing that would be an API-breaking change.  Although, perhaps most of the existing static API could remain as a façade in front of the new service instance, because all of the non-deprecated methods accept a parameter from which the Service Registry could be inferred (the parameterless forceRefresh and scheduleRefresh methods should be deprecated, too, IMHO; I don't know why they aren’t)

With a service, maybe your DSML could have a higher-priority service-factory that installs your helper for editors on SysML models but delegates to the default factory otherwise?  It becomes difficult when more than one DSML wants to be able to do this; we would need to define a new kind of service-factory that implements the Chain of Responsibility pattern.  Maybe the DiagramHelperService would just be a service that provides the diagram-helper and lets its clients replace the helper implementation if they need to.

(that’s enough thinking out loud for me for one day)

Too complicated/risky for a neon integration


Regards,

Benoit

 

 

De : mdt-papyrus.dev-bounces@eclipse.org [mailto:mdt-papyrus.dev-bounces@xxxxxxxxxxx] De la part de Christian Damus
Envoyé : lundi 25 juillet 2016 16:32
À : Papyrus Project list <mdt-papyrus.dev@xxxxxxxxxxx>
Objet : Re: [mdt-papyrus.dev] [Performance] Css/stereotypes element creation

 

Hi, Benoit,

 

I cannot comment on the refreshing of the applied-stereotype edit-part, but I do know some of the background about the CSS refresh.

 

You probably would notice a regression in the refresh of the diagram visuals if you have CSS stylesheets in which the presentation of an element (for example its colours, line styles, SVG shapes, etc.) depended on its relationship to other elements in the diagram and you change those relationships.  For example, if a nested class has a different fill colour to classes in a package, then in a class diagram editor dragging a class from the diagram surface into the nested classifiers compartment of another class, or vice versa, would not result in its colour changing.

 

The TODO has been there since the very beginning of CSS support because the way the cache was initially implemented makes it difficult or impossible to target the styling refresh to specific affected edit-parts; there is currently no way to determine the scope of CSS rules that are affected by any given change in the model.

 

So, please do not comment this out!  😀 

 

HTH,

 

Christian

 

On 25 July, 2016 at 10:24:31, MAGGI Benoit (benoit.maggi@xxxxxx) wrote:

Hi,

 

I found the class that is refreshing all the diagram [1] (There is a TODO suggesting to refresh only a part of the diagram)

ð  I removed this method and didn’t see obvious regressions. What is the catch?

 

I also found a class that seems to call refresh on label too many times [2]

ð  Is there a reason for calling the refresh label many times ? (In a Requirement creation, it will call refresh label 3 times for each property)

 

Regards,

Benoit

 

1:

org.eclipse.papyrus.infra.gmfdiag.css.engine.ExtendedCSSEngineImpl

       /**

       * {@inheritDoc}

       *

       * Handles a notification that an Element has changed.

       *

       * Source: GMFElementAdapter

       */

       @Override

       public void notifyChange(Element elementAdapter) {

             resetCache(); // TODO: We should only refresh a subset of the cache

 

             // FIXME: It seems the refresh can create a deadlock in some cases

 

             Diagram diagram = AdapterUtils.adapt(elementAdapter, Diagram.class, null);

             Set<? extends DiagramEditPart> diagramEditParts = PapyrusDiagramEditPart.getDiagramEditPartsFor(diagram);

             if (!diagramEditParts.isEmpty()) {

                    // TODO: Contextual refresh more specific than the diagram?

                    for (DiagramEditPart next : diagramEditParts) {

                           DiagramHelper.scheduleRefresh(next);

                    }

             } else {

                    DiagramHelper.scheduleRefresh();

             }

       }

 

 

2: org.eclipse.papyrus.uml.diagram.stereotype.edition.editpart.AppliedStereotypeMultilinePropertyEditPart

L820

       protected void handleNotificationEvent(Notification event) {

             refreshLabel();

             Object feature = event.getFeature();

             if (NotationPackage.eINSTANCE.getFontStyle_FontColor().equals(feature)) {

                    Integer c = (Integer) event.getNewValue();

                    setFontColor(DiagramColorRegistry.getInstance().getColor(c));

             } else if (NotationPackage.eINSTANCE.getFontStyle_Underline().equals(feature)) {

                    refreshUnderline();

             } else if (NotationPackage.eINSTANCE.getFontStyle_StrikeThrough().equals(feature)) {

                    refreshStrikeThrough();

             } else if (NotationPackage.eINSTANCE.getFontStyle_FontHeight().equals(feature) || NotationPackage.eINSTANCE.getFontStyle_FontName().equals(feature) || NotationPackage.eINSTANCE.getFontStyle_Bold().equals(feature)

                           || NotationPackage.eINSTANCE.getFontStyle_Italic().equals(feature)) {

                    refreshFont();

             } else {

                    if (getParser() != null && getParser().isAffectingEvent(event, getParserOptions().intValue())) {

                           refreshLabel();

                    }

                    if (getParser() instanceof ISemanticParser) {

                           ISemanticParser modelParser = (ISemanticParser) getParser();

                           if (modelParser.areSemanticElementsAffected(null, event)) {

                                 removeSemanticListeners();

                                 if (resolveSemanticElement() != null) {

                                        addSemanticListeners();

                                 }

                                 refreshLabel();

                           }

                    }

             }

 

             super.handleNotificationEvent(event);

       }

 

 

 

De : mdt-papyrus.dev-bounces@eclipse.org [mailto:mdt-papyrus.dev-bounces@eclipse.org] De la part de MAGGI Benoit
Envoyé : vendredi 22 juillet 2016 16:56
À : Papyrus Project list <mdt-papyrus.dev@xxxxxxxxxxx>
Objet : [PROVENANCE INTERNET] [mdt-papyrus.dev] [Performance] Css/stereotypes element creation

 

Hi everyone,

 

I’m having some performance issues in SysML 1.4. The basic use case is creating some SysML 1.4 stereotypes elements.

 

I used jvm monitor [1] to make a sample analyze [4]

It seems that most of the time is lost in StereotypeDisplayUtil [2] (either it’s called to often, either there is something wrong inside)

 

I looked in the code, and know I have some questions:

-        A diagram seems to be refreshed for each element creation. Is it expected (“normal”)? (seems a lot of useless refresh in most use cases)

-        Is there tests for StereotypeDisplayUtil? (I didn’t find a StereotypeDisplayUtilTest class)

-        A stereotype Compartment seems to be linked to stereotype. Why isn’t it directly linked to a stereotype application?

-        The monitor report didn’t show any sysml14 classes in the Hot spots. But I may probably have a bad configuration in SysML 1.4, any ideas/suggestions?

-        Is there a recommended way to add performance tests/benchmark in Papyrus? (for example using [3])

 

Regards,

Benoit

 

1 : http://jvmmonitor.org/

2 : org.eclipse.papyrus.uml.diagram.common.stereotype.display.helper.StereotypeDisplayUtil

3 :http://openjdk.java.net/projects/code-tools/jmh/

4 :

_______________________________________________ 
mdt-papyrus.dev mailing list 
mdt-papyrus.dev@xxxxxxxxxxx 
To change your delivery options, retrieve your password, or unsubscribe from this list, visit 
https://dev.eclipse.org/mailman/listinfo/mdt-papyrus.dev

_______________________________________________ 
mdt-papyrus.dev mailing list 
mdt-papyrus.dev@xxxxxxxxxxx 
To change your delivery options, retrieve your password, or unsubscribe from this list, visit 
https://dev.eclipse.org/mailman/listinfo/mdt-papyrus.dev

_______________________________________________
mdt-papyrus.dev mailing list
mdt-papyrus.dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/mdt-papyrus.dev


_______________________________________________
mdt-papyrus.dev mailing list
mdt-papyrus.dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/mdt-papyrus.dev


Back to the top