From 0dff68634c7a2655f030c7e43e95f774e3c6437c Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Wed, 17 Jul 2019 17:14:01 -0400 Subject: [PATCH] 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 Signed-off-by: Galen Charlton --- .../perlmods/lib/OpenILS/Application/AppUtils.pm | 1 + .../Application/Storage/Driver/Pg/QueryParser.pm | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/AppUtils.pm index 5ed0e25436..f44e6ef049 100644 --- 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 941085133d..950a081b63 100644 --- 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)"; -- 2.11.0