Community
Participate
Working Groups
Closing a large session/modeling project can take a very long time. It does not look like it at first glace because the UI shows a closed project after a relatively short time, but using the "Progress" view reveals that a ResourceSyncClient job is running in background for a long time after that. In my test, it takes about 70s with one CPU core constantly at 100% before the background jobs is done. This needs more investigation to confirm, but from a quick glance at the code of DAnalysisSessionImpl.close() code, it looks like a lot of the work we do there is redundant. For example we take the time to unload the resources (which takes a lot of time and increases memory usage) and then remove them from the resource set. I need to investigate a bit more, but I believe simply removing the resources could not cause much harm. If it could cause a problem in some rare cases, maybe this should be opt-in behavior, with a faster default for the common case.
I performed a quick test where we simply avoid unloading resources we will remove from the resource set. On the test case described above, closing the session now takes less than 2s (compared to 70s). However with this change our internal test suite becomes much slower. After investigation it seems caused by memory pressure. At least 3 classes keep reference to the models in memory if they are not explicily unloaded: * UML2's CacheAdapter. I need to look if faking the unloading by sending the RESOURCE__IS_LOADED event in this case is enough. * EPackageItemProvider. May be simply because we use a lot of ecore files as test fixtures, and not necesarily representative of a standard use case. * oes.diagram.ui.business.internal.view.SiriusLayoutDataManagerImpl keeps references to diagrams and diagram elements. SiriusLayoutDataFlusher normally does the necessary cleanup, but it relies on the fact the diagrams have become proxies (i.e. their resource is unloaded). It could be improved by also cleaning up diagrams which are not inside a resource or resource set.
EMF Compare has the same issue tracked as bug 409689. It looks like they already tried to "fake" the unloading to trick UML's CacheAdapter but that it is not enough.
As a follow up. It looks like the EMF Compare team found a "sweat spot" here which is calling resource.eAdapters.clear(). It will trigger any CrossReferencerAdapter cleanup, notably the CacheAdapter for UML. In the case of EMF compare they remove the resource from the resourceset first and then clear the eAdapters list. This has one notable change compared to Resource.unload(). Resource.unload() would have done a "properContents.forEach(eAdapters.clear())" clearing any eAdapter which has been registered on one object. We might want to keep that behavior. In the end that would be something like: set.getResources().remove(resource); resource.eAdapters.clear(); properContents.forEach(eAdapters.clear())
About leaks that could be caused by performance optimization (or not) Related to comment 1 about EPackageProvider Actually there is a huge leak due to ComposedAdapterFactory that is not disposed when useless. The ComposedAdapterFactory owns XXXItemProvider. This issue is handled in bug 454388. We can notice that PropertySheet retains the previously selected object for which there was something to display. It is enough to select another object that uses SiriusExtensiblePropertyDescriptor(exact condition to be confirmed) to flush the property view Then to have a pertinent Memory profiling, just close Properties view For the specific case uml2. CacheAdapter is a ECrossReferenceAdapter and is set on semantic resource and its content. CacheAdapter is as a static instance. As it is an ECrossReferenceAdapter it owns a inverseCrossReferencer of type InverseCrossReferencer. If Adapter on resource content object are not clear then inverseCrossReferencer keeps a reference to the object. So, if resource.unload is not done anymore, properContents.forEach(eAdapters.clear()), as mentioned in comment 3, is necessary to avoid leaks.
New Gerrit change created: https://git.eclipse.org/r/65163
Below, there is performance result with reverse-sirius-10times modeling project. https://github.com/cbrun/org.eclipse.ecoretools.performance/tree/master/plugins/org.eclipse.emf.ecoretools.design.tests.perf/projects/reverse-sirius-10times The test compares the actual code that does resource.unload() on session resource from one hand and an optimized implementation that iterates on all object to remove all eAdapters. That optimized implementation ensure that custom adapters on resource content should not retain that content. Particularly, the uml2 resource content is not retained anymore by the CacheAdapter. Normal close: 19.3 * unload : 19.3 s Optimized Close: 0.6 s * iteration 0.4 s * eAdapter.clear() 0.2 s The time consumed in the optimized implementation at DASI.close represents about 3% of the previous time.
Gerrit change https://git.eclipse.org/r/65163 was merged to [master]. Commit: http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=de35505b28c6c6f5d583f3616950708e36b02b73
Verified on Sirius 4.0.0 RC1 (4.0.0.201605180923)
Available in Sirius 4.0.0.
