[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135736#comment-13135736
]
Simone Tripodi commented on OGNL-20:
good catch Daniel!!!
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>Assignee: Maurizio Cucchiara
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135508#comment-13135508
]
Maurizio Cucchiara commented on OGNL-20:
During the merge I had a lot of problem with Ubuntu update, and also with
Intellij Idea, which refused to merge the project (Idea merge use the approach
you said before), I had to use an alternative way which taught me a lot of
things (on top of those: don't use that approach :) ).
Sorry for your patch, I'm going to restore it.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>Assignee: Maurizio Cucchiara
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135490#comment-13135490
]
Daniel Pitts commented on OGNL-20:
--
Maurizio: I have a suggestion for next time; Merge trunk to branch, then
integrate branch to trunk.
Also, you broke the OGNL-28 commit, removing "throws OgnlException" from all
"visit" methods in NodeVisitor.java
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>Assignee: Maurizio Cucchiara
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13133952#comment-13133952
]
Simone Tripodi commented on OGNL-20:
ouch, you risked you own skin! :)
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>Assignee: Maurizio Cucchiara
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13133910#comment-13133910
]
Maurizio Cucchiara commented on OGNL-20:
Not exactly.
I was talking about the merge result, which, aside from it took me more than 2
hours, resided only in my notebook and in my mind (which is definitely less
reliable of the most broken harddisk :) )
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>Assignee: Maurizio Cucchiara
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13133894#comment-13133894
]
Simone Tripodi commented on OGNL-20:
It was anyway saved in the branch, or not? ;)
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>Assignee: Maurizio Cucchiara
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13133889#comment-13133889
]
Maurizio Cucchiara commented on OGNL-20:
I'm sorry for the missing announce. In my defense I can say that yesterday I
have encountered some problems with upgrading of my ubuntu distro.
So I was in a kind of hurry and overall scared to lost everything.
{quote}
Minor question: can I merge now the performances tests in the current /trunk?
{quote}
Sure, you can.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>Assignee: Maurizio Cucchiara
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13133846#comment-13133846
]
Simone Tripodi commented on OGNL-20:
Hola Mau,
after all the effort you put on this issue I have nothing against the merge,
but please - don't get me wrong! - for a better appliance of the Apache Way,
announce what you are going to do before, personally I didn't realize you
terminated - saw continue activity on the merge - in order to have a general
consensus.
Minor question: can I merge now the performances tests in the current
{{/trunk}}? TIA!
Thanks a lot for taking care of it!
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>Assignee: Maurizio Cucchiara
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[ https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13133777#comment-13133777 ] Hudson commented on OGNL-20: Integrated in ognl #147 (See [https://builds.apache.org/job/ognl/147/]) Added OGNL-20 in the issue list OGNL-20 - Performance - Replace synchronized blocks with ReentrantReadWriteLock mcucchiara : http://svn.apache.org/viewvc/?view=rev&rev=1188001 Files : * /commons/proper/ognl/trunk/src/changes/changes.xml mcucchiara : http://svn.apache.org/viewvc/?view=rev&rev=1188000 Files : * /commons/proper/ognl/trunk * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ASTAdd.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ASTAnd.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ASTAssign.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ASTBitAnd.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ASTBitOr.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ArrayPropertyAccessor.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/DefaultTypeConverter.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/EvaluationPool.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ExpressionNode.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/MethodAccessor.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/NodeVisitor.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ObjectArrayPool.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ObjectMethodAccessor.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/ObjectPropertyAccessor.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/Ognl.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/OgnlOps.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/OgnlRuntime.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/TypeConverter.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/enhance/ExpressionCompiler.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/Cache.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/CacheException.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/CacheFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ClassCache.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ClassCacheHandler.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ClassCacheImpl.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ConcurrentHashMapCache.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ConcurrentHashMapCacheFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ConcurrentHashMapClassCache.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/Entry.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/HashMapCache.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/HashMapCacheFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/HashMapClassCache.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ReentrantReadWriteLockCache.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ReentrantReadWriteLockCacheFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ReentrantReadWriteLockClassCache.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/CacheEntry.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/CacheEntryFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/ClassCacheEntryFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/DeclaredMethodCacheEntry.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/DeclaredMethodCacheEntryFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/FiedlCacheEntryFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/GenericMethodParameterTypeCacheEntry.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/GenericMethodParameterTypeFactory.java * /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/entry/Me
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13128550#comment-13128550
]
Maurizio Cucchiara commented on OGNL-20:
OK guys,
my green bar told me that I'm almost at the end.
I'm virtually ready to apply my changes to the trunk revision.
I ask you to check my implementation (before I'll break the code base :)) .
{quote}
How can I check which are the differences between the branch and the current
trunk?
{quote}
Simone, I'm sorry for the late (non-)answer: I think it's a matter of taste, in
Idea I usually use a kind of branch comparison, sometimes I used svn diff
directly.
Thank you everybody.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13128104#comment-13128104
]
Maurizio Cucchiara commented on OGNL-20:
Hi Daniel,
Thanks for your preciuos feedback.
This is exactly what I came to my mind.
I'm going in the following direction:
{code}
public class MethodCacheTest
{
Cache>> cache =
new ConcurrentHashMapCache>>( new MethodCacheEntryFactory());
@Test
public void testStaticGet( )
throws Exception
{
Map> methods = cache.get( new MethodCacheEntry(
Root.class,true ) );
assertNotNull( methods );
assertTrue( methods.containsKey( "getStaticInt" ) );
}
@Test
public void testNonStaticGet( )
throws Exception
{
Map> methods = cache.get( new MethodCacheEntry(
Root.class,false) );
assertNotNull( methods );
assertTrue( methods.containsKey( "format" ) );
}
}
{code}
{code}
public class MethodCacheEntry implements CacheEntry
{
private Class targetClass;
private boolean staticMethods;
public MethodCacheEntry( Class targetClass, boolean staticMethods )
{
this.targetClass = targetClass;
this.staticMethods = staticMethods;
}
@Override
public int hashCode( )
{
int result = targetClass.hashCode( );
result = 31 * result + ( staticMethods ? 1 : 0 );
return result;
}
}
{code}
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13127818#comment-13127818
]
Daniel Pitts commented on OGNL-20:
--
{quote}
Unfortunately, not everything turns out as it should, I would have like to use
flyweights, but it is not always a suitable pattern: not every cache has a so
simple "miss" condition.
Take for example getDeclaredMethods which, given a property name, returns the
list of the declared methods along the hierarchy.
Now, in this specific case, the "miss" condition is absolutely
property-dependent, it is not enough that a cache contains information about a
given class.
{quote}
In these cases, the cache key should *not* be the Class instance, but should
instead be a tuple of
This does change the existing caching semantics, but in my opinion it would
*fix* the semantics.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126518#comment-13126518
]
Simone Tripodi commented on OGNL-20:
Hi Mau,
How can I check which are the differences between the branch and the current
trunk?
TIA?
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126472#comment-13126472
]
Maurizio Cucchiara commented on OGNL-20:
Hi Daniel,
{quote}
First suggestion is ditch the ClassCacheEntryFactory interface, it doesn't do
anything useful, and prevents people from supplying CacheEntryFactory,
...> in its stead.
{quote}
Agreed
{quote}
Also, CacheEntryFactory instances are intended to be flyweights, where you've
actually been instantiating them every time you need them. For example, I
suggest refactoring getConstructors...
{quote}
Unfortunately, not everything turns out as it should, I would have like to use
flyweights, but it is not always a suitable pattern: not every cache has a so
simple "miss" condition.
Take for example {{getDeclaredMethods}} which, given a property name, returns
the list of the declared methods along the hierarchy.
Now, in this specific case, the "miss" condition is absolutely
property-dependent, it is not enough that a cache contains information about a
given class.
Even if I don't like much, I am afraid that the only solution is to use
not-anonymous classes. This strongly increases the classes's proliferation, but
it's the only solution I am able to see.
It is not easy to explain for me, I hope I did it well.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125984#comment-13125984
]
Daniel Pitts commented on OGNL-20:
--
I've just now gone over the branch, and have a couple of suggestions.
First suggestion is ditch the ClassCacheEntryFactory interface, it doesn't do
anything useful, and prevents people from supplying CacheEntryFactory,
...> in its stead.
Also, CacheEntryFactory instances are intended to be flyweights, where you've
actually been instantiating them every time you need them. For example, I
suggest refactoring getConstructors...
First, move the factory to private static final instance:
{code:title=Singleton constructor factory}
private static final CacheEntryFactory, List>>
CONSTRUCTOR_FACTORY
= new CacheEntryFactory, List>>( )
{
public List> create( Class key )
throws CacheException
{
return Arrays.asList(key.getConstructors());
}
};
{code}
Then simplify the getConstructors methods:
{code:title=Simplified getConstructors method}
public static List> getConstructors( final Class
targetClass )
throws OgnlException
{
return _constructorCache.get( targetClass, CONSTRUCTOR_FACTORY);
}
{code}
Also, if you do it this way, the factory object can be added directly to the
cache object, so you would actually only need
_constructorCache.get(targetClass), which would delegate to get(targetClass,
defaultFactory).
That's my first round of advice, I'll look at it a bit more and see if there is
anything else I can suggest.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125661#comment-13125661
]
Maurizio Cucchiara commented on OGNL-20:
Hi Daniel,
I know that, infact I am not going to include the benchmark stuff neither the
different cache implementations*
Anyway, your patch is very good.
Did you take a look at the new
[branch|http://svn.apache.org/viewvc/commons/proper/ognl/branches/new-cache-approach/]?
I would like to know your feedback.
* at least at the moment, but I would like to give to users the opportunity to
choice their preferred implementation (next step should be add custom
configuration support, dependency injection support, and above all logging).
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13125544#comment-13125544
]
Daniel Pitts commented on OGNL-20:
--
Maurizio, my patch was actually just for benchmarking the various approaches,
its far from what I'd consider good for inclusion.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124162#comment-13124162
]
Maurizio Cucchiara commented on OGNL-20:
Ok guys,
here we go, I have just committed the code on a new
[branch|http://svn.apache.org/viewvc/commons/proper/ognl/branches/new-cache-approach/].
Please let me know what do you think about.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124136#comment-13124136
]
Maurizio Cucchiara commented on OGNL-20:
Ok,
it is not simple to explain, I'm going to commit a new branch... stay tuned :)
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13123969#comment-13123969
]
Simone Tripodi commented on OGNL-20:
Hi Mau,
I need some clarifications because I am a little confused by the sneaking
exceptions inside the {{getPropertyDescriptors}} method:
{{getPropertyDescriptors(Class)}} throws {{IntrospectionException,
OgnlException}} that build a new {{ClassCacheEntryFactory}} that wraps
{{IntrospectionException, OgnlException}} in {{CacheException}}...
Should we need to change the method signature or ... ?
TIA!!!
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13123959#comment-13123959
]
Maurizio Cucchiara commented on OGNL-20:
Hi guys,
I'm trying to apply a revisited version of Daniel's ConcurrentHashMap.
During my code refactoring, I realized that CacheEntryFactory needs to throw an
exception (since it's very easy that the instantiation of an element or even
its building throws an exception) .
So I chose to add a new CacheException as a new subclass of the existing
OgnlException.
As a result, I got a general need-to-catch-exception proliferation, such that
most of the OgnlRuntime methods would have to change their signature (and so on
the other dependencies). Generally speaking, I have no problem with it, an api
which provides a custom exception sounds good to me, but I would like to know,
before I proceed, if this is the right way to follow or there is a better way.
For help my explanation, I'm attaching some pieces of code which represent the
new cache access:
{code:java}
public interface CacheEntryFactory
{
public V create( T key )
throws CacheException;
}
{code}
{code:java}
public static Map getPropertyDescriptors( final
Class targetClass )
throws IntrospectionException, **OgnlException**
{
return _propertyDescriptorCache.get( targetClass, new
ClassCacheEntryFactory>( )
{
public Map create( Class key )
throws CacheException
{
Map result = new HashMap( 101 );
PropertyDescriptor[] pda;
try
{
pda = Introspector.getBeanInfo( targetClass
).getPropertyDescriptors( );
...
}
catch ( IntrospectionException e )
{
throw new CacheException( e );
}
catch ( OgnlException e )
{
throw new CacheException( e );
}
return result;
}
} );
}
{code}
WDYT?
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13109631#comment-13109631
]
Daniel Pitts commented on OGNL-20:
--
Adrian, All I was saying is that I believe this discussion has gotten
off-topic, and therefor should move to another forum where it would be on
topic. I was not attempting to wave you off at all, only trying to keep this
discussion on topic.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13109372#comment-13109372
]
Adrian Crum commented on OGNL-20:
-
Daniel,
Keep in mind that somewhere down the road you will be asking a committer to
commit your patches to the project. That committer might have concerns about
your patch and they might offer you advice. If your response to that
committer's analysis is to wave them off and send them to another forum, then
it is unlikely your patch will get committed.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108792#comment-13108792
]
Daniel Pitts commented on OGNL-20:
--
I understand that the DCL is a broken pattern, what I suggested isn't a DCL. If
you don't understand *why* its different, then either you haven't read my posts
carefully or you don't understand why the DCL is broken. Hopefully you re-read
both that article which you posted and my posts here are well.
I fully understand what your concerns are about the other implementations, but
I disagree that they are real issues. If you stipulate in the get() contract
that the values may be different for the same key, then the contract is easier
to provide. It is also not that difficult to solve the problem you have
perceived.
Let's move on and avoid a flame war. This is not the appropriate place to have
these kinds of discussions. If you wish to continue discussing these, lets do
it somewhere more appropriate (perhaps
https://groups.google.com/group/comp.lang.java.programmer/topics?pli=1)
Adrian, thank you for being concerned about this project. I want this project
to succeed and be clean; I'm glad you do too, but please don't assume I
understand less than you. I've been writing software for a *long* time, and
have kept up on my reading.
I agree with your conclusion however that the ConcurrentHashMap *is* the
approach we want to take. My refactoring doesn't work out of the box anyway
because currently some of the "cached" values depend on more than the cache key
(which is also an anti-pattern, and should be examined in another Jira).
So, let us leave this conversation at that (use ConcurrentHashMap), which
hopefully will make both you and I happy. If you are still unhappy about the
rest of the conversation, I'll be glad to continue it
comp.lang.java.programmer. I will no long respond to such a conversation here.
Thanks,
Daniel.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108221#comment-13108221
]
Adrian Crum commented on OGNL-20:
-
That pattern (DCL) should not be used:
http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
The safest thing to do is use ConcurrentHashMap. The other two cache
implementations have concurrency issues. If you can't see what those issues
are, then perhaps it is time to re-read that book.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108188#comment-13108188
]
Daniel Pitts commented on OGNL-20:
--
I have read many times Java Concurrency in Practice, and have a lot of personal
experience with the Java Memory Model and thinking about concurrency.
I think I miscommunicated the solution I meant for the use-cases you're
concerned of.
The order of operations:
1. acquire-lock.
2. Grab item out of cache map.
3. release lock.
4. return item if not null.
5. acquire-lock.
6. Grab item again.
7. return item if not null, releasing lock.
8. Generate new item.
9. Store in cache.
10. release lock.
11. return newly generated item.
That will always return the same item, unless some other process updates the
cache map.
Again, this is a moot point since the Cache requirements don't seem to need
"singleton" status.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108134#comment-13108134
]
Adrian Crum commented on OGNL-20:
-
Daniel: The DCL design pattern does not enforce a singleton pattern, and it has
concurrency issues also. Again, please read the books I recommended.
Also, the concurrency issue I described has nothing to do with the thread
safety of the cached object. Let's use this class as an example:
{code}
public class Counter {
private int count = 0;
public synchronized void increment() {
this.count++;
}
public synchronized int getCount() {
return this.count;
}
}
{code}
An application starts two threads and each one runs the same process that gets
a Count instance from the cache and then calls increment() four times. After
the two threads terminate, the application gets a Count instance from the cache
and calls getCount(). The value returned is 4. Not what a developer would
expect (8). Yet the Count class is thread-safe.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108041#comment-13108041
]
Daniel Pitts commented on OGNL-20:
--
Thanks for your feedback Adrian, I appreciate it.
The caches aren't intended to be "singleton sources", which is what you're
saying is the problem. From what I can see, the cached data isn't manipulated,
only queried. I have *not* done an exhaustive verification. I was mostly doing
this to benchmark the speed of various approaches. Those two methods could use
a double-check to enforce singletons if that was a true requirement. This of
course would add more overhead, and they are already loosing in the benchmarks.
Another note is that mutable objects which are not thread safe will continue to
not be thread safe to access, regardless of the thread-safety of storage
method.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108027#comment-13108027
]
Adrian Crum commented on OGNL-20:
-
Daniel: It appears that you are trying to implement a generic Object cache. If
the cached objects are mutable, then the legacy cache and the read-write-lock
cache will not work - because there is the potential for more than one instance
of the object being created and returned. So, you would have threads using
different "versions" of the object. That potential problem is what the
ConcurrentHashMap.putIfAbsent method solves - it insures that only one copy of
the object is returned.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13108014#comment-13108014
]
Daniel Pitts commented on OGNL-20:
--
Adrian: You mean the patch I attached? I'm pretty sure none of them have
"concurrency issues", unless you mean contention issues. I think we've decided
on ConcurrentHashMap anyway, but I'd still like to know what the issues are you
see.
ConcurrentHashMap is the only solution which doesn't have contention issues
under high load. Relieving the contention issue does leave a sort of race
condition however. Two threads may try to create the same entry, and the work
gets done multiple times. There may be other ways around that issue too, but
my gut it is an edge case under such extreme load that it wouldn't be the first
part of the system to break-down in most cases.
The appropriate solution would be to synchronize on the key (if it is a
singleton such as Class), or create a lazy (or "Future") value which can safely
be access concurrently, and guarantees only a single thread does the work.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13107973#comment-13107973
]
Maurizio Cucchiara commented on OGNL-20:
I am not sure about it, but I think that CHM is the first one that doesn't
grant a fully reliability.
On the other hand, I think that we don't need a heavy-lock approach, in this
specific context reliability is an option.
Doesn't it?
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13107962#comment-13107962
]
Adrian Crum commented on OGNL-20:
-
Two of the approaches in the patch will have concurrency issues, only the one
using ConcurrentHashMap will provide reliable results.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13107929#comment-13107929
]
Simone Tripodi commented on OGNL-20:
Cool, thanks a lot for the effort Mau and congrats for your new house! :)
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13107919#comment-13107919
]
Maurizio Cucchiara commented on OGNL-20:
I have been very busy last week, I just finished moving into a new house.
Anyway, almost done, Simo: CHM implementation is on the way.
Daniel's work gave us a further confirm, thank you Daniel
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13107898#comment-13107898
]
Simone Tripodi commented on OGNL-20:
I personally would like to postpone eventual refactoring for future releases,
if needed, and proceeding with {{ConcurrentHasMap}} as much as possible - also
because it is the faster and simpler solution at the same time.
Is there any OGNL committer is already working on it? Maurizio, maybe?
TIA!
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
> Attachments: Bench Results.txt, Caching_Mechanism_Benchmarks.patch
>
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101057#comment-13101057
]
Simone Tripodi commented on OGNL-20:
Experienced users like you are lifeblood for OGNL, your contributions would be
very appreciated, thanks! :)
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101040#comment-13101040
]
Daniel Pitts commented on OGNL-20:
--
You explained it well enough, I was just trying to justify my concern, that's
all.
I've been swamped with my day job, otherwise I'd have performed some benchmarks
myself :-) I'm hoping that sometime in the near future I can contribute much
more than just random comments. Ognl is something I've been promoting for use
in my department, and there are a lot of things it does very well.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101013#comment-13101013
]
Simone Tripodi commented on OGNL-20:
Sorry, I just figure out that I explained so bad to get wrong - English
improvement is still in my TODO list ;)
The key point is that the current implementation is justified by historical
reasons - first OGNL release date is the far '97 - and never changed since it
was working :)
Maurizio already did some performances tests, using different cache backend
implementations, which results have to be shared, as far as I remember the
current implementation was one of the faster if not the faster.
Anyway if you want to contribute with performances tests, you are more than
welcome, and thanks for sharing your thoughts!
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101002#comment-13101002
]
Daniel Pitts commented on OGNL-20:
--
Simone, I never meant to imply it was Ognl by anyone other than smart people
:-). I'm always suspicious of using custom code over the Java collections
classes (which were also written, and maintained, by very smart people). I'm
suspicious of code *I've* written where I could have used Java collections.
Which is why I was curious about the History. If the reasoning was about
performance or some such, it may no longer be relevant (especially in light of
the concurrency issues).
In any case, it is probably a worthwhile exercise creating a benchmark to
explore the performance characteristics amongst the existing impl, and one
based on using a ConcurrentHashMap and a plain HashMap). My guess is
ConcurrentHashMap scales better, even if it might be slower for the
single-threaded use case. For a "generic" library such as Ognl, allowing the
user the ability to plug in "high concurrent" vs "high throughput" seems like a
useful feature.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13099181#comment-13099181
]
Simone Tripodi commented on OGNL-20:
Daniel, the original version of OGNL was developed by two *really smart* people
- Drew works at Apple and Luke is a team leader at Google - long long time ago
where default java collections didn't have all the features that provide today.
So, being experienced in C programming, they implemented a smarter data
structure to store cached objects.
And reading their code today I still realize there's a lot to learn from -
nothing to be suspicious! ;)
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13099054#comment-13099054
]
Daniel Pitts commented on OGNL-20:
--
I was looking through OgnlRuntime.java, and it looks like it is using Generics
already. I was basing my comments on the code pasted in these comments. Not my
best moment :-)
It looks like there are a lot of uses of the custom ClassCacheImpl too. I
wonder what the performance impact of a ConcurrentHashMap based impl (instead
of the custom hash table impl, which looks suspicious to me).
It appears the SCM import didn't capture the history of that class, I'd be
interested in finding out why it was added, but not interested enough to do
actual digging around :-)
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098778#comment-13098778
]
Maurizio Cucchiara commented on OGNL-20:
Good Morning Julien,
It would be very very appreciated.
TIA
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098779#comment-13098779
]
Maurizio Cucchiara commented on OGNL-20:
Good Morning Julien,
It would be very very appreciated.
TIA
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098773#comment-13098773
]
Julien Aymé commented on OGNL-20:
-
Hi all,
I will provide both a small benchmark test and a patch using ConcurrentHashMap
and Generics, starting from the existing code.
The patch will be ready in 4 hours or so (depending on the free time I can get
at lunch :-) )
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098677#comment-13098677
]
Simone Tripodi commented on OGNL-20:
the OGNL JVM target is 1.5, see
[pom.xml|https://svn.apache.org/repos/asf/commons/proper/ognl/trunk/pom.xml]:
{code}
1.5
1.5
{code}
So we are lucky enough to just put {{ConcurrentHashMap}}
Generics is something still in progress, I'll do a new check as soon as I get a
new slot of spare time - but please don't forget that patches are much more
than welcome and encouraged!
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098362#comment-13098362
]
Daniel Pitts commented on OGNL-20:
--
Do keep in mind that the ConcurrentHashMap is new in Java 1.5. If you are
targetting older JDK/JVM, you'll need to have other strategies that can work
for those too (with perhaps degraded concurrency performance). Relatedly, if
you *are* targeting 1.5+, then you should be using generics here, and not raw
types, eg:
"Map cache = new ConcurrentHashMap();"
I do like the idea of using ConcurrentHashMap though.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098161#comment-13098161
]
Simone Tripodi commented on OGNL-20:
OK for that case you convinced me, thanks :)
Looking forward the whole code modification...
@Izio: new interfaces are not needed, just new implementations that replace
existing behaviors.
@Julien: there was an era when my French was good, unfortunately didn't have so
much chances to use it, so started to forget :(
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098134#comment-13098134
]
Maurizio Cucchiara commented on OGNL-20:
Julien,
it looks like one of us is able to read people's mind :)
Anyway, what about if we start, as I said before, from a test bench.
We could code a new cache interface (or reutilize the existing [ClassCache
one|https://svn.apache.org/viewvc/commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/internal/ClassCache.java?view=markup])
and provide different implementations and see what happens.
What do you thing, guys?
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098110#comment-13098110
]
Julien Aymé commented on OGNL-20:
-
Simone,
The complexity of lock/unlock operation is O(1) I think. But there is a cost
nonetheless.
The
[ConcurrentHashMap|http://download.oracle.com/javase/6/docs/api/java/util/concurrent/ConcurrentHashMap.html]
class javadoc explains the mechanism of its behavior, including:
{quote}
Retrieval operations (including get) generally do not block, so may overlap
with update operations (including put and remove).
...
The table is internally partitioned to try to permit the indicated number of
concurrent updates without contention.
{quote}
Also, the proposed code is rather:
{code}
Map _methodParameterTypesCache = new ConcurrentHashMap();
...
Class[] result;
if ( ( result = (Class[]) _methodParameterTypesCache.get( m )) == null )
{
_methodParameterTypesCache.putIfAbsent( m, result = m.getParameterTypes()
);
}
return result;
{code}
in which the {{m.getParameterTypes()}} is invoked only once per thread and only
as many times as there is concurrent threads trying to get the same value not
already computed (for the same key).
The whole point is that in a really ultra heavy environment (hundreds of
concurrent threads) the cache still behaves efficiently (a value computed in
one thread will be shared to others) without becoming a huge bottleneck (which
will happen when using either lock or synchronized block)
HTH,
De rien !!
Julien
P.S.: In french, we usually say "merci d'avance" for "thanks in advance" ;-)
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098108#comment-13098108
]
Maurizio Cucchiara commented on OGNL-20:
Hi Simone,
to answer your questions...
{quote}
Which complexity has lock/unlock objects? I'm asking because I don't know.
{quote}
I am not sure I understood your question, I guess that lock and unlock are
atomic.
{quote}
Can you point me please to ConcurrentHashMap doc wich explain how locks only
small part of itself and not the whole data?
{quote}
Please, take a look at the
[implementation|http://www.google.it/codesearch#-WpwJU0UKqQ/src/share/classes/java/util/concurrent/ConcurrentHashMap.java&q=ConcurrentHashMap&type=cs&l=242],
you will see that, internally, CHM uses (and hence locks) a segment as a unit
of work (where segment is a small subset of the whole map).
{quote}
except that, for what I can see, the m.getParameterTypes() that worries you, in
the first implementation is invoked only when the key is not found, in the
second is always invoked.
{quote}
you are right, we should clarify how much cost the computation of the final
object value.
Anyway I would not unwrap the {{putIfAbsent}} invocation from the existence
check:
{code}
Map _methodParameterTypesCache = new ConcurrentHashMap();
if ( ( result = (Class[]) _methodParameterTypesCache.get( m )) == null )
{
_methodParameterTypesCache.puIfAbsent( m, result =
m.getParameterTypes() );
}
return result;
{code}
There are many other aspect to take into account: for example CHM doesn't
support null value.
from the javadoc:
{quote}
Like Hashtable but unlike HashMap, this class does not allow null to be used as
a key or value.
{quote}
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098073#comment-13098073
]
Simone Tripodi commented on OGNL-20:
Julien,
Which complexity has lock/unlock objects? I'm asking because I don't know.
Can you point me please to {{ConcurrentHashMap}} doc wich explain how locks
only small part of itself and not the whole data?
Of course, Viva ultra-specialized ultra-fast concurrent collection, I just want
to make sure that, before one of us starts doing the leg-work of moving to the
new implementation, it would really add benefits to OGNL performances.
ATM I don't see much difference from
{code}
Map _methodParameterTypesCache = new HashMap();
...
synchronized (_methodParameterTypesCache)
{
Class[] result;
if ( ( result = (Class[]) _methodParameterTypesCache.get( m )) == null )
{
_methodParameterTypesCache.put( m, result = m.getParameterTypes() );
}
return result;
}
{code}
and
{code}
Map _methodParameterTypesCache = new ConcurrentHashMap();
...
Class[] result = _methodParameterTypesCache.puIfAbsent( m,
m.getParameterTypes() );
return result;
{code}
except that, for what I can see, the {{m.getParameterTypes()}} that worries
you, in the first implementation is invoked only when the key is not found, in
the second is always invoked.
Please provide me the info I miss, merci en avant!
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098040#comment-13098040
]
Julien Aymé commented on OGNL-20:
-
Hi Simone !
You got it right for the main part, except that the most costly operation in
the algorithm is the computation (or it is assumed to be so): The cost of
accessing / comparing / puting in the map is nothing compared to the
computation itself. That's why, the point to remove any synchronized block at
all is to avoid the cost of the computation itself (in other threads of course,
not in the thread which does the computation), not the cost of locking the map.
In the example you gave, the computation is:
{code}
m.getParameterTypes()
{code}
which may not be very costly... But in general, you get the idea.
Plus, ConcurrentHashMap behaves way better than using a read/write lock with an
HashMap (due to the fact that the CHM locks only small part of itself, not the
whole map data. Viva ultra-specialized ultra-fast concurrent collection ! :-) )
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098029#comment-13098029
]
Simone Tripodi commented on OGNL-20:
Salut Julien!
of course I understand the reasons - especially #3 is crucial - and yes
{{puIfAbsent}} would fix the concurrency issue.
Anyway, getting/setting to/from Maps always has complexity O(1), comparing null
objects has complexity O(1) - are we sure, in that specific case, that avoid
the use of a lock improves the performances so much? Moreover, reading from
{{puIfAbsent}} javadoc:
{quote}
If the specified key is not already associated with a value, associate it with
the given value. This is equivalent to
if (!map.containsKey(key))
return map.put(key, value);
else
return map.get(key);
except that the action is performed atomically.
{quote}
that doesn't look so different from the {{synchronized}} code above - unless
you tell me that {{puIfAbsent}} atomicity is implemented using
super-internal-APIs that I don't know - and avoid the lock, that's the critical
point - and for wich I apologize in advance for my ignorance.
Concluding: agreed on replacing data structures to increase code
maintainability and readability - patches are welcome and I invite you to
submit! - not sure to see performances increased in that specific case.
Good catch!
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13098009#comment-13098009
]
Julien Aymé commented on OGNL-20:
-
@Maurizio:
Yes, putIfAbsent is a good idea !
@Simone,
you are not wrong, the two cases do have different semantics.
The thing is, we don't really care if some threads do the same computation at
the same time.
What we really want is to speed up every thread in the long run (hence the
caching) :
- 1. avoid computation twice for the same thread
- 2. ensure that any computed object is available for other threads : the use
of ConcurrentHashMap allows exactly that
- 3. avoid blocking another thread which may not require the same object (if
distinct key) : to do so, if we can avoid using any synchronized block or any
lock it is better !
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13097998#comment-13097998
]
Simone Tripodi commented on OGNL-20:
+1 to {{ReentrantReadWriteLocks}}.
IMHO
{code}
Map _methodParameterTypesCache = new HashMap();
synchronized (_methodParameterTypesCache)
{
Class[] result;
if ( ( result = (Class[]) _methodParameterTypesCache.get( m )) == null )
{
_methodParameterTypesCache.put( m, result = m.getParameterTypes() );
}
return result;
}
{code}
and
{code}
Map _methodParameterTypesCache = new HashMap();
Class[] result;
if ( ( result = (Class[]) _methodParameterTypesCache.get(m) ) == null )
{
_methodParameterTypesCache.put( m, result = m.getParameterTypes() );
}
return result;
{code}
have totally different semantics. In one case, you synchronized the whole block
- including the checks - in the second one, just limited the map accesses.
Am I wrong? If yes, why?
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13097868#comment-13097868
]
Adrian Crum commented on OGNL-20:
-
I highly recommend reading Java Concurrency In Practice
(http://www.amazon.com/Java-Concurrency-Practice-Brian-Goetz/dp/0321349601/ref=sr_1_1?ie=UTF8&qid=1315303458&sr=8-1)
and Concurrent Programming In Java
(http://www.amazon.com/Concurrent-Programming-Java-Principles-Pattern/dp/0201310090/ref=sr_1_1?s=books&ie=UTF8&qid=1315303737&sr=1-1)
- a book written by the author of ConcurrentHashMap.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13097814#comment-13097814
]
Maurizio Cucchiara commented on OGNL-20:
Hi Julien,
that was my original thought, furthermore we could use putIfAbsent method of
CHM.
What do you think, guys?
Anyway we should discuss about it in just one issue.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13097809#comment-13097809
]
Julien Aymé commented on OGNL-20:
-
Note that you also could use ConcurrentHashMap instead of HashMap. Also, since
in many place it is not harmful if the cached object is evaluated twice, you
can remove the whole synchronized block:
cache = new ConcurrentHashMap();
Object cachedObject = cache.get(key);
if (null == cachedObject) {
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13097795#comment-13097795
]
Maurizio Cucchiara commented on OGNL-20:
Hi Olivier,
be aware that there is already:
# a [patch|WW-3580] which sensibly reduces the synchronized code using
something like the double check idiom.
# an old [opensymphony patch|http://jira.opensymphony.com/browse/OGNL-101]
which, if I recall correctly, uses concurrent version of the map implementation.
It would be very interesting to compare this 3 different approaches. Before
that, I think we would need a performance test bench.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13097791#comment-13097791
]
Olivier Lamy commented on OGNL-20:
--
good idea ! I will try a patch or a branch with that.
> Performance - Replace synchronized blocks with ReentrantReadWriteLock
> -
>
> Key: OGNL-20
> URL: https://issues.apache.org/jira/browse/OGNL-20
> Project: OGNL
> Issue Type: Improvement
> Environment: ALL
>Reporter: Greg Lively
>
> I've noticed a lot of synchronized blocks of code in OGNL. For the most part,
> these synchronized blocks are controlling access to HashMaps, etc. I believe
> this could be done far better using ReentrantReadWriteLocks.
> ReentrantReadWriteLock allows unlimited concurrent access, and single threads
> only for writes. Perfect in an environment where the ratio of reads is far
> higher than writes; which is typically the scenario for caching. Plus the
> access control can be tuned for reads and writes; not just a big
> synchronized{} wrapping a bunch of code.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
