LP#1836963: reduce the cost of utility functions, speeding up search

For large org trees, some several seconds are spent testing org visibility.
The immediate cause is that AppUtils::get_org_tree() does not populate the
process-local cache with a memcache-cached org tree. That only happens when
memcache does not have a copy of the org tree. This is obviously a simple
oversight, which is addressed by making sure any memcache return value
is pushed into the the process local cache.

Additionally, the visibility check is making heavy use of lots of
indirection and delegation to utility code, when some slightly smarter code
could avoid many repeated function calls.  We now supply some local
utility code to flesh and unflesh the parent_ou field of objects in the
org tree, allowing us to avoid using find_org() and instead just calling
parent_ou() when walking "up" the tree.

Signed-off-by: Mike Rylander <mrylander@gmail.com>
Signed-off-by: Galen Charlton <gmc@equinoxinitiative.org>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
index 5ed0e25..f44e6ef 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm
@@ -1454,6 +1454,7 @@ sub get_org_tree {
     my $locale = shift || '';
     my $cache = OpenSRF::Utils::Cache->new("global", 0);
     my $tree = $ORG_TREE{$locale} || $cache->get_cache("orgtree.$locale");
+    $ORG_TREE{$locale} = $tree; # make sure to populate the process-local cache
     return $tree if $tree;
 
     my $ses = OpenILS::Utils::CStoreEditor->new;

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
index 9410851..950a081 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Driver/Pg/QueryParser.pm
@@ -1179,13 +1179,28 @@ sub is_org_visible {
     my $non_inherited_vis_gf = shift || $U->get_global_flag('opac.org_unit.non_inherited_visibility');
     return 1 if ($U->is_true($non_inherited_vis_gf->enabled));
 
-    my $ot = $U->get_org_tree;
-    while ($org = $U->find_org($ot,$org->parent_ou)) {
+    while ($org = $org->parent_ou) {
         return 0 if (!$U->is_true($org->opac_visible));
     }
     return 1;
 }
 
+sub flesh_parents {
+    my $t = shift;
+    my $kids = $t->children;
+    if ($kids && @$kids) {
+        map {$_->parent_ou($t); flesh_parents($_)} @$kids;
+    }
+}
+
+sub unflesh_parents {
+    my $t = shift;
+    my $kids = $t->children;
+    if ($kids && @$kids) {
+        map {$_->parent_ou($t->id); unflesh_parents($_)} @$kids;
+    }
+}
+
 sub flatten {
     my $self = shift;
 
@@ -1446,11 +1461,13 @@ sub flatten {
     my $dorgs = $U->get_org_descendants($site_org->id, $depth_filter);
     my $aorgs = $U->get_org_ancestors($site_org->id);
 
+    flesh_parents($ot);
     if (!$self->find_modifier('staff')) {
         my $non_inherited_vis_gf = $U->get_global_flag('opac.org_unit.non_inherited_visibility');
         $dorgs = [ grep { is_org_visible($U->find_org($ot,$_), $non_inherited_vis_gf) } @$dorgs ];
         $aorgs = [ grep { is_org_visible($U->find_org($ot,$_), $non_inherited_vis_gf) } @$aorgs ];
     }
+    unflesh_parents($ot);
 
     push @{$vis_filter{'c_attr'}},
         "search.calculate_visibility_attribute_test('circ_lib','{".join(',', @$dorgs)."}',$negate)";