jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stefan Guggisberg (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JCR-731) Can the caching mechanism be improved?
Date Mon, 05 Feb 2007 15:49:06 GMT

    [ https://issues.apache.org/jira/browse/JCR-731?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12470254

Stefan Guggisberg commented on JCR-731:

first of all, the patch looks good in general, great job martijn!

however, a few comments:

the hasItemState() method was introduced as a performance improvement in order to avoid having
to catch 
NoSuchItemStateException when checking existence using getItemState() method. your suggestion

re-introduces catching exceptions...

the patch is hardly trivial and affects crucial core components. especially MLRUItemStateCache.shrinkIfRequired()
and CacheManager.resizeAll() are significantly more complex and bear the risk of introducing
new issues/bottlenecks. 

code style:
there are some (minor) isssues related to code style. the patch uses tab characters rather
than spaces
for indenting. some changes were pure reformatting (replacing spaces by tabs) which makes
it unnecessary 
hard to read the patch.  another issue is operator padding: we use spaces around operators,
"if (x >= 0) " rather than "if (x>=0)".

before applying the suggested changes i'd like to see some simple benchmark results/simple
test cases that show
 the overall performance imrovement.

again thanks for your contribution!


> Can the caching mechanism be improved?
> --------------------------------------
>                 Key: JCR-731
>                 URL: https://issues.apache.org/jira/browse/JCR-731
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: core
>            Reporter: Martijn Hendriks
>         Attachments: CacheManager.java, JCR-731.diff, MLRUItemStateCache.java, SharedItemStateManager.java
> Hi all,
> We've identified the method "getNonVirtualItemState" in the SharedItemStateManager as
a hot spot in our application. To avoid the contention in "getNonVirtualItemState", we have
pulled the isCached call out of the synchronized block and re-implemented the MLRUItemStateCache.
It uses a HashMap that contains the ItemId-ItemState mapping and a ReadWriteLock to replace
all synchronized blocks in the code. The implementation of "shrinkIfRequired" unfortunatly
got much more expensive as the entryset of the HashMap must be sorted by accesscount. This
method then clearly is a bottleneck. We solved this by changing the CacheManager a bit: the
"resizeAll" method avoids the eviction of items out of caches as long as possible.
> These changes work out really well for our application. I have attached the changed files;
comments/feedback are very welcome!
> Regards,
> Martijn Hendriks
> <GX> creative online development B.V.
> t: 024 - 3888 261
> f: 024 - 3888 621
> e: martijnh@gx.nl
> Wijchenseweg 111
> 6538 SW Nijmegen
> http://www.gx.nl/ 

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message