You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by "Alex Parvulescu (JIRA)" <ji...@apache.org> on 2012/06/11 16:54:43 UTC
[jira] [Created] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Alex Parvulescu created JCR-3335:
------------------------------------
Summary: Reduce synchronized block scope when loading a node by id
Key: JCR-3335
URL: https://issues.apache.org/jira/browse/JCR-3335
Project: Jackrabbit Content Repository
Issue Type: Sub-task
Reporter: Alex Parvulescu
Attachments: CachingEntryCollector.java.patch
This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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] [Commented] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Posted by "Julian Reschke (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13292828#comment-13292828 ]
Julian Reschke commented on JCR-3335:
-------------------------------------
(funny enough, I was looking at this as well :-)
Comments:
- we have that pattern in both getEntries(NodeImpl) and getEntries(NodeId), so we should fix them both
- once we have separated read from write operations, we may want to consider replacing the simple monitor by a ReadWriteLock...
Question: do we have test coverage for multithreaded access?
> Reduce synchronized block scope when loading a node by id
> ---------------------------------------------------------
>
> Key: JCR-3335
> URL: https://issues.apache.org/jira/browse/JCR-3335
> Project: Jackrabbit Content Repository
> Issue Type: Sub-task
> Reporter: Alex Parvulescu
> Attachments: CachingEntryCollector.java.patch
>
>
> This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
> I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
> In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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] [Closed] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu closed JCR-3335.
--------------------------------
> Reduce synchronized block scope when loading a node by id
> ---------------------------------------------------------
>
> Key: JCR-3335
> URL: https://issues.apache.org/jira/browse/JCR-3335
> Project: Jackrabbit Content Repository
> Issue Type: Sub-task
> Reporter: Alex Parvulescu
> Attachments: CachingEntryCollector.java.patch
>
>
> This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
> I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
> In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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] [Reopened] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu reopened JCR-3335:
----------------------------------
> Reduce synchronized block scope when loading a node by id
> ---------------------------------------------------------
>
> Key: JCR-3335
> URL: https://issues.apache.org/jira/browse/JCR-3335
> Project: Jackrabbit Content Repository
> Issue Type: Sub-task
> Reporter: Alex Parvulescu
> Attachments: CachingEntryCollector.java.patch
>
>
> This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
> I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
> In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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] [Commented] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13293428#comment-13293428 ]
Alex Parvulescu commented on JCR-3335:
--------------------------------------
> you replace the 'updateCache' method with 'getEntries'.
I'm not actually replacing the 2 calls, I'm only reducing the mirrored code, and merging the 2 similar 'getEntries' methods together.
And like you've noticed this has the added benefit that it loads the node outside the read lock.
> Reduce synchronized block scope when loading a node by id
> ---------------------------------------------------------
>
> Key: JCR-3335
> URL: https://issues.apache.org/jira/browse/JCR-3335
> Project: Jackrabbit Content Repository
> Issue Type: Sub-task
> Reporter: Alex Parvulescu
> Attachments: CachingEntryCollector.java.patch
>
>
> This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
> I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
> In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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] [Updated] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu updated JCR-3335:
---------------------------------
Attachment: CachingEntryCollector.java.patch
attaching proposed patch.
feedback welcome!
> Reduce synchronized block scope when loading a node by id
> ---------------------------------------------------------
>
> Key: JCR-3335
> URL: https://issues.apache.org/jira/browse/JCR-3335
> Project: Jackrabbit Content Repository
> Issue Type: Sub-task
> Reporter: Alex Parvulescu
> Attachments: CachingEntryCollector.java.patch
>
>
> This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
> I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
> In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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] [Resolved] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Posted by "Alex Parvulescu (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Alex Parvulescu resolved JCR-3335.
----------------------------------
Resolution: Invalid
patch rendered invalid by recent ACL work
> Reduce synchronized block scope when loading a node by id
> ---------------------------------------------------------
>
> Key: JCR-3335
> URL: https://issues.apache.org/jira/browse/JCR-3335
> Project: Jackrabbit Content Repository
> Issue Type: Sub-task
> Reporter: Alex Parvulescu
> Attachments: CachingEntryCollector.java.patch
>
>
> This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
> I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
> In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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] [Resolved] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Posted by "Julian Reschke (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Julian Reschke resolved JCR-3335.
---------------------------------
Resolution: Fixed
This has been addressed as part of r1350686, see JCR-2950.
Note that this change should reduce blocking on the cache, but might add additional load on the persistence, because now we might be fetching ACL information for the same node in parallel (where before it was serialized).
If this turns out to be a problem we can try various things:
- synchronize updateCache
- optimize fetching the ACLs
- optimize filling the cache for parent nodes
- serializing the fetching per nodeId
We probably should wait with this until we have benchmarks.
> Reduce synchronized block scope when loading a node by id
> ---------------------------------------------------------
>
> Key: JCR-3335
> URL: https://issues.apache.org/jira/browse/JCR-3335
> Project: Jackrabbit Content Repository
> Issue Type: Sub-task
> Reporter: Alex Parvulescu
> Attachments: CachingEntryCollector.java.patch
>
>
> This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
> I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
> In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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] [Commented] (JCR-3335) Reduce synchronized block scope when
loading a node by id
Posted by "angela (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/JCR-3335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13293422#comment-13293422 ]
angela commented on JCR-3335:
-----------------------------
while i like the general approach of separating cache reading from writing i am not convinced that the patch would work as you replace
the 'updateCache' method with 'getEntries'.
@julian: i am not aware of such tests. i created some tests to assert that the cache is properly cleared upon ac-modification... maybe we can
add tests for multithreaded access there?
> Reduce synchronized block scope when loading a node by id
> ---------------------------------------------------------
>
> Key: JCR-3335
> URL: https://issues.apache.org/jira/browse/JCR-3335
> Project: Jackrabbit Content Repository
> Issue Type: Sub-task
> Reporter: Alex Parvulescu
> Attachments: CachingEntryCollector.java.patch
>
>
> This is a small(ish) change that takes the 'getNodeById' cll outside of the synchronized block that wraps the cache access.
> I see the load call as a heavy one, and the proposed change offers better concurrency behavior.
> In the tests that I ran, mostly related to queries I got a 20% improvement on average query response time.
--
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