From e42de795dd2d476fb1ed4cb7c6322be7e577d808 Mon Sep 17 00:00:00 2001 From: Matchu Date: Wed, 26 Jun 2013 23:01:12 -0700 Subject: [PATCH] Use item proxies for JSON caching That is, once we get our list of IDs from the search engine, only fetch records whose JSON we don't already have cached. It's simpler here to use as_json, but it'd probably be even faster if I figure out how to serve a plain JSON string from a Rails controller. In the meantime, requests of entirely cached items are coming in at about 85ms on average on my box (dev, cache classes, many items), about 10ms better than the last iteration. --- app/controllers/items_controller.rb | 29 ++++++++++++------- app/flex/flex_search_extender.rb | 16 +++++++--- app/models/item.rb | 4 +++ app/models/item/proxy.rb | 45 +++++++++++++++++++++++++++++ app/models/item/proxy_array.rb | 19 ++++++++++++ app/models/item/search/query.rb | 11 +++++-- 6 files changed, 107 insertions(+), 17 deletions(-) create mode 100644 app/models/item/proxy.rb create mode 100644 app/models/item/proxy_array.rb diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb index c54105d8..3704f63c 100644 --- a/app/controllers/items_controller.rb +++ b/app/controllers/items_controller.rb @@ -13,20 +13,29 @@ class ItemsController < ApplicationController end # Note that we sort by name by hand, since we might have to use # 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). - paginate(:page => params[:page], :per_page => per_page) + paginate(page: params[:page], per_page: per_page, as: output_format) assign_closeted! respond_to do |format| format.html { render } - format.json { render :json => {:items => @items, :total_pages => @items.total_pages} } - format.js { render :json => {:items => @items, :total_pages => @items.total_pages}, :callback => params[:callback] } + format.json { + @items.prepare_method(:as_json) + render json: {items: @items, total_pages: @items.total_pages} + } + format.js { + @items.prepare_method(:as_json) + render json: {items: @items, total_pages: @items.total_pages}, + callback: params[:callback] + } end end elsif params.has_key?(:ids) && params[:ids].is_a?(Array) @items = Item.includes(:translations).find(params[:ids]) assign_closeted! respond_to do |format| - format.json { render :json => @items } + format.json { render json: @items } end else respond_to do |format| @@ -35,7 +44,7 @@ class ItemsController < ApplicationController @newest_items = Item.newest.includes(:translations).limit(18) end } - format.js { render :json => {:error => '$q required'}} + format.js { render json: {error: '$q required'}} end end end @@ -47,10 +56,10 @@ class ItemsController < ApplicationController format.html do unless localized_fragment_exist?("items/#{@item.id} info") @occupied_zones = @item.occupied_zones( - :scope => Zone.includes_translations.alphabetical + scope: Zone.includes_translations.alphabetical ) @restricted_zones = @item.restricted_zones( - :scope => Zone.includes_translations.alphabetical + scope: Zone.includes_translations.alphabetical ) end @@ -72,7 +81,7 @@ class ItemsController < ApplicationController end @current_user_quantities = Hash.new(0) # default is zero - hangers = current_user.closet_hangers.where(:item_id => @item.id). + hangers = current_user.closet_hangers.where(item_id: @item.id). select([:owned, :list_id, :quantity]) hangers.each do |hanger| @@ -122,8 +131,8 @@ class ItemsController < ApplicationController @items = [] respond_to do |format| format.html { flash.now[:alert] = e.message; render } - format.json { render :json => {:error => e.message} } - format.js { render :json => {:error => e.message}, + format.json { render :json => {error: e.message} } + format.js { render :json => {error: e.message}, :callback => params[:callback] } end end diff --git a/app/flex/flex_search_extender.rb b/app/flex/flex_search_extender.rb index 39adae17..06d59151 100644 --- a/app/flex/flex_search_extender.rb +++ b/app/flex/flex_search_extender.rb @@ -7,14 +7,22 @@ module FlexSearchExtender def self.should_extend?(response) true end - + + def proxied_collection + Item.build_proxies(collection.map(&:_id)).tap do |proxies| + proxies.extend Flex::Result::Collection + proxies.setup(self['hits']['total'], variables) + end + end + def scoped_loaded_collection(options) options[:scopes] ||= {} @loaded_collection ||= begin records_by_class_and_id_str = {} - # returns a structure like {Comment=>[{"_id"=>"123", ...}, {...}], BlogPost=>[...]} - h = collection.group_by { |d| d.mapped_class(should_raise=true) } - h.each do |klass, docs| + grouped_collection = collection.group_by { |d| + d.mapped_class(should_raise=true) + } + grouped_collection.each do |klass, docs| record_ids = docs.map(&:_id) scope = options[:scopes][klass.name] || klass.scoped records = scope.find(record_ids) diff --git a/app/models/item.rb b/app/models/item.rb index b1f2dc1d..4ee78eba 100644 --- a/app/models/item.rb +++ b/app/models/item.rb @@ -404,6 +404,10 @@ class Item < ActiveRecord::Base items.values end + def self.build_proxies(ids) + Item::ProxyArray.new(ids) + end + class << self MALL_HOST = 'ncmall.neopets.com' MALL_MAIN_PATH = '/mall/shop.phtml' diff --git a/app/models/item/proxy.rb b/app/models/item/proxy.rb new file mode 100644 index 00000000..f90e3464 --- /dev/null +++ b/app/models/item/proxy.rb @@ -0,0 +1,45 @@ +class Item + class Proxy + include FragmentLocalization + + attr_reader :id + attr_writer :item + + def initialize(id) + @id = id + @known_method_outputs = {} + 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 + + def as_json(options={}) + cache_method(:as_json) + end + + private + + def cache_method(method_name, &block) + # 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. + @known_method_outputs[method_name] ||= begin + key = method_fragment_key(method_name) + Rails.cache.fetch(key) { item.send(method_name) } + end + end + + def item + @item ||= Item.find(@id) + end + + def method_fragment_key(method_name) + localize_fragment_key("item/#{@id}##{method_name}", I18n.locale) + end + end +end \ No newline at end of file diff --git a/app/models/item/proxy_array.rb b/app/models/item/proxy_array.rb new file mode 100644 index 00000000..03ce5a66 --- /dev/null +++ b/app/models/item/proxy_array.rb @@ -0,0 +1,19 @@ +class Item + class ProxyArray < Array + METHOD_SCOPES = {as_json: Item.includes(:translations)} + + def initialize(ids) + self.replace(ids.map { |id| Proxy.new(id.to_i) }) + end + + def prepare_method(method_name) + missed_proxies_by_id = self. + reject { |p| p.method_cached?(method_name) }. + index_by(&:id) + item_scope = METHOD_SCOPES[method_name.to_sym] || Item.scoped + item_scope.find(missed_proxies_by_id.keys).each do |item| + missed_proxies_by_id[item.id].item = item + end + end + end +end \ No newline at end of file diff --git a/app/models/item/search/query.rb b/app/models/item/search/query.rb index 7c2f728b..1f99d7c7 100644 --- a/app/models/item/search/query.rb +++ b/app/models/item/search/query.rb @@ -93,9 +93,14 @@ class Item end result = FlexSearch.item_search(final_flex_params) - result.scoped_loaded_collection( - :scopes => {'Item' => Item.includes(:translations)} - ) + + if options[:as] == :proxies + result.proxied_collection + else + result.scoped_loaded_collection( + :scopes => {'Item' => Item.includes(:translations)} + ) + end end # Load the text query labels from I18n, so that when we see, say,