Hi Jon, It is intended to not cache the missed hits since they shouldn't occur at runtime and would open the door to OOME issues. I looked at your sample, there is indeed a bug in ActionServlet#process which should do that resolution in init().
>From my XP frameworks using that kind of resolution cache it themselves (JSF for ex) if relevant or don't cache it cause it is not a big perf issue (could be for jbatch) else it looks like bugs. Hope it helps Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://rmannibucau.metawerx.net/> | Old Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book <https://www.packtpub.com/application-development/java-ee-8-high-performance> Le mar. 8 nov. 2022 à 15:36, Jonathan Gallimore < jonathan.gallim...@gmail.com> a écrit : > Thanks for the reply, Romain! > > There's a couple of points on this one. Firstly, it's not startup behaviour > that's an issue in this specific case, it's actually resolution at runtime. > In effect, because the child InjectionResolver cannot resolve the bean, and > never caches that fact, it tries to work through the beans available each > and every time an attempt is made to resolve the bean. Only once that has > happened, do we delegate to the parent InjectionResolver (which does have > the bean cached). > > On the TomEE-side, WebappInjectionResolver is a very thin extension of > InjectionResolver, and it hasn't changed in 7 years: > > https://github.com/apache/tomee/blob/tomee-8.x/container/openejb-core/src/main/java/org/apache/openejb/cdi/WebAppInjectionResolver.java > . > If there's a specific change you think might be useful, I'm happy to give > it a go. We could, theoretically, do the caching in > WebappInjectionResolver, although I'm hesitant to double the number of > HashMaps we're keeping around for caching these resolutions. Making > InjectionResolver.resolvedBeansByType (and maybe > InjectionResolver.resolvedBeansByName) protected rather than private could > offer an easy way for anyone consuming OpenWebBeans to extend its behaviour > in this regard. We could then add any settings, etc on the TomEE side and > just use the existing caching hash maps - for example we could check the > cache of both the child and the parent InjectionResolvers before doing > anything else. > > I did put together a sample project to demonstrate what I'm seeing: > https://github.com/jgallimore/cdi-resolve-type-perf. > > It can be built with: > > mvn clean install > > TomEE run with: > > cd hello-perf-test > mvn tomee:run > > And the performance test run with: > > cd hello-perf-test > ./grinder.sh > > In essence, the servlet is looking up a CDI bean each time, and always gets > the InjectionResolver for the web archive. I've deliberately created a > large number of classes (10000) for this to work through in a big jar that > sits in WEB-INF/lib. The actual bean I want to resolve is in the EJB module > in the EAR, and is picked up when trying to resolve it from the war file > fails. Running a performance test on this runs at around 1500 transactions > per second (*). Allowing the InjectionResolver to cache > Collections.EMPTY_SET when a type isn't resolved - e.g.: > > if (!startup) > { > if (resolvedComponents == null || resolvedComponents.size() == > 0) > { > resolvedBeansByType.put(cacheKey, Collections.EMPTY_SET); > } > else > { > resolvedBeansByType.put(cacheKey, resolvedComponents); > } > > if (logger.isLoggable(Level.FINE)) > { > logger.log(Level.FINE, "DEBUG_ADD_BYTYPE_CACHE_BEANS", > cacheKey); > } > } > > this goes up to around 65000 transactions per second (*), so the impact is > definitely felt. > > * WSL2 Ubuntu/Windows 11 on i7-12700H 2.30 GHz, 32GB RAM > > Thanks > > Jon > > On Thu, Nov 3, 2022 at 4:50 PM Romain Manni-Bucau <rmannibu...@gmail.com> > wrote: > > > Hi Jon, > > > > The fix Thomas did is accurate and needed so think we should be fine like > > that but I also agree the "pre-fix" implementation of TomEE never caught > up > > this change. > > I guess the fix is to ensure the wrappers we have in TomEE also have this > > kind of behavior but handling the cache for TomEE startup which is not > > aligned on OWB startup for an EAR. > > > > Hope it makes sense. > > > > Romain Manni-Bucau > > @rmannibucau <https://twitter.com/rmannibucau> | Blog > > <https://rmannibucau.metawerx.net/> | Old Blog > > <http://rmannibucau.wordpress.com> | Github < > > https://github.com/rmannibucau> | > > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > > < > > > https://www.packtpub.com/application-development/java-ee-8-high-performance > > > > > > > > > Le jeu. 3 nov. 2022 à 17:07, Jonathan Gallimore < > > jonathan.gallim...@gmail.com> a écrit : > > > > > Hey folks > > > > > > I've run into an interesting problem on the TomEE-side, running an EAR > > > file. This seems to end up with 2 injection resolvers for the > > application: > > > one for the web part of the application, and a parent for the EAR. > > > > > > When resolving by type at runtime, with large archives, we've seen a > > > slowdown from previous versions of OpenWebBeans. This appears to be > > caused > > > by the InjectionResolver.implResolveByType returning an empty set. > TomEE > > > will then try the parent resolver (successfully), but the result (empty > > > set) in the child resolver is not cached, so it searches through the > > whole > > > set of beans each time. The result used to be cached, prior to this > > commit: > > > > > > > > > https://github.com/apache/openwebbeans/commit/a6d22d2abf6d6d5a02a14be1367daacca450359d > > > > > > Could caching the empty result in the InjectionResolver be > re-introduced, > > > albeit with a switch? (I'm happy for the default to be "don't cache"). > > > Interestingly, InjectionResolver.implResolveByName does cache the empty > > > result. I'm happy to create a PR. > > > > > > Many thanks > > > > > > Jon > > > > > >
