You are here: Foswiki>Tasks Web>Item12964 (05 Jul 2015, GeorgeClark)Edit Attach

Item12964: UserMapping::isInGroup() iterates uselessly over member lists.

pencil
Priority: Normal
Current State: Closed
Released In: 2.0.0
Target Release: major
Applies To: Engine
Component: UserMapping, Performance
Branches: trunk Release01x01
Reported By: FredTarbox
Waiting For:
Last Change By: GeorgeClark
-- FredTarbox - 15 Jul 2014

UserMapping::isInGroup has an expand option. This option is used in two places. First, it is passed to the underlying mapping's eachGroupMember function. Second, if it is set, it attempts to expand the member list given to it by eachGroupMember, testing each member to see if it is a group, and if it finds a group, expanding it.

Both TopicUserMapping::eachGroupMember and LdapUserMapping::eachGroupMember respect the expand tag. They recursively expand their member list so that it only includes users. Furthermore, they both cache the result, so future calls to eachGroupMember are very fast.

Hence, isInGroup's own attempts to expand the member list are guaranteed to expand nothing but call the underlying isGroup and isInGroup for every member.

According to my notes, a page load with these extra expands can call isGroup several thousand times. After commenting out the lines as shown, there are less than 10 such calls.

Note the commented lines:
sub isInGroup {
    my ( $this, $cUID, $group, $options ) = @_;
    ASSERT($cUID) if DEBUG;

    my $expand = $options->{expand};
    $expand = 1 unless ( defined $expand );

    # If not recursively, clear the scanning hash
    if ( ( caller(1) )[3] ne ( caller(0) )[3] ) {
        %scanning = ();
    }

    my @users;

    my $it = $this->eachGroupMember( $group, { expand => $expand } );
    while ( $it->hasNext() ) {
        my $u = $it->next();
        next if $scanning{$u};
        $scanning{$u} = 1;

        return 1 if $u eq $cUID;
#        if ( $expand && $this->isGroup($u) ) {
#            return 1 if $this->isInGroup( $cUID, $u );
#        }
    }
    return 0;
}

 

ItemTemplate edit

Summary UserMapping::isInGroup() iterates uselessly over member lists.
ReportedBy FredTarbox
Codebase 1.1.9, trunk
SVN Range
AppliesTo Engine
Component UserMapping, Performance
Priority Normal
CurrentState Closed
WaitingFor
Checkins distro:37f870a0c908 distro:f9c6e3883d90
TargetRelease major
ReleasedIn 2.0.0
CheckinsOnBranches trunk Release01x01
trunkCheckins distro:37f870a0c908
masterCheckins
ItemBranchCheckins
Release01x01Checkins distro:f9c6e3883d90
Topic revision: r6 - 05 Jul 2015, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy