1
0
Fork 0
forked from OpenNeo/impress

Simplify item_link rendering

In the interest of clearing out Resque, I'm just gonna remove a lot of our more complex caching stuff, and we can do a perf pass for things like big item list pages once everything's upgraded. (I'm hopeful that the upgrades themselves improve perf; and if not, that some improved sensibilities 10 years later can find simpler approaches.)
This commit is contained in:
Matchu 2023-08-02 11:34:27 -07:00 committed by Matchu
parent 44a00146aa
commit 02abd4e07f
6 changed files with 2 additions and 35 deletions

View file

@ -44,14 +44,11 @@ class ClosetHangersController < ApplicationController
visible_groups = @user.closet_hangers_groups_visible_to(@perspective_user) visible_groups = @user.closet_hangers_groups_visible_to(@perspective_user)
@unlisted_closet_hangers_by_owned = find_unlisted_closet_hangers_by_owned(visible_groups) @unlisted_closet_hangers_by_owned = find_unlisted_closet_hangers_by_owned(visible_groups)
item_proxies = Item::ProxyArray.new
items = [] items = []
@closet_lists_by_owned.each do |owned, lists| @closet_lists_by_owned.each do |owned, lists|
lists.each do |list| lists.each do |list|
list.hangers.each do |hanger| list.hangers.each do |hanger|
hanger.item_proxy = Item::Proxy.new(hanger.item)
item_proxies << hanger.item_proxy
items << hanger.item items << hanger.item
end end
end end
@ -59,14 +56,10 @@ class ClosetHangersController < ApplicationController
@unlisted_closet_hangers_by_owned.each do |owned, hangers| @unlisted_closet_hangers_by_owned.each do |owned, hangers|
hangers.each do |hanger| hangers.each do |hanger|
hanger.item_proxy = Item::Proxy.new(hanger.item)
item_proxies << hanger.item_proxy
items << hanger.item items << hanger.item
end end
end end
item_proxies.prepare_partial(:item_link_partial)
if @public_perspective && user_signed_in? if @public_perspective && user_signed_in?
current_user.assign_closeted_to_items!(items) current_user.assign_closeted_to_items!(items)
end end

View file

@ -116,27 +116,7 @@ module ItemsHelper
end end
def render_item_link(item) def render_item_link(item)
# I've discovered that checking the cache *before* attempting to render the render(partial: 'items/item_link', locals: {item: item})
# partial is significantly faster: moving the cache line out here instead
# of having it wrap the partial's content speeds up closet_hangers#index
# rendering time by about a factor of 2. It's uglier, but this call happens
# a lot, so the performance gain is definitely worth it. I'd be interested
# in a more legit partial caching abstraction, but, for now, this will do.
# Because this is a returned-string helper, but uses a buffer-output
# helper, we have to do some indirection. Fake that the render is in a
# template, then capture the resulting buffer output.
capture do
# Try to read from the prepared proxy's known partial output, if it's
# even a proxy at all.
if item.respond_to?(:known_partial_output)
prepared_output = item.known_partial_output(:item_link_partial).try(:html_safe)
else
prepared_output = nil
end
prepared_output || localized_cache("items/#{item.id}#item_link_partial") do
safe_concat render(partial: 'items/item_link', locals: {item: item})
end
end
end end
private private

View file

@ -3,8 +3,6 @@ class ClosetHanger < ActiveRecord::Base
belongs_to :list, :class_name => 'ClosetList' belongs_to :list, :class_name => 'ClosetList'
belongs_to :user belongs_to :user
attr_accessor :item_proxy
delegate :name, to: :item, prefix: true delegate :name, to: :item, prefix: true
validates :item_id, :uniqueness => {:scope => [:user_id, :owned, :list_id]} validates :item_id, :uniqueness => {:scope => [:user_id, :owned, :list_id]}

View file

@ -6,9 +6,6 @@ class Item
SCOPES = HashWithIndifferentAccess.new({ SCOPES = HashWithIndifferentAccess.new({
method: { method: {
as_json: Item.includes(:translations), as_json: Item.includes(:translations),
},
partial: {
item_link_partial: Item.includes(:translations)
} }
}) })

View file

@ -16,7 +16,6 @@ class Item
private private
def self.expire_cache_for(item) def self.expire_cache_for(item)
expire_fragment_in_all_locales("items/#{item.id}#item_link_partial")
expire_fragment_in_all_locales("items/#{item.id} header") expire_fragment_in_all_locales("items/#{item.id} header")
expire_fragment_in_all_locales("items/#{item.id} info") expire_fragment_in_all_locales("items/#{item.id} info")
expire_key_in_all_locales("items/#{item.id}#as_json") expire_key_in_all_locales("items/#{item.id}#as_json")

View file

@ -1,4 +1,4 @@
%div{'class' => closet_hanger_partial_class(closet_hanger), 'data-item-id' => closet_hanger.item_id, 'data-quantity' => closet_hanger.quantity, 'data-id' => closet_hanger.id} %div{'class' => closet_hanger_partial_class(closet_hanger), 'data-item-id' => closet_hanger.item_id, 'data-quantity' => closet_hanger.quantity, 'data-id' => closet_hanger.id}
= render_item_link(closet_hanger.item_proxy || closet_hanger.item) = render_item_link(closet_hanger.item)
.quantity{:class => "quantity-#{closet_hanger.quantity}"} .quantity{:class => "quantity-#{closet_hanger.quantity}"}
%span= closet_hanger.quantity %span= closet_hanger.quantity