[Koha-bugs] [Bug 17941] CanBookBeRenewed is very inefficient/slow

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

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

--- Comment #11 from Francesco Rivetti <oha at oha.it> ---
(In reply to Alex Sassmannshausen from comment #10)
> - Documentation for Attributes mentions it is experimental and the internal
> API may change.  We may need to consider whether this is a risk for Koha? 
> Does anyone know what the status is on that?
> (http://perldoc.perl.org/attributes.html)

yes, I read it as some attributes might change, or added. I used uppercase
naming to avoid future conflicts.

> - I too, like Jonathan, worry slightly about Plack implications.  It may
> just be that I don't quite understand how long the cache exists — but it
> seems to me that in a Plack context the cache that is created by memoization
> here would not be busted.  This would be problematic as follows:
> 
>  + GetMember memoizes a call to a specific borrowernumber
>  + Another user in Plack updates the borrower details
>  + Next call to GetMember returns memoized result, not the updated result.
> 
> How is the cache expired?

I'm working on documenting that hack a bit better, but the whole system relies
on:

local $CACHE = $CACHE//{};

this is a bit of perl trickery here, because the "local" will make a new
"version" of $CACHE while the scope it is in is still alive.

on the other end, the new "version" will use the same "instance" if available,
which means that you can be sure that only one cache is shared but it won't
live longer than the life of the first pure function life.

in other words, when the { local $CACHE = $CACHE//{}; .... } block goes out of
scope, it will revert to the previous version of CACHE, which will (and MUST
be) undef at plack time.

> To be specific, some of the calls that are Memoized are only Idempotent in
> the context of no other db updates happening.  In CGI context, this is no
> problem.  In long-living Plack processes this *may* be a problem.

You are absolutely right, and it is designed to be safe in this case.

you can actually push even further.

sub hybrid {
  pure(); # all happening inside this function will be memoized _temporary_
  db_change();
  pure(); # a new cache will be used here, with new data from db
}

or, in case you need it you could:

sub hybrid {

  K::U::PF::pure { # start a new cache
    for (...) {
      pure($_);
    }
  }; # cache is destroyed

  db_change();

  pure(); # new cache
}

I'm also adding another attribute which will throw an error in case like this

sub db_changes : ImpureFunction {
  ...
}

sub pure : PureFunction {
  db_changes(); # db_changes is marked as not pure, and will complain
}

and while we are in topic, any suggestions for the "Impure" function?

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