Bug 16168 - Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm
Summary: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jacek Ablewicz
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 15342 16169
  Show dependency treegraph
 
Reported: 2016-03-30 10:16 UTC by Jacek Ablewicz
Modified: 2016-12-05 21:27 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm (2.53 KB, patch)
2016-03-30 10:30 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm (2.60 KB, patch)
2016-03-30 10:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 16168: Eliminate unneeded C4::Context->dbh calls in C4/Biblio.pm (2.67 KB, patch)
2016-04-03 20:28 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!