Community
Participate
Working Groups
Created attachment 137980 [details] Test project for reproducing the problem Build ID: I20090515-1143 (RC1) MWE/XPand version: 0.7.0v20090526 (RC1) Steps To Reproduce: 1. Run "RunAsJava.launch" of attached test project 2. Run "RunAsPlugin.launch" of attached test project More information: If Xpand templates call ext helper files, which in turn refer to existing static Java methods, the MWE spends most of the overall execution time for class loading, if it is executed inside a plugin. It is much faster when being executed as a normal Java application. To easily reproduce this behaviour, I have set up a test project, which is attached. For comparison, it contains the two launcher configurations "RunAsJava.launch" and "RunAsPlugin.launch". Both execute exactly the same code: Two MWE workflows, which loop over Integers and either do a direct output (perftest1.mwe) or use a Java helper (perftest2.mwe). Execution times are: 1,0 secs for RunAsJava/perftest1.mwe 4,4 secs for RunAsJava/perftest2.mwe 1,1 secs for RunAsPlugin/perftest1.mwe 16,4 secs for RunAsPlugin/perftest2.mwe This shows that delegating to a Java helper always has a serious performance impact (from 1 up to 4 secs), but when being used inside a plugin this tremendously worsens to a factor of 16. Doing profiling, it can be seen that most of the time is spent on class loading (org.eclipse.emf.mwe.core.resources.ResourceLoaderDefaultImpl.loadClass) - see attached screenshot of JProfiler. I think the problem does not only exist for Java helper classes, but also for meta-model classes that seem to be loaded (again and again) via the same mechanism. Maybe it's worth considering to cache these classes during a workflow execution?
Created attachment 137981 [details] Screenshot from profiler
Created attachment 139174 [details] Proposed patch The attached proposed patch indeed results in a really *dramatic* performance gain. However, before applying it, it should be reviewed carefully to avoid any unwanted side effects.
Caching the classes seems to be the solution. Maybe we should just warn people that dynamic OSGi won't work then (I guess most people can live with that) I'll review your patch. Thanks!
*** Bug 278388 has been marked as a duplicate of this bug. ***
What really seems to slow down the class loading is the fact that the Xtend and MWE plug-ins declare a "dependent" buddy policy. This causes the class loader to search for classes in bundles that directly or indirectly depend on the Xtend and MWE plug-ins. This takes a lot of time. What makes things even worse is that the JavaBeans meta model will also look for matching BeanInfo classes. How about using the "registered" buddy policy instead?
Thanks Knut, for finding out. It would indeed be better to have a registered buddy-policy instead. But changing it now would break clients. Therefore I'ld prefer to go with the caching for now.
I've added a CachingResourceLoader which can be (need to be) configured like this: ResourceLoaderFactory.setCurrentThreadResourceLoader( new CachingResourceLoader( new ResourceLoaderImpl(classLoader) ) ); Does it help? Is it sufficient? So by default the caching is *not active*! As soon as you need performance improvements due to use of JavaBeansMetamodel in an OSGi environment you should add the three lines above.
Created attachment 141199 [details] caching resource loader patch Hi Sven, Thanks. The CachingResourceLoaderImpl doesn't implement the ResourceLoader interface. Please see attached patch.
Ups, sorry :-|
Hi Sven, Thanks a lot for this fix, it indeed improves the performance dramatically. Just a few final remarks/questions from my side: 1. The correct code (note the previously missing "Impl" for the activation is ResourceLoaderFactory.setCurrentThreadResourceLoader( new CachingResourceLoaderImpl( new ResourceLoaderImpl(classLoader) ) ); Will this be documented clearly anywhere in the official docs? Or how can it be made sure that nobody files similar bug reports about the same in future? 2. The CachingResourceLoaderImpl throws a NPE in getResourceAsStream() if a resource cannot be found; this is different from the default implementation and should be changed. I have attached a patch (which I have successfully tested) and reopened this issue. Thanks again for your help, Kai
Created attachment 141606 [details] Proposed patch for the NPE in getResourceAsStream()
Shall we include Kai's patch for SR1?
Looks good so far, please apply the patch.
Patch applied to HEAD
It seems that even with the caching ResourceLoader the JavaBeansMetamodel is still significantly slower in an OSGi environment than in a standard Java process. We'll have to investigate (profile) what the reason is. Targeting for the next bugfix release (0.7.2), which is planned for August 7th.
Ok. Do you happen to have a rest project for profiling?
I'll have a look at it.
Created attachment 142151 [details] Workaround for the buddy policy issue The patch contains another implementation of the ResourceLoader interface, that requires an explicit bundle id as class loading context. This is times faster than the buddy classloading that is done by the default implementation. The resource loader factory should be configured like this in case of serious performance issues with OSGI classloading: CachingResourceLoaderImpl crl = new CachingResourceLoaderImpl(new OsgiResourceLoader( Activator.PLUGIN_ID, // a plugin that has declared the required deps explicitly MWEPlugin.INSTANCE.getBaseURL(), RunWorkflows.class.getClassLoader())); // a class from this plugin ResourceLoaderFactory.setCurrentThreadResourceLoader(crl); Please verify this patch.
The new OsgiResourceLoader appears to nicely circumvent the performance problems caused by the buddy policy! I get the impression that the MWE plug-in's base URL is only required by the WorkflowRunner internally; e.g. to load and display the MWE version (see WorkflowRunner#getVersion()). If that is true then it would maybe be possible to load the version without using the ResourceLoader (possibly using WorkflowRunner.class.getClassLoader()) or just hide that detail by not requiring the baseURL constructor parameter. This is also "problematic" as the class MWEPlugin is in an "internal" package. An alternative for the developer is of course: Platform.getBundle("org.eclipse.emf.mwe.core").getEntry("/"). I like the fact that methods like getResource(String) are not declared as final. (In reply to comment #18) > Created an attachment (id=142151) [details] > Workaround for the buddy policy issue > > The patch contains another implementation of the ResourceLoader interface, that > requires an explicit bundle id as class loading context. This is times faster > than the buddy classloading that is done by the default implementation. > > The resource loader factory should be configured like this in case of serious > performance issues with OSGI classloading: > > CachingResourceLoaderImpl crl = new CachingResourceLoaderImpl(new > OsgiResourceLoader( > Activator.PLUGIN_ID, // a plugin that has declared the required deps > explicitly > MWEPlugin.INSTANCE.getBaseURL(), > RunWorkflows.class.getClassLoader())); // a class from this plugin > ResourceLoaderFactory.setCurrentThreadResourceLoader(crl); > > Please verify this patch. >
Created attachment 142361 [details] Workaround for the buddy policy issue - Update This patch supersedes the previous attachement and incorporates Knut's suggestion.
Committed to HEAD.
