You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacopo Cappellato <ja...@gmail.com> on 2014/09/11 18:46:43 UTC

Removing the CachedClassLoader

Hi all,

after I spent a good amount of time studying how Java class loaders works and are setup in OFBiz I realized that there was room for improving some old code and simplify it a lot by removing the old and custom CachedClassLoader that is created for each of the OFBiz web applications.

I am pretty confident that this change will work well and that will make the framework code that manages classloaders easier to read and maintain, and will also be a small step in the direction of making OFBiz easier to deploy on external application servers, with no penalties for performance.
My preference would be to commit the code in the trunk (no API changes or other impact for the applications and external configuration) and then let you review my work.
If, on the other hand, you prefer I submit a patch to Jira, I will be happy too.

Please let me know.

Jacopo


Re: Removing the CachedClassLoader

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Japopo,

By and large, for relative large amount of changes I believe we should better share before committing. Anyway we need to create Jira issues with our 
new release notes/change log policy
On the other hand if you are pretty confident with your changes, why not a direct commit indeed?

Jacques

Le 11/09/2014 18:46, Jacopo Cappellato a écrit :
> Hi all,
>
> after I spent a good amount of time studying how Java class loaders works and are setup in OFBiz I realized that there was room for improving some old code and simplify it a lot by removing the old and custom CachedClassLoader that is created for each of the OFBiz web applications.
>
> I am pretty confident that this change will work well and that will make the framework code that manages classloaders easier to read and maintain, and will also be a small step in the direction of making OFBiz easier to deploy on external application servers, with no penalties for performance.
> My preference would be to commit the code in the trunk (no API changes or other impact for the applications and external configuration) and then let you review my work.
> If, on the other hand, you prefer I submit a patch to Jira, I will be happy too.
>
> Please let me know.
>
> Jacopo
>
>
>

Re: Removing the CachedClassLoader

Posted by Jacopo Cappellato <ja...@gmail.com>.
Committed with rev. 1624549; unit tests are included.
For easy reference of reviewers and to prepare a changelog for future releases I have also created a Jira ticket (now resolved):
https://issues.apache.org/jira/browse/OFBIZ-5768

Jacopo

On Sep 12, 2014, at 3:09 AM, Hans Bakker <ma...@antwebsystems.com> wrote:

> Jacopo,
> 
> I am fine with these changes as long as junit tests are provided, i feel pretty strong about this. Going to continuous deployment is the way to go. For your information I just wrote a linkedin post about that:
> 
> https://www.linkedin.com/pulse/article/20140909060033-1227556-upgrade-your-erp-system-like-a-phone-app
> 
> I know this is a controversial subject in this mailing list, still very important going with this rather new development.
> 
> Regards,
> Hans
> 
> 
> On 11/09/14 23:46, Jacopo Cappellato wrote:
>> Hi all,
>> 
>> after I spent a good amount of time studying how Java class loaders works and are setup in OFBiz I realized that there was room for improving some old code and simplify it a lot by removing the old and custom CachedClassLoader that is created for each of the OFBiz web applications.
>> 
>> I am pretty confident that this change will work well and that will make the framework code that manages classloaders easier to read and maintain, and will also be a small step in the direction of making OFBiz easier to deploy on external application servers, with no penalties for performance.
>> My preference would be to commit the code in the trunk (no API changes or other impact for the applications and external configuration) and then let you review my work.
>> If, on the other hand, you prefer I submit a patch to Jira, I will be happy too.
>> 
>> Please let me know.
>> 
>> Jacopo
>> 
> 


Re: Removing the CachedClassLoader

Posted by Hans Bakker <ma...@antwebsystems.com>.
Jacopo,

I am fine with these changes as long as junit tests are provided, i feel 
pretty strong about this. Going to continuous deployment is the way to 
go. For your information I just wrote a linkedin post about that:

https://www.linkedin.com/pulse/article/20140909060033-1227556-upgrade-your-erp-system-like-a-phone-app

I know this is a controversial subject in this mailing list, still very 
important going with this rather new development.

Regards,
Hans


