https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36350
--- Comment #66 from David Gustafsson <[email protected]> --- (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. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
