[jira] [Issue Comment Edited] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13128104#comment-13128104
]
Maurizio Cucchiara edited comment on OGNL-20 at 10/15/11 8:19 AM:
--
Hi Daniel,
Thanks for your preciuos feedback.
This is exactly what came to my mind (which, IIUC, is more or less what you
suggested before).
I'm going in the following direction:
{code}
public class MethodCacheTest
{
CacheMethodCacheEntry, MapString, ListMethod cache =
new ConcurrentHashMapCacheMethodCacheEntry,MapString,
ListMethod( new MethodCacheEntryFactory());
@Test
public void testStaticGet( )
throws Exception
{
MapString, ListMethod methods = cache.get( new MethodCacheEntry(
Root.class,true ) );
assertNotNull( methods );
assertTrue( methods.containsKey( getStaticInt ) );
}
@Test
public void testNonStaticGet( )
throws Exception
{
MapString, ListMethod 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}
was (Author: maurizio.cucchiara):
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
{
CacheMethodCacheEntry, MapString, ListMethod cache =
new ConcurrentHashMapCacheMethodCacheEntry,MapString,
ListMethod( new MethodCacheEntryFactory());
@Test
public void testStaticGet( )
throws Exception
{
MapString, ListMethod methods = cache.get( new MethodCacheEntry(
Root.class,true ) );
assertNotNull( methods );
assertTrue( methods.containsKey( getStaticInt ) );
}
@Test
public void testNonStaticGet( )
throws Exception
{
MapString, ListMethod 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] [Issue Comment Edited] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13097795#comment-13097795
]
Maurizio Cucchiara edited comment on OGNL-20 at 9/6/11 8:21 AM:
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.
was (Author: maurizio.cucchiara):
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] [Issue Comment Edited] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13097809#comment-13097809
]
Julien Aymé edited comment on OGNL-20 at 9/6/11 8:50 AM:
-
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:
{code}
// When initializing the cache:
cache = new ConcurrentHashMap();
// Looking up into the cache
Object cachedObject = cache.get(key);
if (null == cachedObject) {
// No need to use a synchronized block here if we don't care that the
getRealObject method is invoked more than once.
Object realObject = getRealObject(key);
cache.put(key, realObject);
cachedObject = realObject;
}
return cachedObject;
{code}
As shown above, the use of a ConcurrentHashMap allows to discard any
synchronized block, and any custom lock use.
HTH,
Regards,
Julien
was (Author: julien.a...@gmail.com):
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:
code
cache = new ConcurrentHashMap();
Object cachedObject = cache.get(key);
if (null == cachedObject) {
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
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] [Issue Comment Edited] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
[
https://issues.apache.org/jira/browse/OGNL-20?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13097998#comment-13097998
]
Simone Tripodi edited comment on OGNL-20 at 9/6/11 1:30 PM:
+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 ConcurrentHashMap();
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?
was (Author: simone.tripodi):
+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