On 11/09/14 23:46, Jacopo Cappellato wrote:
> Hi all,
>
> after I spent a good amount of time studying how Java class loaders works and are setup in OFBiz I realized that there was room for improving some old code and simplify it a lot by removing the old and custom CachedClassLoader that is created for each of the OFBiz web applications.
>
> I am pretty confident that this change will work well and that will make the framework code that manages classloaders easier to read and maintain, and will also be a small step in the direction of making OFBiz easier to deploy on external application servers, with no penalties for performance.
> My preference would be to commit the code in the trunk (no API changes or other impact for the applications and external configuration) and then let you review my work.
> If, on the other hand, you prefer I submit a patch to Jira, I will be happy too.
>
> Please let me know.
>
> Jacopo
>


Re: Removing the CachedClassLoader

Posted by Jacopo Cappellato <ja...@gmail.com>.
David,

thank you for your valuable input that confirms my assumptions and understanding about the CachedClassLoader code; before I proposed this change to the community, we indeed did some load testing benchmarking (with and without CCL) and we didn't noticed significant differences; however we are currently performing further and more accurate tests and we will report any issue we may find.
Of course I would love to get the feedback from others' tests too.

Jacopo

On Sep 17, 2014, at 7:42 AM, David E. Jones <de...@me.com> wrote:

> 
> It may be fine to remove this, but I'd still recommend doing some performance tests with and without to make sure the impact isn't significant.
> 
> JVMs are much better these days about class loading, but 10 years ago that was not the case. The caching classloader alone resulted in something like a 20x performance increase for various things in OFBiz, mostly because OFBiz uses Java reflection so much to do things like run Java object methods for services and request events. 
> 
> In modern JVMs the performance different may be insignificant, but I'm not sure about that... some testing a couple years ago I still found a difference, though it wasn't as dramatic.
> 
> -David
> 
> 
> On Sep 11, 2014, at 9:56 AM, Adrian Crum <ad...@sandglass-software.com> wrote:
> 
>> The CachedClassLoader was another "performance improvement" introduced by David. I have always suspected (but never investigated thoroughly) that the system class loader does some class caching on its own.
>> 
>> I agree that class loading could be handled better - to solve jar version conflicts for example. If I have a newer/older jar in my application, the class loader should find it first, before looking in the OFBiz OOTB jar files.
>> 
>> Anything that can be done to improve the class loader arrangement will be wonderful.
>> 
>> Adrian Crum
>> Sandglass Software
>> www.sandglass-software.com
>> 
>> On 9/11/2014 5:46 PM, Jacopo Cappellato wrote:
>>> Hi all,
>>> 
>>> after I spent a good amount of time studying how Java class loaders works and are setup in OFBiz I realized that there was room for improving some old code and simplify it a lot by removing the old and custom CachedClassLoader that is created for each of the OFBiz web applications.
>>> 
>>> I am pretty confident that this change will work well and that will make the framework code that manages classloaders easier to read and maintain, and will also be a small step in the direction of making OFBiz easier to deploy on external application servers, with no penalties for performance.
>>> My preference would be to commit the code in the trunk (no API changes or other impact for the applications and external configuration) and then let you review my work.
>>> If, on the other hand, you prefer I submit a patch to Jira, I will be happy too.
>>> 
>>> Please let me know.
>>> 
>>> Jacopo
>>> 
> 


Re: Removing the CachedClassLoader

Posted by Adrian Crum <ad...@sandglass-software.com>.
Thanks David!

To be clear: If I am skeptical about some of these legacy performance 
improvements, it is precisely for the reason you stated - the JVM was 
much slower back when some of them were created, and there is a chance 
we don't need them any more.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 9/17/2014 6:42 AM, David E. Jones wrote:
>
> It may be fine to remove this, but I'd still recommend doing some performance tests with and without to make sure the impact isn't significant.
>
> JVMs are much better these days about class loading, but 10 years ago that was not the case. The caching classloader alone resulted in something like a 20x performance increase for various things in OFBiz, mostly because OFBiz uses Java reflection so much to do things like run Java object methods for services and request events.
>
> In modern JVMs the performance different may be insignificant, but I'm not sure about that... some testing a couple years ago I still found a difference, though it wasn't as dramatic.
>
> -David
>
>
> On Sep 11, 2014, at 9:56 AM, Adrian Crum <ad...@sandglass-software.com> wrote:
>
>> The CachedClassLoader was another "performance improvement" introduced by David. I have always suspected (but never investigated thoroughly) that the system class loader does some class caching on its own.
>>
>> I agree that class loading could be handled better - to solve jar version conflicts for example. If I have a newer/older jar in my application, the class loader should find it first, before looking in the OFBiz OOTB jar files.
>>
>> Anything that can be done to improve the class loader arrangement will be wonderful.
>>
>> Adrian Crum
>> Sandglass Software
>> www.sandglass-software.com
>>
>> On 9/11/2014 5:46 PM, Jacopo Cappellato wrote:
>>> Hi all,
>>>
>>> after I spent a good amount of time studying how Java class loaders works and are setup in OFBiz I realized that there was room for improving some old code and simplify it a lot by removing the old and custom CachedClassLoader that is created for each of the OFBiz web applications.
>>>
>>> I am pretty confident that this change will work well and that will make the framework code that manages classloaders easier to read and maintain, and will also be a small step in the direction of making OFBiz easier to deploy on external application servers, with no penalties for performance.
>>> My preference would be to commit the code in the trunk (no API changes or other impact for the applications and external configuration) and then let you review my work.
>>> If, on the other hand, you prefer I submit a patch to Jira, I will be happy too.
>>>
>>> Please let me know.
>>>
>>> Jacopo
>>>
>

Re: Removing the CachedClassLoader

Posted by "David E. Jones" <de...@me.com>.
It may be fine to remove this, but I'd still recommend doing some performance tests with and without to make sure the impact isn't significant.

JVMs are much better these days about class loading, but 10 years ago that was not the case. The caching classloader alone resulted in something like a 20x performance increase for various things in OFBiz, mostly because OFBiz uses Java reflection so much to do things like run Java object methods for services and request events. 

In modern JVMs the performance different may be insignificant, but I'm not sure about that... some testing a couple years ago I still found a difference, though it wasn't as dramatic.

-David


On Sep 11, 2014, at 9:56 AM, Adrian Crum <ad...@sandglass-software.com> wrote:

> The CachedClassLoader was another "performance improvement" introduced by David. I have always suspected (but never investigated thoroughly) that the system class loader does some class caching on its own.
> 
> I agree that class loading could be handled better - to solve jar version conflicts for example. If I have a newer/older jar in my application, the class loader should find it first, before looking in the OFBiz OOTB jar files.
> 
> Anything that can be done to improve the class loader arrangement will be wonderful.
> 
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
> 
> On 9/11/2014 5:46 PM, Jacopo Cappellato wrote:
>> Hi all,
>> 
>> after I spent a good amount of time studying how Java class loaders works and are setup in OFBiz I realized that there was room for improving some old code and simplify it a lot by removing the old and custom CachedClassLoader that is created for each of the OFBiz web applications.
>> 
>> I am pretty confident that this change will work well and that will make the framework code that manages classloaders easier to read and maintain, and will also be a small step in the direction of making OFBiz easier to deploy on external application servers, with no penalties for performance.
>> My preference would be to commit the code in the trunk (no API changes or other impact for the applications and external configuration) and then let you review my work.
>> If, on the other hand, you prefer I submit a patch to Jira, I will be happy too.
>> 
>> Please let me know.
>> 
>> Jacopo
>> 


Re: Removing the CachedClassLoader

Posted by Adrian Crum <ad...@sandglass-software.com>.
The CachedClassLoader was another "performance improvement" introduced 
by David. I have always suspected (but never investigated thoroughly) 
that the system class loader does some class caching on its own.

I agree that class loading could be handled better - to solve jar 
version conflicts for example. If I have a newer/older jar in my 
application, the class loader should find it first, before looking in 
the OFBiz OOTB jar files.

Anything that can be done to improve the class loader arrangement will 
be wonderful.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 9/11/2014 5:46 PM, Jacopo Cappellato wrote:
> Hi all,
>
> after I spent a good amount of time studying how Java class loaders works and are setup in OFBiz I realized that there was room for improving some old code and simplify it a lot by removing the old and custom CachedClassLoader that is created for each of the OFBiz web applications.
>
> I am pretty confident that this change will work well and that will make the framework code that manages classloaders easier to read and maintain, and will also be a small step in the direction of making OFBiz easier to deploy on external application servers, with no penalties for performance.
> My preference would be to commit the code in the trunk (no API changes or other impact for the applications and external configuration) and then let you review my work.
> If, on the other hand, you prefer I submit a patch to Jira, I will be happy too.
>
> Please let me know.
>
> Jacopo
>