Issue is that cache cant really be better until you use xbean finder (discovery) to handle it when finder is used and fallback on reflection in other cases. Requires some work but it is the fastest possible impl for us. For a not xbean related impl current cache is already optimal afaik but load class is slow.
That said, if you use cds for your stack and append to your classpath your own modules (easy in ide/maven) then you benefit from this perf boost since the classpath prefix is what is used. Requires some dev setup and unrelated to owb but can help. +1 for the prop (simple boolean i think) anyway. I can also do it on wednesday if it helps. Le lun. 27 juil. 2020 à 20:46, Thomas Andraschko < [email protected]> a écrit : > I would improve the cache If possible, otherwise add a property and > disable the Feature per default, to have a good startup perf > > vicente Rossello <[email protected]> schrieb am Mo., 27. Juli > 2020, 20:33: > >> Hi, I'll leave the check also outside. >> >> The problem with CDs is that they are not good for development, where I >> really want a fast startup time >> >> If you want after this PR I can make a property to disable this. >> >> El lun., 27 jul. 2020 20:20, Romain Manni-Bucau <[email protected]> >> escribió: >> >>> The check happens before the loop cause it is faster if you have more >>> than one class per package (which is statistically true). >>> >>> You can also enable CDS to bypass this load time. >>> >>> I would also be happy to have a property to skip this whole feature too >>> as proposed some times ago. >>> >>> Le lun. 27 juil. 2020 à 12:57, Vicente Rossello <[email protected]> >>> a écrit : >>> >>>> Hi, >>>> >>>> I made a test with veto at package level to try to see if the patch is >>>> correct, if it's of any use. >>>> https://github.com/apache/openwebbeans/pull/30 >>>> >>>> Feel free to comment or just discard it. >>>> >>>> On Mon, Jul 27, 2020 at 11:30 AM Mark Struberg <[email protected]> >>>> wrote: >>>> >>>>> Should work as expected. I need to think through if we really need to >>>>> walk into all superpackages in the while(true). >>>>> Or if we should try to get those results from the cache as well. >>>>> >>>>> a.b.c.d.e.f.x1 >>>>> a.b.c.d.e.f.x2 >>>>> >>>>> In that case the whole list up to f should already be cached for x2. >>>>> >>>>> LieGrue, >>>>> strub >>>>> >>>>> >>>>> >>>>> Am 27.07.2020 um 11:19 schrieb Vicente Rossello < >>>>> [email protected]>: >>>>> >>>>> Hi, >>>>> >>>>> That's what already did and times become as usual, but I'm not sure if >>>>> it breaks something (I'm not using any veto). Tests in openwebbeans-impl >>>>> do >>>>> work with this change >>>>> >>>>> On Mon, Jul 27, 2020 at 11:14 AM Mark Struberg <[email protected]> >>>>> wrote: >>>>> >>>>>> Hi Vincente! >>>>>> >>>>>> There is a bit code before that block which already checks the cache: >>>>>> >>>>>> Boolean result = packageVetoCache.get(previousPackage); >>>>>> if (result != null && result) >>>>>> { >>>>>> return result; >>>>>> } >>>>>> >>>>>> >>>>>> Imo it should also return if a False is cached. >>>>>> can you please remove the && result and do a bench again? >>>>>> >>>>>> txs and LieGrue, >>>>>> strub >>>>>> >>>>>> >>>>>> >>>>>> Am 27.07.2020 um 10:00 schrieb Vicente Rossello < >>>>>> [email protected]>: >>>>>> >>>>>> Hi, >>>>>> >>>>>> I've seen a startup performance regression since OWB 2.0.17 and >>>>>> latest snapshot. Our boot times have increased from 10 to about 14 >>>>>> seconds >>>>>> (only OWB side). I can see that it always try to load the same >>>>>> package-info's in: >>>>>> >>>>>> while (true) >>>>>> { >>>>>> try // not always existing but enables to go further when getPackage >>>>>> is not available (graal) >>>>>> { >>>>>> pckge = classLoader.loadClass(previousPackage + >>>>>> (previousPackage.isEmpty() ? "" :".") + >>>>>> "package-info").getPackage(); >>>>>> break; >>>>>> } >>>>>> catch (Exception e) >>>>>> { >>>>>> if (previousPackage.isEmpty()) >>>>>> { >>>>>> pckge = null; >>>>>> break; >>>>>> } >>>>>> packageVetoCache.put(previousPackage, false); >>>>>> idx = previousPackage.lastIndexOf('.'); >>>>>> if (idx > 0) >>>>>> { >>>>>> previousPackage = previousPackage.substring(0, idx); >>>>>> } >>>>>> else >>>>>> { >>>>>> previousPackage = ""; >>>>>> } >>>>>> } >>>>>> } >>>>>> >>>>>> >>>>>> I think that, in this loop, it should take into account the >>>>>> packageVetoCache (whether it's true or false). Is it correct? Do you >>>>>> want a PR with this correction? >>>>>> >>>>>> >>>>>> Best regards, >>>>>> >>>>>> Vicente. >>>>>> >>>>>> >>>>>> >>>>>
