[buddypress-trac] [BuddyPress Trac] #7130: bp_notifications_get_all_notifications_for_user gets all records and can cause memory leak

[buddypress-trac]

#7130: bp_notifications_get_all_notifications_for_user gets all records and can
cause memory leak
-------------------------------------+-----------------------------
 Reporter:  hberberoglu              |       Owner:
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  Future Release
Component:  Toolbar & Notifications  |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:                           |
-------------------------------------+-----------------------------

Comment (by m_uysl):

 I think `bp_notifications_get_unread_notification_count` should use
 `BP_Notifications_Notification::get_total_count` and
 `bp_notifications_get_notifications_for_user` shouldn't make expensive
 calculations, we can use group query to fetch latest unread notifications
 and  total count.

 So, I just added the patch that reduces memory usage.

 Benchmarking:

 ||notification count|| unread || external object cache || patch status ||
 memory usage ||
 || 37980 || 37977  || none || without patch || 141,150 kB ||
 || 37980 || 37977  || memcache || without patch || 118,976 kB ||
 || 30001 || 26999  || none || without patch || 109,732 kB ||
 || 30001 || 26999  || memcache || without patch || 94,342 kB ||
 || 37980 || 37977  || none || with patch || 41,136 kB ||
 || 37980 || 37977  || memcache || with patch || 40,056 kB ||
 || 30001 || 26999  || none ||  with patch || 41,138 kB ||
 || 30001 || 26999  || memcache ||  with patch || 40,056 kB ||


 (Probably, the patch needs to be updated, it may not be ready to merge but
 can give some thoughts.)

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7130#comment:4>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac