Bugzilla – Full Text Bug Listing
| Summary: | Improve RefreshIDFactory.findID(EObject) implementation to make it faster | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] Sirius | Reporter: | Cedric Brun <cedric.brun> | ||||
| Component: | Core | Assignee: | Cedric Brun <cedric.brun> | ||||
| Status: | CLOSED FIXED | QA Contact: | Pierre-Charles David <pierre-charles.david> | ||||
| Severity: | enhancement | ||||||
| Priority: | P3 | CC: | laurent.redor, maxime.porhel, pierre-charles.david | ||||
| Version: | unspecified | Keywords: | triaged | ||||
| Target Milestone: | 3.0.0 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| See Also: |
https://git.eclipse.org/r/47217 https://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=9e8e0e378e9a7ec6827cafe75fa0a805d42bb1f5 https://git.eclipse.org/r/56943 https://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=9a180dc90f0ef92bc573acf72e90552a9c729cce |
||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
New Gerrit change created: https://git.eclipse.org/r/47217 After checking the implementation of org.eclipse.emf.ecore.impl.MinimalEObjectImpl.eAdapters() indeed the logic is more complex than just returning (and maybe creating) a field like in org.eclipse.emf.ecore.impl.EObjectImpl.eAdapters(). This is a performance regression introduced by Bug 456344 during the 3.0 development cycle. Gerrit change https://git.eclipse.org/r/47217 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=9e8e0e378e9a7ec6827cafe75fa0a805d42bb1f5 Correction has been merged on master. Verified on Sirius 3.0.0rc1a. RefreshIDHolder.getOrCreateID() is still called 132K times in the diagram I chose, but this now amounts to ~0% of the refresh time. Available in Sirius 3.0.0. See https://wiki.eclipse.org/Sirius/3.0.0. New Gerrit change created: https://git.eclipse.org/r/56943 A problem has been detected on ContainerMappingHelper.getNodesCandidates since the add of RefreshIdsHolder. The code should be similar to NodeMappingHelper.getNodesCandidates (by handling the case when container is a DDiagram). The above commit fixes this problem on Sirius 3.1.0. Gerrit change https://git.eclipse.org/r/56943 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=9a180dc90f0ef92bc573acf72e90552a9c729cce |
Created attachment 253181 [details] sirius-interpreter-benchmark-projects Steps to reproduce : 1- install EcoreTools >= 3.0M7 (to be able to load the .ecorebin models) 2- import the projects included in the attached zip 3- open the 8K project, and then one of the diagram (the one you want, it does not really matter) 4- launch a refresh with a Java profiler with no sampling, I use Yourkit with "tracing" mode. Looking at the CPU usage report, one can see that RefreshIDFactory is consistently representing 17%/18% of the time spent in refreshing (136K invocations) org.eclipse.emf.common.util.AbstractEList$EIterator.next() represents 50% of this time (730K invocations) ending up in org.eclipse.emf.ecore.impl.MinimalEObjectImpl.getField(int) org.eclipse.emf.common.util.AbstractEList$EIterator.hasNext() is also fairly well represented with 731K calls and 22% of the findID method time spent. This cost might be caused by the MinimalEObjectImpl (to be confirmed) which has the capability to "share" the list of eAdapters() among a model tree, in that case as the ID is stored as an adapter it might cause inefficiency (here again, that's only suspicion) Anyway, replacing the eAdapters() mechanism by a simple static Map<EObject,Integer> lead to findID representing only 1% of the diagram refresh. This is not a suitable solution but such a differences makes me think it is worth to investigate.