[
https://issues.apache.org/jira/browse/OAK-4119?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15194887#comment-15194887
]
angela edited comment on OAK-4119 at 3/15/16 10:31 AM:
-------------------------------------------------------
h3. Benchmark Results for Patch OAK-4119_2
assuming that the benchmarks are suited to reflect a realistic scenario, the
preliminary benchmark results seem to indicate the following effects of the
proposed improvements:
h4. Group.isMember
- usage of membership query positive for groups with lots of members
- for very small groups the query comes with some overhead
_Note_: while looking at the profiling and the query-code in
{{IdentifierManager.getReferences}} again, I found the getReferences code
overly complex and unnecessarily resolving the path to a tree and testing for
node-type inheritance and testing for value type and matching against the
searched uuid => will use a simplified, readable variant in patch OAK-4119_3.
h4. Group.isDeclaredMember
- it seems that the query optimization is only justified for groups with really
huge number of members. the original threshold of 10 child nodes (i.e. 1000
members) is most likely too small; => additional bench-runs needed to identify
an optimized value.
h4. Group.addMember
_TODO_
h4. Group.addMembers
- it seems the proposed modification have a positive effect; specially for huge
groups and batched adding. IMO that's not too surprising as the logic didn't
changed but some overhead is omitted.
h4. Group.removeMember
_TODO_
h4. Group.removeMembers
_TODO_
was (Author: anchela):
h3. Benchmark Results for Patch OAK-4119_2
assuming that the benchmarks are suited to reflect a realistic scenario, the
preliminary benchmark results seem to indicate the following effects of the
proposed improvements:
h4. Group.isMember
- usage of membership query positive for groups with lots of members
- for very small groups the query comes with some overhead
_Note_: while looking at the profiling and the query-code in
{{IdentifierManager.getReferences}} again, I found the getReferences code
overly complex and unnecessarily resolving the path to a tree and testing for
node-type inheritance and testing for value type and matching against the
searched uuid => will use a simplified, readable variant in patch OAK-4119_3.
h4. Group.isDeclaredMember
- it seems that the query optimization is only justified for groups with really
huge number of members. the original threshold of 10 child nodes (i.e. 1000
members) is most likely too small; => additional bench-runs needed to identify
an optimized value.
h4. Group.addMember
_TODO_
h4. Group.addMembers
_TODO_
h4. Group.removeMember
_TODO_
h4. Group.removeMembers
_TODO_
> Improvements Take 1
> -------------------
>
> Key: OAK-4119
> URL: https://issues.apache.org/jira/browse/OAK-4119
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: angela
> Assignee: angela
> Attachments: OAK-4119.patch, OAK-4119_2.patch, OAK-4119_tests.patch
>
>
> First bunch of potential improvements to be tested/verified using the
> benchmarks:
> h4. General
> - dedicated index for {{rep:members}} property defined by nt
> {{rep:MemberReferences}} (=> mod in {{UserInitializer}})
> - adjust nt-handling in {{IdentifierManager.getReferences}} : if a single nt
> name is specified in the {{nodeTypeNames}} param it is inserted in the query
> statement instead of looking for {{nt:base}}
> - introduce {{hasAnyReferenceInPath}}, which allows to search for any
> references being located in a give subtree of the repository (with TODOs)
> - {{MembershipProvider.MemberReferenceIterator}}: keeping track of processed
> references is optional i.e. an implementation detail
> - {{MembershipProvider.AbstractMemberIterator}}: no more protected fields
> (patch OAK-4119_2)
> - {{MembershipProvider.AbstractMemberIterator}}: breadth-first iteration
> before performing queries for the inherited members/membership (patch
> OAK-4119_2)
> h4. Group.isMember
> - {{MembershipProvider}}: introduce shortcut if a Group doesn't have any
> members
> - {{MembershipProvider}}: keep old logic if the target Group has
> (potentially) pending changes
> - {{MembershipProvider}}: for unmodified Groups use reverse membership
> lookup of the target authorizable instead (patch OAK-4119_2: further
> optimizes 'hasMembership' as the {{AbstractMemberIterator}} doesn't need to
> search for the complete membership; limiting the number of queries to be
> executed. see above)
> - {{GroupImpl}}: add shortcut if member to be tested is a Group representing
> the {{EveryonePrincipal}}
> h4. Group.isDeclaredMember
> - introduce shortcut if a Group doesn't have any members
> - keep old logic if target Group (potentially) has pending changes or if the
> number of members is small (=> threshold with {{MembershipProvider}})
> - minor change with the old behavior, which does no longer keep track of
> processed references, which is not needed here
> - for unmodified Groups with plenty of members use
> {{IdentifierManager.hasAnyReferenceInPath}}
> - All modifications in {{MembershipProvider}}
> h4. Group.addMember(Authorizable)
> - {{GroupImpl}}: drop extra test for circular membership, which is anyway
> enforced by the {{UserValidator}} => needs adjustment of test-cases that
> expect this to be detected immediately => intoducing save-call to verify if
> cycles are properly detected latest during commit.
> - {{UserValidator}}: if {{isMember}} is improved by the changes above, the
> check for circular membership would be faster as well
> h4. Group.addMembers(String...)
> - introduce new methods on {{MembershipProvider}} and {{{{MembershipWriter}}
> that don't take a single contentID but rather a Map of contentID:memberID and
> returns the set of memberIDs that could not be processed successfully.
> - consequently finding the best {{rep:members property}} and avoiding
> duplicate membership is performed only once for the batch of ids to be added
> - Note: once best-property reaches threshold new member-ref nodes will be
> created
> h4. Group.removeMembers(String...)
> - introduce new methods on {{MembershipProvider}} and {{{{MembershipWriter}}
> that don't take a single contentID but rather a Map of contentID:memberID and
> returns the set of memberIDs that could not be processed successfully.
> - consequently for a given batch of ids the declared member are traversed
> only once
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
