[
https://issues.apache.org/jira/browse/OFBIZ-4282?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13037522#comment-13037522
]
Philippe Mouawad edited comment on OFBIZ-4282 at 5/22/11 7:51 AM:
------------------------------------------------------------------
Hello M. Jones,
First let met describe to you how/when I propose an optimisation:
1) We track down in our production CPU and Contention points
2) We make thread dumps
Then analyse them.
We make a load test on a platform that mimics the production with only the
change and track response times, CPU and monitor changes.
We also profile ofbiz on load test platform with some well known Profiling
tools I won't put here.
Then to answer your remarks:
1) I am sorry but I don't agree on the fact that making the method not
synchronized, the timeout for a transaction would bleed over into other
transaction, timeout is a parameter so local to the call, synchro would be
required if timeout was instance variable (and it wouldn't anyway protect from
bleed) or static without being thread local. I think synchronized was here at
the start because there were some instance and static variables in
TransactionUtil that were not thread safe, but there were some contributions
(one of mine if I remember well ?) that fixed the issues to make them Thread
Local or use synchronized Collections so this is not more useful. This
modification is in production for more than a year now and Contention point was
solved.
2) I agree with you that this feature is helpful for finding the source of
transaction timeout BUT consider its cost on a web site with 700 Transaction
Hits/s (which is my case), we get 700 Exceptions created each second just for a
potential issue, that's why I was saying that disabling it should be the
default and proposing method to enable it thrhough webadmin once you have
timeout transaction or through a property is better. Once again at the time we
saw CPU on production servers drop from 5%.
Anyway I repeat that these 2 fixes were HARDLY tested on preproduction then
production (where synchronisation issues have the best chance to occur) since a
year now.
Regards
Philippe
was (Author: [email protected]):
Hello M. Jones,
First let met describe to you how/when I propose an optimisation:
1) We track down in our production CPU and Contention points
2) We make thread dumps
Then analyse them.
We make a load test on a platform that mimics the production with only the
change and track response times, CPU and monitor changes.
We also profile ofbiz on load test platform with some well known Profiling
tools I won't put here.
Then to answer your remarks:
1) I am sorry but I don't agree on the fact that making the method synchronized
the timeout for a transaction would bleed over into other transaction, timeout
is a parameter so local to the call, synchro would be required if timeout was
instance variable (and it wouldn't anyway protect from bleed, or static without
being thread local). This modification is in production for more than a year
now and Contention point was solved. I think synchronized was here at the start
because there were some instance and static variables that were not thread
safe, but there were some contribution (one of mine) that fixed the issue to
make them Thread Local or synchronized so this is no more useful
2) I agree with you that this feature is helpful for finding the source of
transaction timeout BUT consider its cost on a web site with 700 Transaction
Hits/s (which is my case), we get 700 Exceptions created each second just for a
potential issue, that's why I was saying that disabling it should be the
default and proposing method to enable it thrhough webadmin once you have
timeout transaction or through a property is better
Anyway I repeat that these 2 fixes were HARDLY tested on production (where
synchronisation issues have the best chance to occur) since a year now.
Regards
Philippe
> TransactionUtil performance optimisations
> -----------------------------------------
>
> Key: OFBIZ-4282
> URL: https://issues.apache.org/jira/browse/OFBIZ-4282
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Affects Versions: SVN trunk
> Reporter: Philippe Mouawad
> Labels: PERFORMANCE
> Attachments: patch-OFBIZ-4282.patch
>
>
> Hello,
> Reviewing TransactionUtil code, I have seen 2 problems:
> - internalBegin is uselessly synchronized , since it is a static method it is
> a very big useless Contention Point since not unthread safe instance variable
> is used
> - debugResources is true which creates a DebugXAResource (that creates an
> Exception) , it should be false and made an option for debuging
> These 2 modifications have been in our production for a while and we noticed
> CPU reduction and no more contention on TransactionUtil#begin
> Regards
> Philippe Mouawad
> http://www.ubik-ingenierie.com
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
