Bug 16168

Summary: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm
Product: Koha Reporter: Jacek Ablewicz <abl>
Component: Architecture, internals, and plumbingAssignee: Jacek Ablewicz <abl>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: brendan, jonathan.druart
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16156
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 15342, 16169    
Attachments: Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm
Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm
[PASSED QA] Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm

Description Jacek Ablewicz 2016-03-30 10:16:14 UTC
Right now, ->dbh calls are actually quite expensive (they involve DB connection health checks, each and every time). Some speed-sensitive subroutines inside C4/Biblio.pm (GetMarcStructure, GetAuthorisedValueDesc) have this statement

    my $dbh = C4::Context->dbh;

on top of the code, but they don't always/don't usually need DB handle - not at that stage at least. This trivial patch eliminates unneeded ->dbh calls in those subroutines. With it, average GetMarcStructure() running time goes down from 14 miliseconds to 9 miliseconds (on top of Bug 16166), it also makes catalogue search profiling a bit easier.
Comment 1 Jacek Ablewicz 2016-03-30 10:30:05 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-03-30 10:53:41 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-03-30 10:54:44 UTC
Comment on attachment 49679 [details] [review]
Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm

Review of attachment 49679 [details] [review]:
-----------------------------------------------------------------

::: C4/Biblio.pm
@@ +2618,4 @@
>  
>  sub TransformMarcToKoha {
>      my ( $dbh, $record, $frameworkcode, $limit_table ) = @_;
> +    ## FIXME: $dbh parameter is never used inside this subroutine ???

Do you plan to work on it?
Otherwise I will provide a patch.
Comment 4 Jonathan Druart 2016-03-30 11:04:12 UTC
(In reply to Jonathan Druart from comment #3)
> Comment on attachment 49679 [details] [review] [review]
> Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm
> 
> Review of attachment 49679 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Biblio.pm
> @@ +2618,4 @@
> >  
> >  sub TransformMarcToKoha {
> >      my ( $dbh, $record, $frameworkcode, $limit_table ) = @_;
> > +    ## FIXME: $dbh parameter is never used inside this subroutine ???

Done on bug 16169.
Comment 5 Jacek Ablewicz 2016-03-30 11:04:48 UTC
(In reply to Jonathan Druart from comment #3)
> Comment on attachment 49679 [details] [review] [review]
> Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm
> 
> Review of attachment 49679 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Biblio.pm
> @@ +2618,4 @@
> >  
> >  sub TransformMarcToKoha {
> >      my ( $dbh, $record, $frameworkcode, $limit_table ) = @_;
> > +    ## FIXME: $dbh parameter is never used inside this subroutine ???
> 
> Do you plan to work on it?

Nah, too many changes in too many places ;)
Comment 6 Katrin Fischer 2016-04-03 20:28:43 UTC
Created attachment 49828 [details] [review]
[PASSED QA] Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm

Right now, ->dbh calls are actually quite expensive (they involve
DB connection health checks, each and every time). Some speed-sensitive
subroutines inside C4/Biblio.pm (GetMarcStructure, GetAuthorisedValueDesc)
have this statement

    my $dbh = C4::Context->dbh;

on top of the code, but they don't always/don't usually need DB
handle - not at that stage at least. This trivial patch eliminates
unneeded ->dbh calls in those subroutines. With it, average
GetMarcStructure() running time goes down from 14 miliseconds
to 9 miliseconds (on top of Bug 16166), it also makes catalogue
search profiling a bit easier.

Test plan:

1) apply patch
2) ensure that catalogue searches are still working
3) run t/*Biblio* tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Brendan Gallagher 2016-04-07 00:04:54 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks!