commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Pitts (Commented) (JIRA)" <>
Subject [jira] [Commented] (OGNL-20) Performance - Replace synchronized blocks with ReentrantReadWriteLock
Date Wed, 12 Oct 2011 17:29:12 GMT


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<?>>>
            = new CacheEntryFactory<Class<?>, List<Constructor<?>>>(
        public List<Constructor<?>> create( Class<?> key )
            throws CacheException
            return Arrays.asList(key.getConstructors());

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);

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:
>             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:!default.jspa
For more information on JIRA, see:


View raw message