activity_notification icon indicating copy to clipboard operation
activity_notification copied to clipboard

Performance issue with a large target collection

Open markedmondson opened this issue 4 years ago • 0 comments

Steps to reproduce

In NotificationApi#notify the call to targets.blank? (https://github.com/simukappu/activity_notification/blob/master/lib/activity_notification/apis/notification_api.rb#L231) loads all of the targets, for a large collection, this loads all the records into memory and it seems pretty pointless as the subsequent call is to NotificationApi#notify_all which iterates the enumerable.

Additionally, that map call in NotificationApi#notify_all (https://github.com/simukappu/activity_notification/blob/master/lib/activity_notification/apis/notification_api.rb#L293) also loads all of the collection objects into memory, it'd be nicer if we could use something like .in_batches from the relation in order to do this with more performance on a large set.

Making this change would require a change to the API though as returning an Array of the generated notifications will be trickier, additionally, it looks like the NotifyAllJob passes the collection and not the relation.

I'm happy to open a PR if you have some ideas on what might make a good approach.

System configuration

activity_notification gem version: 2.1.3 Rails version: 6.0 ORM (ActiveRecord, Mongoid or Dynamoid): AR

markedmondson avatar Sep 02 '20 05:09 markedmondson