From 982e3d04f6976a6a2515536c15a4dda8ef98ff14 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 3 May 2017 14:20:49 +0800 Subject: [PATCH] PERF: Allow memory to be freed instead of fetching all the objects into memory at once. ``` MemoryProfiler.report do Jobs::UserEmail.new.execute(type: :mailing_list, user_id: user.id) end.pretty_print ``` Before: ``` Total allocated: 180096119 bytes (1962025 objects) Total retained: 2194 bytes (16 objects) allocated memory by gem ----------------------------------- 66979096 activerecord-4.2.8 43507184 nokogiri-1.7.1 43365188 mail-2.6.4 5960201 activesupport-4.2.8 5056267 discourse/lib 4835284 rack-mini-profiler-0.10.1 3825817 arel-6.0.4 2186088 i18n-0.8.1 1719330 discourse/app ``` After: ``` Total allocated: 161935975 bytes (1473940 objects) Total retained: 2234 bytes (17 objects) allocated memory by gem ----------------------------------- 45430264 activerecord-4.2.8 43568627 nokogiri-1.7.1 43430754 mail-2.6.4 11233878 rack-mini-profiler-0.10.1 5260825 activesupport-4.2.8 5054491 discourse/lib 2186088 i18n-0.8.1 1822494 arel-6.0.4 ``` --- app/mailers/user_notifications.rb | 20 +++++++++++++++---- app/models/post.rb | 2 -- .../user_notifications/mailing_list.html.erb | 12 +++++------ .../user_notifications/mailing_list.text.erb | 14 ++++++------- 4 files changed, 29 insertions(+), 19 deletions(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 06189a1ab22..0f91088de55 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -80,10 +80,21 @@ class UserNotifications < ActionMailer::Base @since = opts[:since] || 1.day.ago @since_formatted = short_date(@since) - @new_topic_posts = Post.mailing_list_new_topics(user, @since).group_by(&:topic) || {} - @existing_topic_posts = Post.mailing_list_updates(user, @since).group_by(&:topic) || {} - @posts_by_topic = @new_topic_posts.merge @existing_topic_posts - return unless @posts_by_topic.present? + topics = Topic + .joins(:posts) + .includes(:posts) + .for_digest(user, 100.years.ago) + .where("posts.created_at > ?", @since) + + unless user.staff? + topics = topics.where("posts.post_type <> ?", Post.types[:whisper]) + end + + @new_topics = topics.where("topics.created_at > ?", @since).uniq + @existing_topics = topics.where("topics.created_at <= ?", @since).uniq + @topics = topics.uniq + + return if @topics.empty? build_summary_for(user) opts = { @@ -93,6 +104,7 @@ class UserNotifications < ActionMailer::Base add_unsubscribe_link: true, unsubscribe_url: "#{Discourse.base_url}/email/unsubscribe/#{@unsubscribe_key}", } + apply_notification_styles(build_email(@user.email, opts)) end diff --git a/app/models/post.rb b/app/models/post.rb index 13d88aec1b8..bdbf598682b 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -77,8 +77,6 @@ class Post < ActiveRecord::Base q.order('posts.created_at ASC') } - scope :mailing_list_new_topics, ->(user, since) { for_mailing_list(user, since).where('topics.created_at > ?', since) } - scope :mailing_list_updates, ->(user, since) { for_mailing_list(user, since).where('topics.created_at <= ?', since) } delegate :username, to: :user diff --git a/app/views/user_notifications/mailing_list.html.erb b/app/views/user_notifications/mailing_list.html.erb index 77c13b81e7d..076204dd2fe 100644 --- a/app/views/user_notifications/mailing_list.html.erb +++ b/app/views/user_notifications/mailing_list.html.erb @@ -20,21 +20,21 @@

<%= t('user_notifications.mailing_list.new_topics') %>

<%= t('user_notifications.mailing_list.topic_updates') %>

- <% @posts_by_topic.each do |topic, posts| %> + <% @topics.each do |topic| %>

@@ -45,7 +45,7 @@ <%- unless SiteSetting.private_email? %>

- <% posts.each do |post| %> + <% topic.posts.each do |post| %>

diff --git a/app/views/user_notifications/mailing_list.text.erb b/app/views/user_notifications/mailing_list.text.erb index dfe0fe16831..90f63dbfd0d 100644 --- a/app/views/user_notifications/mailing_list.text.erb +++ b/app/views/user_notifications/mailing_list.text.erb @@ -1,28 +1,28 @@ <%- site_link = raw(@markdown_linker.create(@site_name, '/')) %> <%= raw(t 'user_notifications.mailing_list.why', site_link: site_link, date: @since_formatted) %> -<%- if @new_topic_posts.keys.present? -%> +<%- if @new_topics.present? -%> ### <%= t 'user_notifications.mailing_list.new_topics' %> - <%- @new_topic_posts.keys.each do |topic| %> + <%- @new_topics.each do |topic| %> <%= mailing_list_topic_text(topic) %> <%- end -%> <%- end -%> -<%- if @existing_topic_posts.keys.present? -%> +<%- if @existing_topics.present? -%> ### <%= t 'user_notifications.mailing_list.new_topics' %> - <%- @existing_topic_posts.keys.each do |topic| -%> + <%- @existing_topics.each do |topic| -%> <%= mailing_list_topic_text(topic) %> - <%= @existing_topic_posts[topic].length %> + <%= topic.posts.length %> <%- end -%> <%- end -%> -------------------------------------------------------------------------------- <%- unless SiteSetting.private_email? %> - <%- @posts_by_topic.keys.each do |topic| %> + <%- @topics.each do |topic| %> ### <%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %> - <%- @posts_by_topic[topic].each do |post| -%> + <%- topic.posts.each do |post| -%> <%= post.user.name || post.user.username %> - <%= post.created_at %> -------------------------------------------------------------------------------- <%= post.raw %>