jackrabbit-oak-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "angela (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (OAK-4119) Improvements Take 1
Date Tue, 15 Mar 2016 11:11:33 GMT

    [ 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 11:11 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:

h5. 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.

h5. Group.isDeclaredMember
- it seems that the query optimization might 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; also the results from the slightly different benchmarks {{MemberIsDeclardMemberTest}}
and {{IsDeclaredMemberTest}} leaves some doubts => additional bench-runs needed to identify
an optimized  value and verify if the proposed changes are really justified.

h5. Group.addMember
_TODO_

h5. 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.

h5. Group.removeMember
_TODO_

h5. 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 might 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; also the results from the slightly different benchmarks {{MemberIsDeclardMemberTest}}
and {{IsDeclaredMemberTest}} leaves some doubts => additional bench-runs needed to identify
an optimized  value and verify if the proposed changes are really justified.

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_

> 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_3.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}}-
> - simplified, new version of {{IdentifierManager.getReferences}} to match needs of membership
lookup where propName and a single nt-name is given. see below (in OAK-4119_3.patch)
> - introduce {{hasAnyReferenceInPath}}, which allows to search for any references being
located in a give subtree of the repository (escaping fixed in OAK-4119_3.patch)
> - {{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)

Mime
View raw message