[ https://issues.apache.org/jira/browse/OAK-3616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15001862#comment-15001862
]
angela commented on OAK-3616:
-----------------------------
even for read?
at least it seems that we are having benchmarks that perform concurrent read operations.
but i agree that it might not be too common in practice (see above).
on the other hand i fail to see the benefit of having the checkLive method synchronized. so,
i would really love to understand why we have this in the first place and second if we can
imagine _any_ kind of regressions that might be introduced if we remove it. as you can see
{{ContentSession#close()}} is also synchronized... that looks odd to me.
in other words: what could go wrong if we make the change? if we can't find a scenario where
this would lead to real problems, we might still want to change it for the sake of proper
synchronization.
> Reconsider Synchronization with ContentSessionImpl.checkLive
> ------------------------------------------------------------
>
> Key: OAK-3616
> URL: https://issues.apache.org/jira/browse/OAK-3616
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: angela
> Priority: Minor
>
> while running permission related benchmarks directly on Oak (without the extra oak-jcr
layer), i found a considerable difference between
> - concurrent read with a single content session
> - concurrent read with different content sessions
> according to the built-in profiler information the former seemed to be limited by the
synchronized {{ContentSessionImpl.checkLive}} call:
> {code}
> # CugOakTest C min 10% 50% 90% max N
> Import deep tree: 8432
> All paths: 123545
> Oak-Tar 1 15 18 22 25 31 229
> Oak-Tar 2 19 25 29 33 38 344
> Oak-Tar 4 26 33 39 45 51 513
> Oak-Tar 8 70 86 94 102 110 431
> Oak-Tar 10 65 105 119 131 143 427
> Oak-Tar 15 92 152 169 186 210 449
> Oak-Tar 20 148 212 229 250 265 440
> Oak-Tar 50 283 485 549 602 666 480
> Profiler: top 20 stack trace(s) of 50647 ms:
> 15517/45026 (34%):
> at org.apache.jackrabbit.oak.core.ContentSessionImpl.checkLive(ContentSessionImpl.java:85)
> at org.apache.jackrabbit.oak.core.MutableRoot.checkLive(MutableRoot.java:172)
> at org.apache.jackrabbit.oak.core.MutableTree.beforeRead(MutableTree.java:333)
> at org.apache.jackrabbit.oak.core.MutableTree.hasProperty(MutableTree.java:133)
> at org.apache.jackrabbit.oak.plugins.tree.TreeLocation$NodeLocation.getChild(TreeLocation.java:166)
> at org.apache.jackrabbit.oak.plugins.tree.TreeLocation.create(TreeLocation.java:62)
> at org.apache.jackrabbit.oak.benchmark.CugOakTest.runTest(CugOakTest.java:95)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:347)
> at org.apache.jackrabbit.oak.benchmark.ReadDeepTreeTest.execute(ReadDeepTreeTest.java:35)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:356)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.access$000(AbstractTest.java:45)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest$Executor.run(AbstractTest.java:277)
> 12279/45026 (27%):
> at org.apache.jackrabbit.oak.core.ContentSessionImpl.checkLive(ContentSessionImpl.java:85)
> at org.apache.jackrabbit.oak.core.MutableRoot.checkLive(MutableRoot.java:172)
> at org.apache.jackrabbit.oak.core.MutableTree.beforeRead(MutableTree.java:333)
> at org.apache.jackrabbit.oak.core.MutableTree.getChild(MutableTree.java:160)
> at org.apache.jackrabbit.oak.plugins.tree.TreeLocation$NodeLocation.getChild(TreeLocation.java:169)
> at org.apache.jackrabbit.oak.plugins.tree.TreeLocation.create(TreeLocation.java:62)
> at org.apache.jackrabbit.oak.benchmark.CugOakTest.runTest(CugOakTest.java:95)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:347)
> at org.apache.jackrabbit.oak.benchmark.ReadDeepTreeTest.execute(ReadDeepTreeTest.java:35)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:356)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.access$000(AbstractTest.java:45)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest$Executor.run(AbstractTest.java:277)
> 3119/45026 (6%):
> at org.apache.jackrabbit.oak.core.ContentSessionImpl.checkLive(ContentSessionImpl.java:85)
> at org.apache.jackrabbit.oak.core.MutableRoot.checkLive(MutableRoot.java:172)
> at org.apache.jackrabbit.oak.core.MutableTree.beforeRead(MutableTree.java:333)
> at org.apache.jackrabbit.oak.core.MutableTree.hasProperty(MutableTree.java:133)
> at org.apache.jackrabbit.oak.plugins.tree.TreeLocation$PropertyLocation.exists(TreeLocation.java:222)
> at org.apache.jackrabbit.oak.benchmark.CugOakTest.runTest(CugOakTest.java:96)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:347)
> at org.apache.jackrabbit.oak.benchmark.ReadDeepTreeTest.execute(ReadDeepTreeTest.java:35)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:356)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.access$000(AbstractTest.java:45)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest$Executor.run(AbstractTest.java:277)
> 1694/45026 (3%):
> at org.apache.jackrabbit.oak.core.ContentSessionImpl.checkLive(ContentSessionImpl.java:85)
> at org.apache.jackrabbit.oak.core.MutableRoot.checkLive(MutableRoot.java:172)
> at org.apache.jackrabbit.oak.core.MutableRoot.getTree(MutableRoot.java:216)
> at org.apache.jackrabbit.oak.core.MutableRoot.getTree(MutableRoot.java:66)
> at org.apache.jackrabbit.oak.plugins.tree.TreeLocation.create(TreeLocation.java:60)
> at org.apache.jackrabbit.oak.benchmark.CugOakTest.runTest(CugOakTest.java:95)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:347)
> at org.apache.jackrabbit.oak.benchmark.ReadDeepTreeTest.execute(ReadDeepTreeTest.java:35)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:356)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.access$000(AbstractTest.java:45)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest$Executor.run(AbstractTest.java:277)
> 1356/45026 (3%):
> at org.apache.jackrabbit.oak.core.ContentSessionImpl.checkLive(ContentSessionImpl.java:85)
> at org.apache.jackrabbit.oak.core.MutableRoot.checkLive(MutableRoot.java:172)
> at org.apache.jackrabbit.oak.core.MutableTree.beforeRead(MutableTree.java:333)
> at org.apache.jackrabbit.oak.core.MutableTree.getProperty(MutableTree.java:127)
> at org.apache.jackrabbit.oak.plugins.tree.TreeLocation$PropertyLocation.getProperty(TreeLocation.java:233)
> at org.apache.jackrabbit.oak.benchmark.CugOakTest.runTest(CugOakTest.java:97)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:347)
> at org.apache.jackrabbit.oak.benchmark.ReadDeepTreeTest.execute(ReadDeepTreeTest.java:35)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.execute(AbstractTest.java:356)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest.access$000(AbstractTest.java:45)
> at org.apache.jackrabbit.oak.benchmark.AbstractTest$Executor.run(AbstractTest.java:277)
> summary:
> 79%: org.apache.jackrabbit.oak.core
> 8%: org.apache.jackrabbit.oak.plugins.segment
> 5%: org.apache.jackrabbit.oak.plugins.memory
> {code}
> after discussing this with [~mduerig], i tmp made the {{live}} field {{volatile}} and
removed the synchronization. as a result the concurrent random read with a single session
seemed to be improved (in fact showing more or less the identical figures than when executing
the test with concurrent reads of different sessions:
> {code}
> # CugOakTest , C, min, 10%, 50%, 90%, max, N
> Oak-Tar , 1, 11, 17, 21, 26, 30, 234
> Oak-Tar , 2, 17, 25, 29, 34, 37, 345
> Oak-Tar , 4, 34, 39, 43, 47, 51, 465
> Oak-Tar , 8, 28, 37, 47, 65, 96, 813
> Oak-Tar , 10, 29, 41, 55, 76, 135, 873
> Oak-Tar , 15, 27, 48, 80, 127, 220, 894
> Oak-Tar , 20, 26, 54, 103, 178, 337, 910
> Oak-Tar , 50, 26, 82, 230, 444, 1237, 953
> summary:
> 35%: org.apache.jackrabbit.oak.plugins.segment
> 13%: org.apache.jackrabbit.oak.plugins.memory
> 12%: org.apache.jackrabbit.oak.security.authorization.permission
> {code}
> Should be consider removing the synchronized statement? I am not sure how important concurrent
reads with the same content sessions are, but nevertheless i was a bit surprised by the numbers.
> [~mduerig], what do you think?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
|