Mailing-List: contact dev-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@jackrabbit.apache.org
Date: Thu, 2 Mar 2017 15:57:45 +0000 (UTC)
From: "Benjamin Papez (JIRA)" <jira@apache.org>
To: dev@jackrabbit.apache.org
Message-ID: <JIRA.13047629.1488448411000.43863.1488470265233@Atlassian.JIRA>
In-Reply-To: <JIRA.13047629.1488448411000@Atlassian.JIRA>
References: <JIRA.13047629.1488448411000@Atlassian.JIRA> <JIRA.13047629.1488448411467@jira-lw-us.apache.org>
Subject: [jira] [Commented] (JCR-4116)
 SharedItemStateManager.getNonVirtualItemState is over-synchronized
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Thu, 02 Mar 2017 15:57:49 -0000


    [ https://issues.apache.org/jira/browse/JCR-4116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15892453#comment-15892453 ] 

Benjamin Papez commented on JCR-4116:
-------------------------------------

Thanks once more. So at least there cannot be an inconsistency. Still it looks like the synchronization is quite a bottleneck here. In a thread-dump I looked at today 246 threads are within {{getNonVirtualItemState}}:  
229 threads are waiting in {{synchronized (this)}},
16 threads are on the {{wait()}} and
1 thread is in {{notifyAll}}

> SharedItemStateManager.getNonVirtualItemState is over-synchronized
> ------------------------------------------------------------------
>
>                 Key: JCR-4116
>                 URL: https://issues.apache.org/jira/browse/JCR-4116
>             Project: Jackrabbit Content Repository
>          Issue Type: Bug
>          Components: jackrabbit-core
>    Affects Versions: 2.10.5, 2.12.6, 2.15.0, 2.13.7, 2.14.0, 2.8.5
>            Reporter: Benjamin Papez
>
> With JCR-2813 and revision 1038201 bigger synchronization has been removed, but I think it is still too big.
> {code}
>         // Wait if another thread is already loading this item state
>         synchronized (this) {
>             while (currentlyLoading.contains(id)) {
>                 try {
>                     wait();
>                 } catch (InterruptedException e) {
>                     throw new ItemStateException(
>                             "Interrupted while waiting for " + id, e);
>                 }
>             }
>             state = cache.retrieve(id);
>             if (state != null) {
>                 return state;
>             }
>             // No other thread has loaded the item state, so we'll do it
>             currentlyLoading.add(id);
>         }
> {code}
> Looking at thread dumps I see that the reality does not match the comment {{Wait if another thread is already loading this item state}}, because threads have to wait even if they want a different item state. 
> If one thread goes into the wait() method it locks all other threads out, which use the same SharedItemsStateManager instance, because of the {{synchronized (this)}}. 
> I think that it would be better if {{currentlyLoading}} would be backed by a thread-safe {{CoincurrentHashMap}}, so {{synchronized (this)}} would not be necessary around {{currentlyLoading.contains}}.


--
This message was sent by Atlassian JIRA
(v6.3.15#6346)