[Rt-commit] rt branch, 4.2/cache-ocfvs-on-update, created. rt-4.2.14-8-g97a3570

Jim Brandt jbrandt at bestpractical.com
Mon Sep 18 14:49:54 EDT 2017


The branch, 4.2/cache-ocfvs-on-update has been created
        at  97a3570b6cfdfa78eee027eb753ea237541b0cba (commit)

- Log -----------------------------------------------------------------
commit 97a3570b6cfdfa78eee027eb753ea237541b0cba
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Mon Sep 18 14:14:07 2017 -0400

    Cache OCFVs to improve performance searching for duplicates on add
    
    When adding a new value to a custom field, the HasEntry code is used
    to first determine whether the new value already exists. When inserting
    a large number of values one after the other (like along list of IPs),
    the HasEntry code pulled the full list of values from the DB for every
    check, creating a performance issue. Cache the values to speed up the
    existing value check.

diff --git a/lib/RT/CustomField.pm b/lib/RT/CustomField.pm
index 62afb67..ecf5907 100644
--- a/lib/RT/CustomField.pm
+++ b/lib/RT/CustomField.pm
@@ -1843,6 +1843,10 @@ sub DeleteValueForObject {
         return ( 0, $self->loc('Input must match [_1]', $self->FriendlyPattern) );
     }
 
+    # Clear any cached values
+    my $ocfv_key = $oldval->GetOCFVCacheKey;
+    delete $RT::ObjectCustomFieldValues::_OCFV_CACHE->{$ocfv_key};
+
     # delete it
 
     my $ret = $oldval->Delete();
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index 9af328a..f5f66c1 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -481,6 +481,20 @@ sub ParseIP {
 }
 
 
+=head2 GetOCFVCacheKey
+
+Get the OCFV cache key for this object
+
+=cut
+
+sub GetOCFVCacheKey {
+    my $self = shift;
+    my $ocfv_key = "CustomField-" . $self->CustomField
+        . '-ObjectType-' . $self->ObjectType
+        . '-ObjectId-' . $self->ObjectId;
+    return $ocfv_key;
+}
+
 =head2 id
 
 Returns the current value of id.
diff --git a/lib/RT/ObjectCustomFieldValues.pm b/lib/RT/ObjectCustomFieldValues.pm
index 4ea660c..4bcc7ec 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -56,6 +56,10 @@ use base 'RT::SearchBuilder';
 
 use RT::ObjectCustomFieldValue;
 
+# Set up the OCFV cache for faster comparison on add/update
+our $_OCFV_CACHE;
+ClearOCFVCache();
+
 sub Table { 'ObjectCustomFieldValues'}
 
 sub _Init {
@@ -74,6 +78,15 @@ sub _Init {
     return ( $self->SUPER::_Init(@_) );
 }
 
+=head2 ClearOCFVCache
+
+Cleans out and reinitializes the OCFV cache
+
+=cut
+
+sub ClearOCFVCache {
+    $_OCFV_CACHE = {}
+}
 
 # {{{ sub LimitToCustomField
 
@@ -129,10 +142,30 @@ sub HasEntry {
     my $large_content = shift;
     return undef unless defined $value && length $value;
 
+    my $first = $self->First;
+    return undef unless $first;  # No entries to check
+
+    # Key should be the same for all values of the same ocfv
+    my $ocfv_key = $first->GetOCFVCacheKey;
+
+    # This cache relieves performance issues when adding large numbers of values
+    # to a CF since each add compares against the full list each time.
+
+    unless ( keys %$_OCFV_CACHE ) {
+        # Load the cache with existing values
+        foreach my $item ( @{$self->ItemsArrayRef} ) {
+            push @{$_OCFV_CACHE->{$ocfv_key}}, {
+                'ObjectId'       => $item->Id,
+                'Object'         => $item,
+                'CustomFieldObj' => $item->CustomFieldObj,
+                'Content'        => $item->_Value('Content'),
+                'LargeContent'   => $item->LargeContent };
+        }
+    }
+
     my %canon_value;
-    #TODO: this could cache and optimize a fair bit.
-    foreach my $item ( @{$self->ItemsArrayRef} ) {
-        my $cf = $item->CustomFieldObj;
+    foreach my $item ( @{$_OCFV_CACHE->{$ocfv_key}} ) {
+        my $cf = $item->{'CustomFieldObj'};
         my $args = $canon_value{ $cf->Type };
         if ( !$args ) {
             $args = { Content => $value, LargeContent => $large_content };
@@ -143,23 +176,24 @@ sub HasEntry {
 
         if ( $cf->Type eq 'Select' ) {
             # select is case insensitive
-            return $item if lc $item->Content eq lc $args->{Content};
+            return $item->{'Object'} if lc $item->{'Content'} eq lc $args->{Content};
         }
         else {
-            if ( ($item->_Value('Content') // '') eq $args->{Content} ) {
-                if ( defined $item->LargeContent ) {
-                    return $item
+            if ( ($item->{'Content'} // '') eq $args->{Content} ) {
+                if ( defined $item->{'LargeContent'} ) {
+                    return $item->{'Object'}
                       if defined $args->{LargeContent}
-                      && $item->LargeContent eq $args->{LargeContent};
+                      && $item->{'LargeContent'} eq $args->{LargeContent};
                 }
                 else {
-                    return $item unless defined $args->{LargeContent};
+                    return $item->{'Object'} unless defined $args->{LargeContent};
                 }
-            } elsif ( $item->LargeContent && $args->{Content} ) {
-                return $item if ($item->LargeContent eq $args->{Content});
+            } elsif ( $item->{'LargeContent'} && $args->{Content} ) {
+                return $item->{'Object'} if ($item->{'LargeContent'} eq $args->{Content});
             }
         }
     }
+
     return undef;
 }
 
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index f80061f..8e0392b 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -2050,12 +2050,27 @@ sub _AddCustomFieldValue {
         my $new_value = RT::ObjectCustomFieldValue->new( $self->CurrentUser );
         $new_value->Load( $new_value_id );
 
+        # Prepare to update the OCFV cache
+        my $ocfv_key = $new_value->GetOCFVCacheKey;
+
         # now that adding the new value was successful, delete the old one
         if ( $old_value ) {
+            # Remove old cached value
+            @{$RT::ObjectCustomFieldValues::_OCFV_CACHE->{$ocfv_key}} =
+                grep { $_->{'ObjectId'} == $old_value->Id } @{$RT::ObjectCustomFieldValues::_OCFV_CACHE->{$ocfv_key}};
+
             my ( $val, $msg ) = $old_value->Delete();
             return ( 0, $msg ) unless $val;
         }
 
+        # Add the new one
+        push @{$RT::ObjectCustomFieldValues::_OCFV_CACHE->{$ocfv_key}}, {
+            'ObjectId'       => $new_value->Id,
+            'Object'         => $new_value,
+            'CustomFieldObj' => $new_value->CustomFieldObj,
+            'Content'        => $args{'Value'},
+            'LargeContent'   => $args{'LargeContent'} };
+
         if ( $args{'RecordTransaction'} ) {
             my ( $TransactionId, $Msg, $TransactionObj ) =
               $self->_NewTransaction(
@@ -2117,6 +2132,18 @@ sub _AddCustomFieldValue {
         unless ( $new_value_id ) {
             return ( 0, $self->loc( "Could not add new custom field value: [_1]", $msg ) );
         }
+
+        my $new_value = RT::ObjectCustomFieldValue->new( $self->CurrentUser );
+        $new_value->Load( $new_value_id );
+
+        # Update the OCFV cache
+        my $ocfv_key = $new_value->GetOCFVCacheKey;
+        push @{$RT::ObjectCustomFieldValues::_OCFV_CACHE->{$ocfv_key}}, {
+            'Object'         => $new_value,
+            'CustomFieldObj' => $new_value->CustomFieldObj,
+            'Content'        => $args{'Value'},
+            'LargeContent'   => $args{'LargeContent'} };
+
         if ( $args{'RecordTransaction'} ) {
             my ( $tid, $msg ) = $self->_NewTransaction(
                 Type          => 'CustomField',

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


More information about the rt-commit mailing list