Re: [forge-dev] Forge performance considerations / Bootstrap / modularity

Yes, just for clarification, we are still making major updates and improvements to 1.x in parallel to 2.0 research. On Mon, Nov 5, 2012 at 3:54 AM, Paul Bakker <paul.bakker.nl(a)gmail.com <mailto:paul.bakker.nl@gmail.com>> wrote: +1 for any non-breaking changes. 2.0 is still far away and development on 1.0 shouldn't stop until then. Sent from my iPhone On 5 nov. 2012, at 01:27, "Lincoln Baxter, III" <lincolnbaxter(a)gmail.com <mailto:lincolnbaxter@gmail.com>> wrote: > I think performance improvements in Forge 1.x are very important > even now! > > It's going to be around for a very long time and while we are > working on Forge 2.0 now, the kinds of improvements you are > talking about, Thomas, are very significant! > > How can we help? Do you think you could put this code into a > branch with a single commit (or just one for each change) so that > we can see just how much you had to change? > > The problem with the weld container is the Hibernate Tools > plugin. If that one works, and the Openshift and AS7 plugins > work, then it's safe to assume that it's working fine. It was a > hack to solve a classloading problem, but if we can fix it now, > that's great! > > ~Lincoln > > > On Sat, Nov 3, 2012 at 3:27 PM, George Gastaldi > <ggastald(a)redhat.com <mailto:ggastald@redhat.com>> wrote: > > Hi Thomas, > > Nice to know that you've found some improvements for Forge > 1.x startup time. However, we are rewriting Forge in 2.x from > the ground up and we're focusing on performance and > flexibility in first place. > > We encourage you to try the 2.0 branch and help us on finding > some possible performance problems if you feel so. > > As for now, we are avoiding any major changes and updates to 1.x. > > Best Regards, > > George Gastaldi > > Em 03/11/2012, às 15:41, Thomas Frühbeck <fruehbeck(a)aon.at > <mailto:fruehbeck@aon.at>> escreveu: > > > Hi, > > > > following the discussions about boot-up performance of > Forge I made some > > investigations using a profiler. > > After testing a lot of different scenarios I would like to > share my > > results and ask for your opinion on it. > > > > > ************************************************************************************* > > Issue 1.: hot spot inv. > > org.jboss.forge.bus.cdi.ObserverCaptureExtension.scan > > profiling shows that 37% CPU time are used in firing > > ProcessAnnotatedType events, which are AFAICS only used to > filter old > > event types > > "37,2% - 13.442 ms - 1.866 inv. > > org.jboss.weld.bootstrap.events.ProcessAnnotatedTypeImpl.fire" > > > > By introducing a preparatory check in > ObserverCaptureExtension like > >> if (isMethodsAnnotated(originalType, Observes.class)) { > >> .... > >> } > > > > and something like: > >> private boolean isMethodsAnnotated(AnnotatedType<?> type, > Class<? > > extends Annotation> annotation) { > >> if (type.getClass().isAnnotationPresent(annotation)) > >> return true; > >> for (Method m : type.getClass().getMethods()) { > >> if (m.isAnnotationPresent(annotation)) > >> return true; > >> for (Annotation[] arr : > m.getParameterAnnotations()) { > >> for (Annotation a : arr) { > >> if (a.equals(annotation)) > >> return true; > >> } > >> } > >> } > >> return false; > >> } > > > > we could reduce startup by 20% > > "13,8% - 2.438 ms - 1.798 inv. > > org.jboss.weld.bootstrap.events.ProcessAnnotatedTypeImpl.fire > > > > Test: call Forge with commands "list-plugins" and "exit", > like ~> time > > ((echo forge list-plugins; echo exit;)| bin/forge) > > > > Before optimization: > > ~/forge> time ((echo forge list-plugins && echo exit)| > bin/forge ) > > _____ > > | ___|__ _ __ __ _ ___ > > | |_ / _ \| `__/ _` |/ _ \ \\ > > | _| (_) | | | (_| | __/ // > > |_| \___/|_| \__, |\___| > > |___/ > > > > [no project] forge-distribution-1.1.1-SNAPSHOT $ forge > list-plugins > > > org.richfaces.forge.richfaces-forge-plugin:1.0.5.Final:1.0.0-SNAPSHOT-b1a6ebed-462f-40e1-ac08-82f927489301 > > > com.ocpsoft.forge.prettyfaces-plugin:1.0.2.Final:1.0.0-SNAPSHOT-c6b071ad-a6df-4112-8fff-82ecccd003fb > > > org.jboss.hibernate.forge.hibernate-tools-plugin:1.0.5.Final:1.0.0-SNAPSHOT-afb1da29-a99f-4dd7-b020-5f3ba6073cde > > > org.arquillian.forge.arquillian-plugin:1.0.3-SNAPSHOT:1.0.0-SNAPSHOT-42907982-d02e-4a1d-8ea5-8e66c8013fde > > [no project] forge-distribution-1.1.1-SNAPSHOT $ exit > > > > > > real 0m9.721s > > user 0m26.533s > > sys 0m0.433s > > > > After optimization: > > ~forge1.1.1-weld1.2> time ((echo forge list-plugins; echo > exit;)| bin/forge) > > Using Forge at /raid/home/thomas/java/forge1.1.1-weld1.2 > > _____ > > | ___|__ _ __ __ _ ___ > > | |_ / _ \| `__/ _` |/ _ \ \\ > > | _| (_) | | | (_| | __/ // > > |_| \___/|_| \__, |\___| > > |___/ > > > > [no project] forge1.1.1-weld1.2 $ forge list-plugins > > > org.richfaces.forge.richfaces-forge-plugin:1.0.5.Final:1.0.0-SNAPSHOT-b1a6ebed-462f-40e1-ac08-82f927489301 > > > com.ocpsoft.forge.prettyfaces-plugin:1.0.2.Final:1.0.0-SNAPSHOT-c6b071ad-a6df-4112-8fff-82ecccd003fb > > > org.jboss.hibernate.forge.hibernate-tools-plugin:1.0.5.Final:1.0.0-SNAPSHOT-afb1da29-a99f-4dd7-b020-5f3ba6073cde > > > org.arquillian.forge.arquillian-plugin:1.0.3-SNAPSHOT:1.0.0-SNAPSHOT-42907982-d02e-4a1d-8ea5-8e66c8013fde > > [no project] forge1.1.1-weld1.2 $ exit > > > > > > real 0m6.016s > > user 0m14.482s > > sys 0m0.449s > > > > > > > ************************************************************************************* > > Issue 2.: double starting of Weld container > > I admit that I do not fully understand, what I did and if > this really > > provides the full solution, but it worked and produced a fully > > functional Forge instance. > > > > I tried to restructure the Bootstrap logic a bit: > > > > initializeClassloader(); > > WeldContainer container = weld.initialize(); > > manager = container.getBeanManager(); > > > > try > > { > > loadPlugins(); > > weld.reInitialize(); > > > > where initializeClassloader already uses the > CompositeClassloader: > > > > private static void initializeClassloader() { > > ModuleLoader moduleLoader = > Module.getBootModuleLoader(); > > > > CompositeClassLoader composite = new > CompositeClassLoader(); > > > composite.add(Module.forClassLoader(Bootstrap.class.getClassLoader(), > > true).getClassLoader()); > > > > Thread.currentThread().setContextClassLoader(composite); > > } > > > > and loadPlugins simply adds more classloaders: > > > > CompositeClassLoader composite = > > > (CompositeClassLoader)Thread.currentThread().getContextClassLoader(); > > for (PluginEntry plugin : toLoad) > > { > > Module module = > > > moduleLoader.loadModule(ModuleIdentifier.fromString(plugin.toModuleId())); > > composite.add(module.getClassLoader()); > > > > > > This way the classes are already available in the correct > classloader > > for the later ModularWeld.reInitialize(): > > > > public void reInitialize() { > > bootstrap.revisit(); > > bootstrap.validateBeans(); > > bootstrap.endInitialization(); > > } > > > > Where Bootstrap.revisit simply revisits the BeanDeployments > which have > > been newly found: > > > > private static class DeploymentVisitor { > > ..... > > public Map<BeanDeploymentArchive, BeanDeployment> > revisit() { > > for (BeanDeploymentArchive archvive : > > deployment.getBeanDeploymentArchives()) { > > if > > (!managerAwareBeanDeploymentArchives.containsKey(archvive)) { > > visit(archvive, > managerAwareBeanDeploymentArchives, > > new HashSet<BeanDeploymentArchive>(), true); > > BeanDeployment bd = > > managerAwareBeanDeploymentArchives.get(archvive); > > bd.createBeans(environment); > > bd.deployBeans(environment); > > } > > } > > return managerAwareBeanDeploymentArchives; > > } > > > > > > > ************************************************************************************* > > Issue 3.: invocation of potentially expensive operations on > > non-annotated or annotation-relevant classes > > The current BeanDeployer simply invokes > ProcessAnnotatedType on any > > class, w/o checking, if this class is used by any > annotation anywhere. > > By jandexing all jars and assembling the dispersed Jandex > indices during > > URLScanning we could reduce the expensive WeldClass conversion. > > > > Original: > > 11.108 ms - 2.024 inv. > org.jboss.weld.bootstrap.BeanDeployer.addClass > > Jandex-aware: > > 4.721 ms - 990 inv. > org.jboss.weld.bootstrap.BeanDeployer.addClass > > > > Possible solution: > > - 1. URLHandler scans Jandex file locations > > > > protected void addToDiscovered(String name, URL url) { > > ... > > } else if (name.endsWith(JANDEX_IDX)) { > > discoveredJandexIndexUrls.add(url); > > } > > } > > > > - 2. URLScanner assembles all found Jandex indices: > > > > List<Index> jandexIndexes = new ArrayList<Index>(); > > for (URL jandexUrl : > handler.getDiscoveredJandexIndexUrls()) { > > jandexIndexes.add(new > > IndexReader(jandexUrl.openStream()).read()); > > } > > > > - 3. BeanDeployer does expensive WeldClass loading only > if class is > > mentioned in the Jandex indices > > - 3.a. first collapse the Jandex-indices to a simple > list of class > > names: > > for (Index jandexIndex : jandexIndexes) { > > for (List<AnnotationInstance> aiList : > > jandexIndex.getAnnotations().values()) { > > for (AnnotationInstance ai : aiList) { > > AnnotationTarget at = ai.target(); > > if (at instanceof ClassInfo) { > > jandexAnnotated.add(((ClassInfo)at).name().toString()); > > } else if (at instanceof MethodInfo) { > > > jandexAnnotated.add(((MethodInfo)at).declaringClass().name().toString()); > > } else if (at instanceof > MethodParameterInfo) { > > > jandexAnnotated.add(((MethodParameterInfo)at).method().declaringClass().name().toString()); > > } else if (at instanceof FieldInfo) { > > > jandexAnnotated.add(((FieldInfo)at).declaringClass().name().toString()); > > } > > } > > } > > } > > > > - 3.b. query the aggregated list of Jandex-aware classes > > > > public BeanDeployer addClass(String className) { > > Class<?> clazz = loadClass(className); > > if (clazz != null && isBeanCandidate(clazz) && > > jandexContains(className)) { > > WeldClass<?> weldClass = loadWeldClass(clazz); > > > > > > Yet unsolved issue is the question, wheter an index is > found in the > > corresponding Archive or not, so not to enforce Jandexing > _all_ jars for > > Forge. > > Jandexing is very easy though: jandex -m <jar> > > > > The abovementioned solutions have been implemented and > produced a > > functional and correctly working Forge instance. > > AFAICS the solution for issue 2 could be a means to solve > the reloading > > of plugins too, but I didn't implement and verify this. > > > > The sources are not available online, because the current > implementation > > is highly fragile and not safe for simple usage - too many > changes in > > APIs, dependencies, etc. > > > > Comments welcome! > > > > Regards, > > Thomas > > > > _______________________________________________ > > forge-dev mailing list > > forge-dev(a)lists.jboss.org <mailto:forge-dev@lists.jboss.org> > > https://lists.jboss.org/mailman/listinfo/forge-dev > > _______________________________________________ > forge-dev mailing list > forge-dev(a)lists.jboss.org <mailto:forge-dev@lists.jboss.org> > https://lists.jboss.org/mailman/listinfo/forge-dev > > > > > -- > Lincoln Baxter, III > http://ocpsoft.org > "Simpler is better." > _______________________________________________ > forge-dev mailing list > forge-dev(a)lists.jboss.org <mailto:forge-dev@lists.jboss.org> > https://lists.jboss.org/mailman/listinfo/forge-dev _______________________________________________ forge-dev mailing list forge-dev(a)lists.jboss.org <mailto:forge-dev@lists.jboss.org> https://lists.jboss.org/mailman/listinfo/forge-dev -- Lincoln Baxter, III http://ocpsoft.org "Simpler is better." _______________________________________________ forge-dev mailing list forge-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/forge-dev