Community
Participate
Working Groups
M5. EMF has an intelligent Delegate/Descriptor structure to avoid premature loading of modeling plugins. GenericMApplicationElementFactoryImpl is undermining this. The problem is that EPackage ePackage = EPackage.Registry.INSTANCE.getEPackage(emfNsURI); forces a load. It MUST be coded as get() so that the Descriptor is not resolved. The stack trace below shows a plugin that should not be started early doing do. Thread [main] (Suspended (breakpoint at line 195 in PivotPlugin$Implementation)) PivotPlugin$Implementation.start(BundleContext) line: 195 BundleContextImpl$3.run() line: 771 BundleContextImpl$3.run() line: 1 AccessController.doPrivileged(PrivilegedExceptionAction<T>) line: not available [native method] BundleContextImpl.startActivator(BundleActivator) line: 764 BundleContextImpl.start() line: 721 EquinoxBundle.startWorker0() line: 928 EquinoxBundle$EquinoxModule.startWorker() line: 317 EquinoxBundle$EquinoxModule(Module).doStart(Module$StartOptions...) line: 558 EquinoxBundle$EquinoxModule(Module).start(Module$StartOptions...) line: 429 SecureAction.start(Module, Module$StartOptions...) line: 454 EclipseLazyStarter.postFindLocalClass(String, Class<?>, ClasspathManager) line: 107 ClasspathManager.findLocalClass(String) line: 531 EquinoxClassLoader(ModuleClassLoader).findLocalClass(String) line: 330 BundleLoader.findLocalClass(String) line: 306 SingleSourcePackage.loadClass(String) line: 36 BundleLoader.findClassInternal(String, boolean) line: 371 BundleLoader.findClass(String, boolean) line: 331 BundleLoader.findClass(String) line: 323 EquinoxClassLoader(ModuleClassLoader).loadClass(String, boolean) line: 160 EquinoxClassLoader(ClassLoader).loadClass(String) line: 358 CGModelPackageImpl.initializePackageContents() line: 3350 CGModelPackageImpl.init() line: 849 CGModelPackage.<clinit>() line: 87 AutoCGModelPackageImpl.init() line: 111 AutoCGModelPackage.<clinit>() line: 85 [local variables unavailable] Unsafe.ensureClassInitialized(Class) line: not available [native method] UnsafeFieldAccessorFactory.newFieldAccessor(Field, boolean) line: 43 ReflectionFactory.newFieldAccessor(Field, boolean) line: 140 Field.acquireFieldAccessor(boolean) line: 1057 Field.getFieldAccessor(Object) line: 1038 Field.get(Object) line: 379 RegistryReader$EPackageDescriptor.getEPackage() line: 273 EPackageRegistryImpl.getEPackage(String) line: 127 GenericMApplicationElementFactoryImpl$MApplicationElementClassToEClass.addToMapping(IConfigurationElement[]) line: 257 GenericMApplicationElementFactoryImpl$MApplicationElementClassToEClass.added(IExtension[]) line: 183 GenericMApplicationElementFactoryImpl$MApplicationElementClassToEClass.initialize(IExtensionRegistry) line: 159 GenericMApplicationElementFactoryImpl.<init>(IExtensionRegistry) line: 78 ModelServiceImpl.<init>(IEclipseContext) line: 110 E4Application.createDefaultContext() line: 488 E4Application.createE4Workbench(IApplicationContext, Display) line: 202 Workbench$5.run() line: 580 Realm.runWithDefault(Realm, Runnable) line: 332 Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 566 PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 150 IDEApplication.start(IApplicationContext) line: 125 EclipseAppHandle.run(Object) line: 196 EclipseAppLauncher.runApplication(Object) line: 109 EclipseAppLauncher.start(Object) line: 80 EclipseStarter.run(Object) line: 372 EclipseStarter.run(String[], Runnable) line: 226 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: 57 DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 43 Method.invoke(Object, Object...) line: 606 Main.invokeFramework(String[], URL[]) line: 636 Main.basicRun(String[]) line: 591 Main.run(String[]) line: 1450 Main.main(String[]) line: 1426
René: Any thoughts on this. This was added as part of your work on bug 419888.
This sounds like a bit of a disaster that you might not notice in the platform directly but there are so many projects using generated models that activating all of them largely defeats the purpose of lazy activation...
(In reply to Ed Merks from comment #2) > defeats the purpose of lazy activation... and may explain why on M5 I'm seeing PermGen problems.
(In reply to Ed Willink from comment #3) > (In reply to Ed Merks from comment #2) > > defeats the purpose of lazy activation... > > and may explain why on M5 I'm seeing PermGen problems. Could one of you provide a patch / Gerrit review?
(In reply to Ed Willink from comment #3) > (In reply to Ed Merks from comment #2) > > defeats the purpose of lazy activation... > > and may explain why on M5 I'm seeing PermGen problems. The behavior has been in since M4, so it may not be the sole cause. That said, the current behavior is unacceptable. I`m looking at reverting the change.
(In reply to Paul Elder from comment #5) > The behavior has been in since M4, so it may not be the sole cause. That > said, the current behavior is unacceptable. I`m looking at reverting the > change. A bit extreme IMHO, if I get Eds first comment correctly, the patch is trivial, just using a different method. I can be wrong here, as my EMF knowledge is very limited, that is why I suggested to add a patch to the bug report.
(In reply to Lars Vogel from comment #6) > A bit extreme IMHO, if I get Eds first comment correctly, the patch is > trivial, just using a different method. I can be wrong here, as my EMF > knowledge is very limited, that is why I suggested to add a patch to the bug > report. Hi. I doubt it's trivial. get() rather than getEPackage() avoids the load, but gives you only a descriptor which is almost useless. I couldn't underrstand the code; no obvious comments and too many opaque configuration elements, but there seemed to be comments about EClass. If so the whole algorithm is fundamentally flawed.
I've tried to make sense of the Bug 419888 and Bug 402781 and they don't make much sense to me. I find it deeply worrying that the platform in using EMF seems unaware of how HUGE EMF is. Real Eclipse users have thousands of EMF models, so performance assessment by the platform team using the platform is very naive. As a minimum at least Papyrus and Ecore Tools/Sirius and a major CDO application should be installed. The specific building of a Class to EClass map seems to be solely for the tiny problem of platform model extensibility. Scanning all EMF models is about as mad as JDT creating a class hierarchy of the world. For the specific problem a major improvement might be: for (Object object : EPackage.Registry.INSTANCE.values()) { if (object instanceof EPackage) { EPackage ePackage = (EPackage) object; ... } } but again I'm puzzled. The original recommendation did not use configuration elements but the M5 code does just repeating all the extension point processing that EMF already did. The above will be much better but will steadily degrade as more plugins are started increasing the number of instanceof hits. I strongly recommend that the platform uses a platform-specific solution that does not accidentally include all user developments as potential platform contributions. The bug threads seem to have some references to dynamic extension and listening to the EMF extension point. I've no idea what the purpose of this is, but beware that EMF has two very different modes of operation, one for not-extensible plugged in compiled models, and another for dynamic models. There is a bit of a gap between these two and the platform is in danger of re-inventing the wheel in trying to bridge the gap.
Hi folks, sorry that I didn't thought about this follow effective in my solution of Bug 419888. I think I can provide a solution for this with an idea Tom Schindl once suggested. He thought we should use a custom registry which contains only those elements relevant for the application model. So we could provide a registry in the E4 UI workbench which contains all UI relevant EPackages and the EModelService#createModelElement(*) code is only building a map for elements registered there. If somebody wants to provide some new UI relevant model elements he has to register his EPackage either via Code or via ExtensionPoint in that registry. This would cause only the UI relevant elements to be loaded eagerly and all other EMF related objects would be loaded lazily in their usual way. If everybody is happy with this solution, I can provide a patch in the next few days. If somebody is not happy with it, tell me why and provide an alternative please :-). René P.S.: @Ed Merks: Will there be a solution for such a scenario (Class-to-EClass) in one of the following EMF versions?
That sounds like a reasonable approach. In terms of mapping a java.lang.Class to an EClass, is it the generated Impl class you're looking to map, or the generated interface? What do you need this mapping for?
(In reply to Ed Merks from comment #10) > That sounds like a reasonable approach. > > In terms of mapping a java.lang.Class to an EClass, is it the generated Impl > class you're looking to map, or the generated interface? What do you need > this mapping for? Most of the time (99.9%) the generated interface will be used and so a mapping form the generated interface class to the EClass would be reasonable. With the resolved EClass the EcoreUtil#create(EClass) can be called to use the EMF-Factory approach to create a correct implementation instead of finding a way to resolve the correct implementation (as we did in this case). As you can see in the application model the developers wanted to hide the EMF nature from the API and so you are left alone how to find the correct implementation for that interface. Another usage example would be a third-party framework like MyBatis which is able to register Object-Factories to create concrete objects from classes. But the class objects which are used by the framework are simple Java-classes and not EClasses which means a normal create instance will not work. And so the developer has to find a way to map the interface class to the correct implementation.
I've provided a solution on gerrit https://git.eclipse.org/r/#/c/21828/ which uses a custom ExtensionPoint to populate UI related EPackages. With this solution all other EPackages will be loaded lazy again and we still have a solution to create all UI related model objects (even custom ones) with the EModelService. I didn't implement a programmatic way to register those packages because the previous solution didn't have one.
(In reply to René Brandstetter from comment #12) > I've provided a solution on gerrit > > https://git.eclipse.org/r/#/c/21828/ > Thanks René. This patch is missing the extension point definition. Can you push a revised version?
Created attachment 239850 [details] The missing exsd file Sorry, I'm currently behind a firewall and I have problems with my connection (even via the proxy.eclipse.org) and so I will push it tonight. I've added the missing file as an attachment, if you can't wait until this happens. The file is located under: bundles/org.eclipse.e4.ui.workbench/schema/
Hi, I've added the file and the build was successful, so feel free to review and put your comments on it. René
(In reply to René Brandstetter from comment #15) > Hi, > > I've added the file and the build was successful, so feel free to review and > put your comments on it. > > René Looks good. Released to master as: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=0e28da05588004852d2f57c5b6c1f36106342bd9
(In reply to René Brandstetter from comment #15) > Hi, > > I've added the file and the build was successful, so feel free to review and > put your comments on it. Hi René, could you confirm the fix on http://download.eclipse.org/eclipse/downloads/drops4/I20140306-1200/ ? PW
(In reply to Paul Webster from comment #17) > (In reply to René Brandstetter from comment #15) > > Hi, > > > > I've added the file and the build was successful, so feel free to review and > > put your comments on it. > > Hi René, could you confirm the fix on > http://download.eclipse.org/eclipse/downloads/drops4/I20140306-1200/ ? > > PW Hi Paul, everything required is in this build (schema and correct class files). René
Thanks René In I20140306-1200 PW
