You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@openwebbeans.apache.org by Vicente Rossello <co...@gmail.com> on 2020/07/27 08:00:36 UTC
OWB 2.0.17 startup performance regression
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.
Re: OWB 2.0.17 startup performance regression
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Yes with some bit of reflection but not sure it helps there. The only trick
we have for graal is a catch (we could move outside to have a single catch)
but this costs almost nothing compared the loadclass itself in jvm mode.
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 mer. 29 juil. 2020 à 12:02, Mark Struberg <st...@yahoo.de> a écrit :
> Is there a way to detect whether we are running in Graal?
>
> LieGrue,
> strub
>
>
> Am 29.07.2020 um 08:46 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> FYI, created https://issues.apache.org/jira/browse/OWB-1343 for the flag
> to skip all the loadClass(package-info).
>
> 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. 28 juil. 2020 à 10:10, Mark Struberg <st...@yahoo.de> a écrit :
>
>> Go for it please. Can also do smallish tweaks later on. It's for sure
>> much better than before!
>> And thanks Vincente for the work!
>>
>> LieGrue,
>> strub
>>
>>
>> Am 28.07.2020 um 08:52 schrieb Romain Manni-Bucau <rmannibucau@gmail.com
>> >:
>>
>> Current pr is mergeable for me and fix is good.
>> Will let Mark have a last review since he commented first the pr but
>> looks good to me now, thanks a lot Vincente.
>>
>>
>> Le lun. 27 juil. 2020 à 22:35, Thomas Andraschko <
>> andraschko.thomas@gmail.com> a écrit :
>>
>>> Then +1 to remove the &&
>>>
>>> Vicente Rossello <co...@gmail.com> schrieb am Mo., 27. Juli
>>> 2020, 22:32:
>>>
>>>> Hi,
>>>>
>>>> Just changing outside the loop
>>>>
>>>> Boolean result = packageVetoCache.get(previousPackage);
>>>> if (result != null && result)
>>>> {
>>>> return result;
>>>> }
>>>>
>>>>
>>>> to
>>>>
>>>> Boolean result = packageVetoCache.get(previousPackage);
>>>> if (result != null)
>>>> {
>>>> return result;
>>>> }
>>>>
>>>>
>>>> solves the performance problem, the difference is unnoticeably. The real problem is that, for every class, is checking for a package-info in all "upper packages" every single time.
>>>>
>>>>
>>>> On Mon, Jul 27, 2020 at 10:07 PM Romain Manni-Bucau <
>>>> rmannibucau@gmail.com> wrote:
>>>>
>>>>> 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 <
>>>>> andraschko.thomas@gmail.com> 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 <ro...@gmail.com> 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 <
>>>>>>> rmannibucau@gmail.com> 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 <
>>>>>>>> cocorossello@gmail.com> 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 <st...@yahoo.de>
>>>>>>>>> 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 <
>>>>>>>>>> cocorossello@gmail.com>:
>>>>>>>>>>
>>>>>>>>>> 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 <st...@yahoo.de>
>>>>>>>>>> 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 <
>>>>>>>>>>> cocorossello@gmail.com>:
>>>>>>>>>>>
>>>>>>>>>>> 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.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>
>
Re: OWB 2.0.17 startup performance regression
Posted by Mark Struberg <st...@yahoo.de>.
Is there a way to detect whether we are running in Graal?
LieGrue,
strub
> Am 29.07.2020 um 08:46 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> FYI, created https://issues.apache.org/jira/browse/OWB-1343 <https://issues.apache.org/jira/browse/OWB-1343> for the flag to skip all the loadClass(package-info).
>
> 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. 28 juil. 2020 à 10:10, Mark Struberg <struberg@yahoo.de <ma...@yahoo.de>> a écrit :
> Go for it please. Can also do smallish tweaks later on. It's for sure much better than before!
> And thanks Vincente for the work!
>
> LieGrue,
> strub
>
>
>> Am 28.07.2020 um 08:52 schrieb Romain Manni-Bucau <rmannibucau@gmail.com <ma...@gmail.com>>:
>>
>> Current pr is mergeable for me and fix is good.
>> Will let Mark have a last review since he commented first the pr but looks good to me now, thanks a lot Vincente.
>>
>>
>> Le lun. 27 juil. 2020 à 22:35, Thomas Andraschko <andraschko.thomas@gmail.com <ma...@gmail.com>> a écrit :
>> Then +1 to remove the &&
>>
>> Vicente Rossello <cocorossello@gmail.com <ma...@gmail.com>> schrieb am Mo., 27. Juli 2020, 22:32:
>> Hi,
>>
>> Just changing outside the loop
>>
>> Boolean result = packageVetoCache.get(previousPackage);
>> if (result != null && result)
>> {
>> return result;
>> }
>>
>> to
>> Boolean result = packageVetoCache.get(previousPackage);
>> if (result != null)
>> {
>> return result;
>> }
>>
>> solves the performance problem, the difference is unnoticeably. The real problem is that, for every class, is checking for a package-info in all "upper packages" every single time.
>>
>> On Mon, Jul 27, 2020 at 10:07 PM Romain Manni-Bucau <rmannibucau@gmail.com <ma...@gmail.com>> wrote:
>> 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 <andraschko.thomas@gmail.com <ma...@gmail.com>> 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 <rossello.vicente@gmail.com <ma...@gmail.com>> 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 <rmannibucau@gmail.com <ma...@gmail.com>> 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 <cocorossello@gmail.com <ma...@gmail.com>> 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 <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 <struberg@yahoo.de <ma...@yahoo.de>> 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 <cocorossello@gmail.com <ma...@gmail.com>>:
>>>
>>> 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 <struberg@yahoo.de <ma...@yahoo.de>> 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 <cocorossello@gmail.com <ma...@gmail.com>>:
>>>>
>>>> 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.
>>>
>>
>
Re: OWB 2.0.17 startup performance regression
Posted by Romain Manni-Bucau <rm...@gmail.com>.
FYI, created https://issues.apache.org/jira/browse/OWB-1343 for the flag to
skip all the loadClass(package-info).
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. 28 juil. 2020 à 10:10, Mark Struberg <st...@yahoo.de> a écrit :
> Go for it please. Can also do smallish tweaks later on. It's for sure much
> better than before!
> And thanks Vincente for the work!
>
> LieGrue,
> strub
>
>
> Am 28.07.2020 um 08:52 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Current pr is mergeable for me and fix is good.
> Will let Mark have a last review since he commented first the pr but looks
> good to me now, thanks a lot Vincente.
>
>
> Le lun. 27 juil. 2020 à 22:35, Thomas Andraschko <
> andraschko.thomas@gmail.com> a écrit :
>
>> Then +1 to remove the &&
>>
>> Vicente Rossello <co...@gmail.com> schrieb am Mo., 27. Juli 2020,
>> 22:32:
>>
>>> Hi,
>>>
>>> Just changing outside the loop
>>>
>>> Boolean result = packageVetoCache.get(previousPackage);
>>> if (result != null && result)
>>> {
>>> return result;
>>> }
>>>
>>>
>>> to
>>>
>>> Boolean result = packageVetoCache.get(previousPackage);
>>> if (result != null)
>>> {
>>> return result;
>>> }
>>>
>>>
>>> solves the performance problem, the difference is unnoticeably. The real problem is that, for every class, is checking for a package-info in all "upper packages" every single time.
>>>
>>>
>>> On Mon, Jul 27, 2020 at 10:07 PM Romain Manni-Bucau <
>>> rmannibucau@gmail.com> wrote:
>>>
>>>> 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 <
>>>> andraschko.thomas@gmail.com> 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 <ro...@gmail.com> 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 <
>>>>>> rmannibucau@gmail.com> 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 <
>>>>>>> cocorossello@gmail.com> 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 <st...@yahoo.de>
>>>>>>>> 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 <
>>>>>>>>> cocorossello@gmail.com>:
>>>>>>>>>
>>>>>>>>> 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 <st...@yahoo.de>
>>>>>>>>> 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 <
>>>>>>>>>> cocorossello@gmail.com>:
>>>>>>>>>>
>>>>>>>>>> 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.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>
Re: OWB 2.0.17 startup performance regression
Posted by Mark Struberg <st...@yahoo.de>.
Go for it please. Can also do smallish tweaks later on. It's for sure much better than before!
And thanks Vincente for the work!
LieGrue,
strub
> Am 28.07.2020 um 08:52 schrieb Romain Manni-Bucau <rm...@gmail.com>:
>
> Current pr is mergeable for me and fix is good.
> Will let Mark have a last review since he commented first the pr but looks good to me now, thanks a lot Vincente.
>
>
> Le lun. 27 juil. 2020 à 22:35, Thomas Andraschko <andraschko.thomas@gmail.com <ma...@gmail.com>> a écrit :
> Then +1 to remove the &&
>
> Vicente Rossello <cocorossello@gmail.com <ma...@gmail.com>> schrieb am Mo., 27. Juli 2020, 22:32:
> Hi,
>
> Just changing outside the loop
>
> Boolean result = packageVetoCache.get(previousPackage);
> if (result != null && result)
> {
> return result;
> }
>
> to
> Boolean result = packageVetoCache.get(previousPackage);
> if (result != null)
> {
> return result;
> }
>
> solves the performance problem, the difference is unnoticeably. The real problem is that, for every class, is checking for a package-info in all "upper packages" every single time.
>
> On Mon, Jul 27, 2020 at 10:07 PM Romain Manni-Bucau <rmannibucau@gmail.com <ma...@gmail.com>> wrote:
> 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 <andraschko.thomas@gmail.com <ma...@gmail.com>> 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 <rossello.vicente@gmail.com <ma...@gmail.com>> 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 <rmannibucau@gmail.com <ma...@gmail.com>> 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 <cocorossello@gmail.com <ma...@gmail.com>> 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 <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 <struberg@yahoo.de <ma...@yahoo.de>> 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 <cocorossello@gmail.com <ma...@gmail.com>>:
>>
>> 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 <struberg@yahoo.de <ma...@yahoo.de>> 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 <cocorossello@gmail.com <ma...@gmail.com>>:
>>>
>>> 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.
>>
>
Re: OWB 2.0.17 startup performance regression
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Current pr is mergeable for me and fix is good.
Will let Mark have a last review since he commented first the pr but looks
good to me now, thanks a lot Vincente.
Le lun. 27 juil. 2020 à 22:35, Thomas Andraschko <
andraschko.thomas@gmail.com> a écrit :
> Then +1 to remove the &&
>
> Vicente Rossello <co...@gmail.com> schrieb am Mo., 27. Juli 2020,
> 22:32:
>
>> Hi,
>>
>> Just changing outside the loop
>>
>> Boolean result = packageVetoCache.get(previousPackage);
>> if (result != null && result)
>> {
>> return result;
>> }
>>
>>
>> to
>>
>> Boolean result = packageVetoCache.get(previousPackage);
>> if (result != null)
>> {
>> return result;
>> }
>>
>>
>> solves the performance problem, the difference is unnoticeably. The real problem is that, for every class, is checking for a package-info in all "upper packages" every single time.
>>
>>
>> On Mon, Jul 27, 2020 at 10:07 PM Romain Manni-Bucau <
>> rmannibucau@gmail.com> wrote:
>>
>>> 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 <
>>> andraschko.thomas@gmail.com> 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 <ro...@gmail.com> 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 <rm...@gmail.com>
>>>>> 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 <
>>>>>> cocorossello@gmail.com> 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 <st...@yahoo.de>
>>>>>>> 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 <
>>>>>>>> cocorossello@gmail.com>:
>>>>>>>>
>>>>>>>> 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 <st...@yahoo.de>
>>>>>>>> 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 <
>>>>>>>>> cocorossello@gmail.com>:
>>>>>>>>>
>>>>>>>>> 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.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
Re: OWB 2.0.17 startup performance regression
Posted by Thomas Andraschko <an...@gmail.com>.
Then +1 to remove the &&
Vicente Rossello <co...@gmail.com> schrieb am Mo., 27. Juli 2020,
22:32:
> Hi,
>
> Just changing outside the loop
>
> Boolean result = packageVetoCache.get(previousPackage);
> if (result != null && result)
> {
> return result;
> }
>
>
> to
>
> Boolean result = packageVetoCache.get(previousPackage);
> if (result != null)
> {
> return result;
> }
>
>
> solves the performance problem, the difference is unnoticeably. The real problem is that, for every class, is checking for a package-info in all "upper packages" every single time.
>
>
> On Mon, Jul 27, 2020 at 10:07 PM Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
>> 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 <
>> andraschko.thomas@gmail.com> 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 <ro...@gmail.com> 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 <rm...@gmail.com>
>>>> 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 <
>>>>> cocorossello@gmail.com> 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 <st...@yahoo.de>
>>>>>> 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 <
>>>>>>> cocorossello@gmail.com>:
>>>>>>>
>>>>>>> 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 <st...@yahoo.de>
>>>>>>> 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 <
>>>>>>>> cocorossello@gmail.com>:
>>>>>>>>
>>>>>>>> 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.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
Re: OWB 2.0.17 startup performance regression
Posted by Vicente Rossello <co...@gmail.com>.
Hi,
Just changing outside the loop
Boolean result = packageVetoCache.get(previousPackage);
if (result != null && result)
{
return result;
}
to
Boolean result = packageVetoCache.get(previousPackage);
if (result != null)
{
return result;
}
solves the performance problem, the difference is unnoticeably. The
real problem is that, for every class, is checking for a package-info
in all "upper packages" every single time.
On Mon, Jul 27, 2020 at 10:07 PM Romain Manni-Bucau <rm...@gmail.com>
wrote:
> 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 <
> andraschko.thomas@gmail.com> 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 <ro...@gmail.com> 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 <rm...@gmail.com>
>>> 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 <co...@gmail.com>
>>>> 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 <st...@yahoo.de>
>>>>> 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 <
>>>>>> cocorossello@gmail.com>:
>>>>>>
>>>>>> 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 <st...@yahoo.de>
>>>>>> 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 <
>>>>>>> cocorossello@gmail.com>:
>>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
Re: OWB 2.0.17 startup performance regression
Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <
andraschko.thomas@gmail.com> 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 <ro...@gmail.com> 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 <rm...@gmail.com>
>> 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 <co...@gmail.com>
>>> 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 <st...@yahoo.de>
>>>> 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 <
>>>>> cocorossello@gmail.com>:
>>>>>
>>>>> 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 <st...@yahoo.de>
>>>>> 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 <
>>>>>> cocorossello@gmail.com>:
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>>
>>>>>>
>>>>>
Re: OWB 2.0.17 startup performance regression
Posted by Thomas Andraschko <an...@gmail.com>.
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 <ro...@gmail.com> 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 <rm...@gmail.com>
> 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 <co...@gmail.com>
>> 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 <st...@yahoo.de>
>>> 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 <cocorossello@gmail.com
>>>> >:
>>>>
>>>> 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 <st...@yahoo.de>
>>>> 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 <
>>>>> cocorossello@gmail.com>:
>>>>>
>>>>> 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.
>>>>>
>>>>>
>>>>>
>>>>
Re: OWB 2.0.17 startup performance regression
Posted by vicente Rossello <ro...@gmail.com>.
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 <rm...@gmail.com>
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 <co...@gmail.com>
> 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 <st...@yahoo.de> 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 <cocorossello@gmail.com
>>> >:
>>>
>>> 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 <st...@yahoo.de>
>>> 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 <cocorossello@gmail.com
>>>> >:
>>>>
>>>> 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.
>>>>
>>>>
>>>>
>>>
Re: OWB 2.0.17 startup performance regression
Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <co...@gmail.com> 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 <st...@yahoo.de> 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 <co...@gmail.com>:
>>
>> 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 <st...@yahoo.de> 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 <cocorossello@gmail.com
>>> >:
>>>
>>> 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.
>>>
>>>
>>>
>>
Re: OWB 2.0.17 startup performance regression
Posted by Vicente Rossello <co...@gmail.com>.
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 <st...@yahoo.de> 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 <co...@gmail.com>:
>
> 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 <st...@yahoo.de> 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 <co...@gmail.com>:
>>
>> 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.
>>
>>
>>
>
Re: OWB 2.0.17 startup performance regression
Posted by Mark Struberg <st...@yahoo.de>.
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 <co...@gmail.com>:
>
> 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 <struberg@yahoo.de <ma...@yahoo.de>> 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 <cocorossello@gmail.com <ma...@gmail.com>>:
>>
>> 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.
>
Re: OWB 2.0.17 startup performance regression
Posted by Vicente Rossello <co...@gmail.com>.
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 <st...@yahoo.de> 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 <co...@gmail.com>:
>
> 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.
>
>
>
Re: OWB 2.0.17 startup performance regression
Posted by Mark Struberg <st...@yahoo.de>.
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 <co...@gmail.com>:
>
> 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.