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
```
This commit is contained in:
Guo Xiang Tan 2017-05-03 14:20:49 +08:00
parent 90cd35c496
commit 982e3d04f6
4 changed files with 29 additions and 19 deletions

View File

@ -80,10 +80,21 @@ class UserNotifications < ActionMailer::Base
@since = opts[:since] || 1.day.ago @since = opts[:since] || 1.day.ago
@since_formatted = short_date(@since) @since_formatted = short_date(@since)
@new_topic_posts = Post.mailing_list_new_topics(user, @since).group_by(&:topic) || {} topics = Topic
@existing_topic_posts = Post.mailing_list_updates(user, @since).group_by(&:topic) || {} .joins(:posts)
@posts_by_topic = @new_topic_posts.merge @existing_topic_posts .includes(:posts)
return unless @posts_by_topic.present? .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) build_summary_for(user)
opts = { opts = {
@ -93,6 +104,7 @@ class UserNotifications < ActionMailer::Base
add_unsubscribe_link: true, add_unsubscribe_link: true,
unsubscribe_url: "#{Discourse.base_url}/email/unsubscribe/#{@unsubscribe_key}", unsubscribe_url: "#{Discourse.base_url}/email/unsubscribe/#{@unsubscribe_key}",
} }
apply_notification_styles(build_email(@user.email, opts)) apply_notification_styles(build_email(@user.email, opts))
end end

View File

@ -77,8 +77,6 @@ class Post < ActiveRecord::Base
q.order('posts.created_at ASC') 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 delegate :username, to: :user

View File

@ -20,21 +20,21 @@
<td> <td>
<h4 style="padding: 0 25px; margin: 10px 0;"><%= t('user_notifications.mailing_list.new_topics') %></h3> <h4 style="padding: 0 25px; margin: 10px 0;"><%= t('user_notifications.mailing_list.new_topics') %></h3>
<ul> <ul>
<% @new_topic_posts.each do |topic, posts| %> <% @new_topics.each do |topic| %>
<%= mailing_list_topic(topic, posts.length) %> <%= mailing_list_topic(topic, topic.posts.length) %>
<% end %> <% end %>
</ul> </ul>
<h4 style="padding: 0 25px; margin: 10px 0;"><%= t('user_notifications.mailing_list.topic_updates') %></h3> <h4 style="padding: 0 25px; margin: 10px 0;"><%= t('user_notifications.mailing_list.topic_updates') %></h3>
<ul> <ul>
<% @existing_topic_posts.each do |topic, posts| %> <% @existing_topics.each do |topic| %>
<%= mailing_list_topic(topic, posts.length) %> <%= mailing_list_topic(topic, topic.posts.length) %>
<% end %> <% end %>
</ul> </ul>
</td> </td>
</tr> </tr>
</table> </table>
<table style='width: 100%; background: #eee;' cellspacing='0' cellpadding='0'> <table style='width: 100%; background: #eee;' cellspacing='0' cellpadding='0'>
<% @posts_by_topic.each do |topic, posts| %> <% @topics.each do |topic| %>
<tr> <tr>
<td> <td>
<h3 style='padding: 20px 20px 10px; margin: 0;'> <h3 style='padding: 20px 20px 10px; margin: 0;'>
@ -45,7 +45,7 @@
<%- unless SiteSetting.private_email? %> <%- unless SiteSetting.private_email? %>
<tr> <tr>
<td style='border-left: 20px solid #eee; border-right: 20px solid #eee; border-bottom: 10px solid #eee; padding: 10px 10px; background: #fff;'> <td style='border-left: 20px solid #eee; border-right: 20px solid #eee; border-bottom: 10px solid #eee; padding: 10px 10px; background: #fff;'>
<% posts.each do |post| %> <% topic.posts.each do |post| %>
<div> <div>
<img style="float: left; width: 20px; padding-right: 5px;" src="<%= post.user.small_avatar_url %>" title="<%= post.user.username%>"> <img style="float: left; width: 20px; padding-right: 5px;" src="<%= post.user.small_avatar_url %>" title="<%= post.user.username%>">
<p style="font-size: 15px; color: #888"> <p style="font-size: 15px; color: #888">

View File

@ -1,28 +1,28 @@
<%- site_link = raw(@markdown_linker.create(@site_name, '/')) %> <%- site_link = raw(@markdown_linker.create(@site_name, '/')) %>
<%= raw(t 'user_notifications.mailing_list.why', site_link: site_link, date: @since_formatted) %> <%= 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' %> ### <%= t 'user_notifications.mailing_list.new_topics' %>
<%- @new_topic_posts.keys.each do |topic| %> <%- @new_topics.each do |topic| %>
<%= mailing_list_topic_text(topic) %> <%= mailing_list_topic_text(topic) %>
<%- end -%> <%- end -%>
<%- end -%> <%- end -%>
<%- if @existing_topic_posts.keys.present? -%> <%- if @existing_topics.present? -%>
### <%= t 'user_notifications.mailing_list.new_topics' %> ### <%= t 'user_notifications.mailing_list.new_topics' %>
<%- @existing_topic_posts.keys.each do |topic| -%> <%- @existing_topics.each do |topic| -%>
<%= mailing_list_topic_text(topic) %> <%= mailing_list_topic_text(topic) %>
<%= @existing_topic_posts[topic].length %> <%= topic.posts.length %>
<%- end -%> <%- end -%>
<%- end -%> <%- end -%>
-------------------------------------------------------------------------------- --------------------------------------------------------------------------------
<%- unless SiteSetting.private_email? %> <%- unless SiteSetting.private_email? %>
<%- @posts_by_topic.keys.each do |topic| %> <%- @topics.each do |topic| %>
### <%= raw(@markdown_linker.create(topic.title, topic.relative_url)) %> ### <%= 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.user.name || post.user.username %> - <%= post.created_at %>
-------------------------------------------------------------------------------- --------------------------------------------------------------------------------
<%= post.raw %> <%= post.raw %>