From 0df4f2951febd2db9c3eb3450690cf3a273c5a6d Mon Sep 17 00:00:00 2001 From: Mike Rylander Date: Fri, 16 Feb 2018 12:35:41 -0500 Subject: [PATCH] LP#1744385: Speed up highlighting This commit removes a layer of opensrf-perl indirection and uses cstore instead of storage for retrieval. This trims seconds off the render time of results and a bit off record detail. Signed-off-by: Mike Rylander Signed-off-by: Kathy Lussier Signed-off-by: Dan Wells --- .../lib/OpenILS/Application/Search/Biblio.pm | 40 ++++++++++++++------ .../Application/Storage/Publisher/metabib.pm | 43 ---------------------- .../src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm | 9 +++-- Open-ILS/src/sql/Pg/300.schema.staged_search.sql | 23 ++++++++---- Open-ILS/src/sql/Pg/950.data.seed-values.sql | 2 +- .../upgrade/WWWW.data.display-field-seed-data.sql | 2 +- .../Pg/upgrade/XXXX.schema.highlight_search.sql | 24 ++++++++---- Open-ILS/src/templates/opac/parts/misc_util.tt2 | 5 +++ 8 files changed, 72 insertions(+), 76 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm index 96ce5da3ee..96e6a18d96 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Search/Biblio.pm @@ -1306,26 +1306,42 @@ sub staged_search { return cache_facets($facet_key, $new_ids, $IAmMetabib, $ignore_facet_classes) if $docache; } -sub passthrough_fetch_display_fields { +sub fetch_display_fields { my $self = shift; my $conn = shift; my $highlight_map = shift; my @records = @_; - return $U->storagereq( - 'open-ils.storage.fetch.metabib.display_field.highlight', - $highlight_map, - @records - ) if (@records == 1); + unless (@records) { + $conn->respond_complete; + return; + } - return $U->storagereq( - 'open-ils.storage.fetch.metabib.display_field.highlight.atomic', - $highlight_map, - \@records - ); + my $hl_map_string = "''::HSTORE"; + if (ref($highlight_map) =~ /HASH/) { + $hl_map_string = ""; + for my $tsq (keys %$highlight_map) { + my $field_list = join(',', @{$$highlight_map{$tsq}}); + $hl_map_string .= ' || ' if $hl_map_string; + $hl_map_string .= "hstore(($tsq)\:\:TEXT,'$field_list')"; + } + } + + my $e = new_editor(); + + for my $record ( @records ) { + next unless $record; + $conn->respond( + $e->json_query( + {from => ['search.highlight_display_fields', $record, $hl_map_string]} + ) + ); + } + + return undef; } __PACKAGE__->register_method( - method => 'passthrough_fetch_display_fields', + method => 'fetch_display_fields', api_name => 'open-ils.search.fetch.metabib.display_field.highlight' ); diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm index eb3f423279..506846a5c2 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/metabib.pm @@ -92,49 +92,6 @@ sub _initialize_parser { die("Cannot initialize $parser!") unless ($parser->initialization_complete); } -sub fetch_highlighted_display_fields { - my $self = shift; - my $client = shift; - my $records = shift; - my $highlight_map = shift; - - unless ($records) { - $client->respond_complete; - return; - } - - my $hl_map_string = "''::HSTORE"; - if (ref($highlight_map)) { - $hl_map_string = ""; - for my $tsq (keys %$highlight_map) { - my $field_list = join(',', @{$$highlight_map{$tsq}}); - $hl_map_string .= ' || ' if $hl_map_string; - $hl_map_string .= "hstore(($tsq)\:\:TEXT,'$field_list')"; - } - } - - my $sth = metabib::metarecord_source_map->db_Main->prepare( - "SELECT * FROM search.highlight_display_fields(?, $hl_map_string)" - ); - - $records = [$records] unless ref($records); - for my $record ( @$records ) { - next unless $record; - $sth->execute($record); - my $rows = $sth->fetchall_arrayref({}); - $client->respond($rows); - } - - return undef; -} -__PACKAGE__->register_method( - api_name => 'open-ils.storage.fetch.metabib.display_field.highlight', - method => 'fetch_highlighted_display_fields', - api_level => 1, - stream => 1 -); - - sub ordered_records_from_metarecord { # XXX Replace with QP-based search-within-MR my $self = shift; my $client = shift; diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm index e386d4375a..c243d728a6 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm @@ -39,7 +39,7 @@ use constant COOKIE_ANON_CACHE => 'anoncache'; use constant ANON_CACHE_MYLIST => 'mylist'; use constant ANON_CACHE_STAFF_SEARCH => 'staffsearch'; -use constant DEBUG_TIMING => 0; +use constant DEBUG_TIMING => 1; sub new { my($class, $apache, $ctx) = @_; @@ -51,6 +51,9 @@ sub new { $self->cgi(new CGI); $self->timelog("New page"); + # Add a timelog helper to the context + $self->ctx->{timelog} = sub { return $self->timelog(@_) }; + OpenILS::Utils::CStoreEditor->init; # just in case $self->editor(new_editor()); @@ -346,8 +349,8 @@ sub load_common { return $U->simplereq( 'open-ils.search', 'open-ils.search.fetch.metabib.display_field.highlight', - $id, - $ctx->{query_struct}{additional_data}{highlight_map} + $ctx->{query_struct}{additional_data}{highlight_map}, + map {int($_)} @$id ); }; diff --git a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql index 71a2e7d76f..eee307dc4c 100644 --- a/Open-ILS/src/sql/Pg/300.schema.staged_search.sql +++ b/Open-ILS/src/sql/Pg/300.schema.staged_search.sql @@ -1324,7 +1324,7 @@ CREATE OR REPLACE VIEW search.best_tsconfig AS CREATE TYPE search.highlight_result AS ( id BIGINT, source BIGINT, field INT, value TEXT, highlight TEXT ); -CREATE OR REPLACE FUNCTION search.highlight_display_fields( +CREATE OR REPLACE FUNCTION search.highlight_display_fields_impl( rid BIGINT, tsq TEXT, field_list INT[] DEFAULT '{}'::INT[], @@ -1410,7 +1410,7 @@ $$ LANGUAGE SQL IMMUTABLE LEAKPROOF STRICT COST 10; CREATE OR REPLACE FUNCTION search.highlight_display_fields( rid BIGINT, - tsq_map HSTORE, -- { '(a | b) & c' => '1,2,3,4', ...} + tsq_map TEXT, -- { '(a | b) & c' => '1,2,3,4', ...} css_class TEXT DEFAULT 'oils_SH', hl_all BOOL DEFAULT TRUE, minwords INT DEFAULT 5, @@ -1420,18 +1420,25 @@ CREATE OR REPLACE FUNCTION search.highlight_display_fields( delimiter TEXT DEFAULT ' ... ' ) RETURNS SETOF search.highlight_result AS $f$ DECLARE - tsq TEXT; - fields TEXT; - afields INT[]; - seen INT[]; + tsq_hstore TEXT; + tsq TEXT; + fields TEXT; + afields INT[]; + seen INT[]; BEGIN - FOR tsq, fields IN SELECT key, value FROM each(tsq_map) LOOP + IF (tsq_map ILIKE 'hstore%') THEN + EXECUTE 'SELECT ' || tsq_map INTO tsq_hstore; + ELSE + tsq_hstore := tsq_map::HSTORE; + END IF; + + FOR tsq, fields IN SELECT key, value FROM each(tsq_hstore) LOOP SELECT ARRAY_AGG(unnest::INT) INTO afields FROM unnest(regexp_split_to_array(fields,',')); seen := seen || afields; RETURN QUERY - SELECT * FROM search.highlight_display_fields( + SELECT * FROM search.highlight_display_fields_impl( rid, tsq, afields, css_class, hl_all,minwords, maxwords, shortwords, maxfrags, delimiter ); diff --git a/Open-ILS/src/sql/Pg/950.data.seed-values.sql b/Open-ILS/src/sql/Pg/950.data.seed-values.sql index a66a12d52b..a8ef6e212e 100644 --- a/Open-ILS/src/sql/Pg/950.data.seed-values.sql +++ b/Open-ILS/src/sql/Pg/950.data.seed-values.sql @@ -364,7 +364,7 @@ INSERT INTO config.display_field_map (name, field, multi) VALUES ('creators', 37, TRUE), ('subject', 16, TRUE), ('isbn', 18, TRUE), - ('series_title', 1, FALSE), + ('series_title', 1, TRUE), ('subject_geographic', 11, TRUE), ('subject_name', 12, TRUE), ('subject_temporal', 13, TRUE), diff --git a/Open-ILS/src/sql/Pg/upgrade/WWWW.data.display-field-seed-data.sql b/Open-ILS/src/sql/Pg/upgrade/WWWW.data.display-field-seed-data.sql index fae9b210f9..e87c170300 100644 --- a/Open-ILS/src/sql/Pg/upgrade/WWWW.data.display-field-seed-data.sql +++ b/Open-ILS/src/sql/Pg/upgrade/WWWW.data.display-field-seed-data.sql @@ -136,7 +136,7 @@ UPDATE config.metabib_field SET display_field = TRUE WHERE id IN ( -- Map display field names to config.metabib_field entries INSERT INTO config.display_field_map (name, field, multi) VALUES - ('series_title', 1, FALSE), + ('series_title', 1, TRUE), ('subject_geographic', 11, TRUE), ('subject_name', 12, TRUE), ('subject_temporal', 13, TRUE), diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.highlight_search.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.highlight_search.sql index 7a4e5517a6..cc3f913bea 100644 --- a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.highlight_search.sql +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.highlight_search.sql @@ -338,7 +338,7 @@ CREATE OR REPLACE VIEW search.best_tsconfig AS CREATE TYPE search.highlight_result AS ( id BIGINT, source BIGINT, field INT, value TEXT, highlight TEXT ); -CREATE OR REPLACE FUNCTION search.highlight_display_fields( +CREATE OR REPLACE FUNCTION search.highlight_display_fields_impl( rid BIGINT, tsq TEXT, field_list INT[] DEFAULT '{}'::INT[], @@ -424,7 +424,7 @@ $$ LANGUAGE SQL IMMUTABLE LEAKPROOF STRICT COST 10; CREATE OR REPLACE FUNCTION search.highlight_display_fields( rid BIGINT, - tsq_map HSTORE, -- { '(a | b) & c' => '1,2,3,4', ...} + tsq_map TEXT, -- { '(a | b) & c' => '1,2,3,4', ...} css_class TEXT DEFAULT 'oils_SH', hl_all BOOL DEFAULT TRUE, minwords INT DEFAULT 5, @@ -434,18 +434,26 @@ CREATE OR REPLACE FUNCTION search.highlight_display_fields( delimiter TEXT DEFAULT ' ... ' ) RETURNS SETOF search.highlight_result AS $f$ DECLARE - tsq TEXT; - fields TEXT; - afields INT[]; - seen INT[]; + tsq_hstore HSTORE; + tsq TEXT; + fields TEXT; + afields INT[]; + seen INT[]; BEGIN - FOR tsq, fields IN SELECT key, value FROM each(tsq_map) LOOP + + IF (tsq_map ILIKE 'hstore%') THEN + EXECUTE 'SELECT ' || tsq_map INTO tsq_hstore; + ELSE + tsq_hstore := tsq_map::HSTORE; + END IF; + + FOR tsq, fields IN SELECT key, value FROM each(tsq_hstore) LOOP SELECT ARRAY_AGG(unnest::INT) INTO afields FROM unnest(regexp_split_to_array(fields,',')); seen := seen || afields; RETURN QUERY - SELECT * FROM search.highlight_display_fields( + SELECT * FROM search.highlight_display_fields_impl( rid, tsq, afields, css_class, hl_all,minwords, maxwords, shortwords, maxfrags, delimiter ); diff --git a/Open-ILS/src/templates/opac/parts/misc_util.tt2 b/Open-ILS/src/templates/opac/parts/misc_util.tt2 index ba507bd6eb..e8d48959b6 100644 --- a/Open-ILS/src/templates/opac/parts/misc_util.tt2 +++ b/Open-ILS/src/templates/opac/parts/misc_util.tt2 @@ -109,8 +109,12 @@ args.display_fields = {}; args.hl = {}; IF !CGI.param('no_highlight') && !search.no_highlight; + + junk = ctx.timelog('Fetching of highlighted display fields for bib(s) ' _ args.df_bib_list.list.join(', ')); args.display_field_list = ctx.fetch_display_fields(args.df_bib_list.list); + junk = ctx.timelog('Finished fetch of highlighted display fields for bib(s) ' _ args.df_bib_list.list.join(', ')); + junk = ctx.timelog('Mapping highlighted display fields for bib(s) ' _ args.df_bib_list.list.join(', ')); FOR df IN args.display_field_list; df_map = ctx.search_cdfm('field', df.field).0; df_name = df_map.name(); @@ -126,6 +130,7 @@ args.hl.$df_name = df.highlight || df.value; END; END; + junk = ctx.timelog('Finished mapping highlighted display fields for bib(s) ' _ args.df_bib_list.list.join(', ')); END; # Map item types to schema.org types; impedance mismatch :( -- 2.11.0