[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365

--- Comment #6 from Jacek Ablewicz <abl at biblos.pk.edu.pl> ---
(In reply to Marcel de Rooy from comment #3)
> --- a/C4/Items.pm
> +++ b/C4/Items.pm
> 
> @@ -2815,6 +2815,10 @@ sub PrepareItemrecordDisplay {
>      my $dbh = C4::Context->dbh;
>      $frameworkcode = &GetFrameworkCode($bibnum) if $bibnum;
>      my ( $itemtagfield, $itemtagsubfield ) = &GetMarcFromKohaField(
> "items.itemnumber", $frameworkcode );
> +
> +    # it would be perhaps beneficial (?) to call GetMarcStructure with
> 'unsafe' parameter
> +    # for performance reasons, but $tagslib may be passed to
> $plugin->build(), and there
> +    # is no way to ensure that this structure is not getting corrupted
> somewhere in there
>      my $tagslib = &GetMarcStructure( 1, $frameworkcode );
> 
> Hm. I have the strong impression that the item plugins do not even read the
> tagslib structure. I kept the parameter in terms of backward compatibility
> for custom plugins that would need it. It might be an idea to let these
> custom (item) plugins call GetMarcStructure itself when they really need
> it.. But yes, that would be another report :)
> 
> The assumption that tagslib would be corrupted by a plugin is not very
> realistic imo; most do not even read, not to mention change. If the included
> plugins do not, how far should we go in protecting the author of a custom
> plugin from the potential threat he created himself? We can't stop him now
> from doing all kinds of things..

+1, overcautiousness is not always a virtue ;). I'll post a follow-up.

> So I would not object to calling it unsafe here too. In my tests it makes a
> remarkable difference: 0.0015 versus 0.046 per GetMarcStructure call. Please
> note too that PrepareItemrecordDisplay is called twice in a for loop (in
> serials code: for each item or subscription).

Ouch, for serial subscriptions with 'serialsadditems' enabled, it might be
called a lot of times for a single subscription. This is seriously unoptimized
function btw, traverses a whole MARC structure while it only needs 33 or so
entries, and also generates a lot of unneeded database traffic. But that's
outside the scope of this report.

-- 
You are receiving this mail because:
You are watching all bug changes.