[Rt-commit] rt branch, 4.4/cache-roles, created. rt-4.4.4-106-g5372526694

[? sunnavy]

The branch, 4.4/cache-roles has been created
        at  53725266941c2aca7522ddea7439da52c1e899d1 (commit)

- Log -----------------------------------------------------------------
commit a6e66eff7de39dbbf257916fec1cf0f775b27739
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat May 30 05:39:53 2020 +0800

    Cache Roles method for ticket/queue objects
    
    Roles method is called a lot of times in RT, e.g. it is called dozens of
    times on ticket creation, which could cause performance issue when there
    are a few custom roles involved, as the relatively slow
    AppliesToObjectPredicate is called once for each custom role in each
    Roles method call.
    
    This commit caches the result of Roles at ticket/queue object level, to
    improve the performance. As ticket/queue objects don't live long
    generally and the cache is cleared after every Load call, we don't need
    to worry about the issue that the cache might be outdated in nearly all
    the cases.
    
    We tested an RT instance with 10+ custom roles, this change could
    improve the performance by about 20%.

diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 3503f67f30..210ef46fe0 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -371,6 +371,7 @@ sub LoadByCols {
 
     # We don't want to hang onto this
     $self->ClearAttributes;
+    delete $self->{_Roles};
 
     unless ( $self->_Handle->CaseSensitive ) {
         my ( $ret, $msg ) = $self->SUPER::LoadByCols( @_ );
diff --git a/lib/RT/Record/Role/Roles.pm b/lib/RT/Record/Role/Roles.pm
index b973eb21b1..ef628e5978 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -276,7 +276,10 @@ sub Roles {
     my $self = shift;
     my %attr = @_;
 
-    return   map { $_->[0] }
+    my $key  = join ',', @_;
+    return @{ $self->{_Roles}{$key} } if ref($self) && $self->{_Roles}{$key};
+
+    my @roles =  map { $_->[0] }
             sort {   $a->[1]{SortOrder} <=> $b->[1]{SortOrder}
                   or $a->[0] cmp $b->[0] }
             grep {
@@ -289,6 +292,13 @@ sub Roles {
                  or $_->[1]{AppliesToObjectPredicate}->($self) }
              map { [ $_, $self->Role($_) ] }
             keys %{ $self->_ROLES };
+
+    # Cache at ticket/queue object level mainly to reduce calls of
+    # custom role's AppliesToObjectPredicate for performance.
+    if ( ref($self) =~ /RT::(?:Ticket|Queue)/ ) {
+        $self->{_Roles}{$key} = \@roles;
+    }
+    return @roles;
 }
 
 {

commit 53725266941c2aca7522ddea7439da52c1e899d1
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Sat May 30 07:21:11 2020 +0800

    Update tests to reload tickets as Roles are cached now

diff --git a/t/customroles/existing-tickets.t b/t/customroles/existing-tickets.t
index 13312e0916..5980e336c4 100644
--- a/t/customroles/existing-tickets.t
+++ b/t/customroles/existing-tickets.t
@@ -82,6 +82,7 @@ diag 'create roles and add them to the queue' if $ENV{'TEST_VERBOSE'};
 for my $t ($t1, $t2) {
     diag 'test managing watchers of new roles on #' . $t->id if $ENV{'TEST_VERBOSE'};
 
+    $t->Load($t->Id);
     my ($ok, $msg) = $t->AddWatcher(Type => $sales->GroupType, Principal => $ricky->PrincipalObj);
     ok($ok, "add sales: $msg");
     is($t->RoleAddresses($sales->GroupType), $ricky->EmailAddress, 'sales ricky');

-----------------------------------------------------------------------