use proxies for item html, too

Some lame benchmarking on my box, dev, cache classes, many items:

No proxies:
    Fresh JSON:  175,  90,  90,  93,  82, 88, 158, 150, 85, 167 = 117.8
    Cached JSON: (none)
    Fresh HTML:  371, 327, 355, 328, 322, 346 = 341.5
    Cached HTML: 173, 123, 175, 187, 171, 179 = 168

Proxies:
    Fresh JSON:  175, 183, 269, 219, 195, 178 = 203.17
    Cached JSON:  88,  70,  89, 162,  80,  77 = 94.3
    Fresh HTML:  494, 381, 350, 334, 451, 372 = 397
    Cached HTML: 176, 170, 104, 101, 111, 116 = 129.7

So, overhead is significant, but the gains when cached (and that should be
all the time, since we currently have 0 evictions) are definitely worth
it. Worth pushing, and probably putting some future effort into reducing
overhead.

On production (again, lame), items#index was consistently averaging
73-74ms when super healthy, and 82ms when pets#index was being louder
than usual. For reference is all. This will probably perform
significantly worse at first (in JSON, anyway, since HTML is already
mostly cached), so it might be worth briefly warming the cache after
pushing.
This commit is contained in:
Emi Matchu 2013-06-26 23:39:04 -07:00
parent e42de795dd
commit 9e3cac82ec
5 changed files with 68 additions and 28 deletions

View file

@ -13,13 +13,14 @@ class ItemsController < ApplicationController
end end
# Note that we sort by name by hand, since we might have to use # Note that we sort by name by hand, since we might have to use
# fallbacks after the fact # fallbacks after the fact
# TODO: use proxies for everything!
output_format = params[:format] == :html ? :records : :proxies
@items = Item::Search::Query.from_text(@query, current_user). @items = Item::Search::Query.from_text(@query, current_user).
paginate(page: params[:page], per_page: per_page, as: output_format) paginate(page: params[:page], per_page: per_page, as: :proxies)
assign_closeted! assign_closeted!
respond_to do |format| respond_to do |format|
format.html { render } format.html {
@items.prepare_partial(:item_link_partial)
render
}
format.json { format.json {
@items.prepare_method(:as_json) @items.prepare_method(:as_json)
render json: {items: @items, total_pages: @items.total_pages} render json: {items: @items, total_pages: @items.total_pages}

View file

@ -288,6 +288,12 @@ class Item < ActiveRecord::Base
translatable_locales - translated_locales translatable_locales - translated_locales
end end
def method_cached?(method_name)
# No methods are cached on a full item. This is for duck typing with item
# proxies.
false
end
def self.all_by_ids_or_children(ids, swf_assets) def self.all_by_ids_or_children(ids, swf_assets)
swf_asset_ids = [] swf_asset_ids = []
swf_assets_by_id = {} swf_assets_by_id = {}

View file

@ -3,33 +3,47 @@ class Item
include FragmentLocalization include FragmentLocalization
attr_reader :id attr_reader :id
attr_writer :item attr_writer :item, :owned, :wanted
delegate :description, :name, :nc?, :thumbnail_url, to: :item
def initialize(id) def initialize(id)
@id = id @id = id
@known_method_outputs = {} @known_outputs = {method: {}, partial: {}}
end
def method_cached?(method_name)
# TODO: is there a way to cache nil? Right now we treat is as a miss.
# We eagerly read the cache rather than just check if the value exists,
# which will usually cut down on cache requests.
@known_method_outputs[method_name] ||= Rails.cache.read(
method_fragment_key(method_name))
!@known_method_outputs[method_name].nil?
end end
def as_json(options={}) def as_json(options={})
cache_method(:as_json) cache_method(:as_json)
end end
def cached?(type, name)
# TODO: is there a way to cache nil? Right now we treat is as a miss.
# We eagerly read the cache rather than just check if the value exists,
# which will usually cut down on cache requests.
@known_outputs[type][name] ||= Rails.cache.read(fragment_key(type, name))
!@known_outputs[type][name].nil?
end
def owned?
@owned
end
def to_partial_path
# HACK: could break without warning!
Item._to_partial_path
end
def wanted?
@wanted
end
private private
def cache_method(method_name, &block) def cache_method(method_name, &block)
# Two layers of cache: a local copy, in case the method is called again, # Two layers of cache: a local copy, in case the method is called again,
# and then the Rails cache, before we hit the actual method call. # and then the Rails cache, before we hit the actual method call.
@known_method_outputs[method_name] ||= begin @known_outputs[method_name] ||= begin
key = method_fragment_key(method_name) key = fragment_key(:method, method_name)
Rails.cache.fetch(key) { item.send(method_name) } Rails.cache.fetch(key) { item.send(method_name) }
end end
end end
@ -38,8 +52,10 @@ class Item
@item ||= Item.find(@id) @item ||= Item.find(@id)
end end
def method_fragment_key(method_name) def fragment_key(type, name)
localize_fragment_key("item/#{@id}##{method_name}", I18n.locale) prefix = type == :partial ? 'views/' : ''
base = localize_fragment_key("items/#{@id}##{name}", I18n.locale)
prefix + base
end end
end end
end end

View file

@ -1,16 +1,37 @@
class Item class Item
class ProxyArray < Array class ProxyArray < Array
METHOD_SCOPES = {as_json: Item.includes(:translations)} # TODO: do we really need to include translations? The search documents
# know the proper name for each locale, so proxies can tell their
# parent items what their names are and save the query entirely.
SCOPES = HashWithIndifferentAccess.new({
method: {
as_json: Item.includes(:translations),
},
partial: {
item_link_partial: Item.includes(:translations)
}
})
def initialize(ids) def initialize(ids)
self.replace(ids.map { |id| Proxy.new(id.to_i) }) self.replace(ids.map { |id| Proxy.new(id.to_i) })
end end
def prepare_method(method_name) def prepare_method(name)
prepare(:method, name)
end
def prepare_partial(name)
prepare(:partial, name)
end
private
def prepare(type, name)
missed_proxies_by_id = self. missed_proxies_by_id = self.
reject { |p| p.method_cached?(method_name) }. reject { |p| p.cached?(type, name) }.
index_by(&:id) index_by(&:id)
item_scope = METHOD_SCOPES[method_name.to_sym] || Item.scoped item_scope = SCOPES[type][name]
raise "unexpected #{type} #{name.inspect}" unless item_scope
item_scope.find(missed_proxies_by_id.keys).each do |item| item_scope.find(missed_proxies_by_id.keys).each do |item|
missed_proxies_by_id[item.id].item = item missed_proxies_by_id[item.id].item = item
end end

View file

@ -57,11 +57,7 @@ class User < ActiveRecord::Base
# Assigning these items to a hash by ID means that we don't have to go # Assigning these items to a hash by ID means that we don't have to go
# N^2 searching the items list for items that match the given IDs or vice # N^2 searching the items list for items that match the given IDs or vice
# versa, and everything stays a lovely O(n) # versa, and everything stays a lovely O(n)
items_by_id = {} items_by_id = items.group_by(&:id)
items.each do |item|
items_by_id[item.id] ||= []
items_by_id[item.id] << item
end
closet_hangers.where(:item_id => items_by_id.keys).each do |hanger| closet_hangers.where(:item_id => items_by_id.keys).each do |hanger|
items = items_by_id[hanger.item_id] items = items_by_id[hanger.item_id]
items.each do |item| items.each do |item|