https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36350 --- Comment #66 from David Gustafsson <glasklas@gmail.com> --- (In reply to Jonathan Druart from comment #65)
1. diff --git a/Koha/ILL/Requests.pm b/Koha/ILL/Requests.pm
-use base qw(Koha::Objects::Mixin::ExtendedAttributes Koha::Objects); +use base qw(Koha::Objects);
This does not seem correct.
2.
diff --git a/Koha/Object.pm b/Koha/Object.pm
@@ -1017,6 +1017,9 @@ sub AUTOLOAD { my $accessor = sub { my $self = shift; if (@_) { + if ($self->isa("Koha::Object::CachedExpiration")) { + $self->_objects_cache_expire();
diff --git a/Koha/Object/CachedExpiration.pm b/Koha/Object/CachedExpiration.pm +sub _objects_cache_expire { + my ($self) = @_; + if ($self->_result->in_storage) { + my @primary_keys = $self->_result->id; + my $ids_cache_key = $self->_objects_cache_cache_key('find', @primary_keys); + $self->_objects_cache_clear($ids_cache_key); + } +}
Isn't it going to add one fetch for each insert/update statement?
3. _objects_cache_expire is unnecessary called when we store a new object (not blocker)
4. very quick benchmark to give an idea: ``` use Benchmark qw(:all); my $count = 10000; my $t = timeit($count, sub { my $l = Koha::Library->new({branchcode => 'foo', branchname => 'foo'})->store; $l->branchname('bar')->store; $l->delete; }); say timestr($t); ``` with the patches: 139 wallclock secs (59.03 usr + 5.42 sys = 64.45 CPU) @ 155.16/s (n=10000) without the patches: 133 wallclock secs (54.21 usr + 5.13 sys = 59.34 CPU) @ 168.52/s (n=10000)
``` use Benchmark qw(:all); my $count = 10000; my $i;
my $t = timeit($count, sub {
my $id = "foo" . $i++;
my $l = Koha::Library->new({branchcode => $id, branchname => $id})->store; $l->branchname('bar')->store;
}); ``` with the patches: 100 wallclock secs (45.76 usr + 4.15 sys = 49.91 CPU) @ 200.36/s (n=10000) without the patches: 98 wallclock secs (44.62 usr + 3.95 sys = 48.57 CPU) @ 205.89/s (n=10000)
say timestr($t);
=> it's going to be slightly slower for inserts in bulk.
I not sure I understand what you mean.
. _objects_cache_expire is unnecessary called when we store a new object.
Cache needs to be expired when an item is modified, or else we risk getting the modified item back if performing a find with the same id. So not sure what you mean by "unnecessary".
Isn't it going to add one fetch for each insert/update statement?
Perhaps you are confusing this line: my $ids_cache_key = $self->_objects_cache_cache_key('find',
@primary_keys);
With at database fetch? Is just the function for getting the cache key. There is going to be some overhead for setting columns, but considering its just a few percent, and the expiration logic can't be avoided, I don't think it's a problem. It should be a pretty rare operation to create of thousands of objects, in in those cases the extra overhead of cache expiration should be neglectable compared to other operations. -- You are receiving this mail because: You are watching all bug changes.