[
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<Class<?>,
...> 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<Class<?>, List<Constructor<?>>>
CONSTRUCTOR_FACTORY
= new CacheEntryFactory<Class<?>, List<Constructor<?>>>( )
{
public List<Constructor<?>> create( Class<?> key )
throws CacheException
{
return Arrays.asList(key.getConstructors());
}
};
{code}
Then simplify the getConstructors methods:
{code:title=Simplified getConstructors method}
public static List<Constructor<?>> 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
