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. >>>>> >>>>> >>>>> >>>>
