From 3c127569fe57d7b2881725680084e627ae74501c Mon Sep 17 00:00:00 2001 From: Matchu Date: Sun, 23 Jun 2013 22:35:27 -0700 Subject: [PATCH] stop caching item preview species images, and fix the bad query instead Most of the reasoning is documented in the big comment. In short, we tried to solve the problem with caching, but the caching should hardly be necessary now that the bottleneck should be fixed. We'll see on production if it actually solves the whole problem, but I've confirmed in the console that redefining this function makes random_basic_per_species (as called during rendering) a ton faster. And this way we keep our randomness, woo! --- app/models/pet_type.rb | 10 +++++++++- app/views/items/show.html.haml | 7 +------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/models/pet_type.rb b/app/models/pet_type.rb index 7b730368..8bb8b419 100644 --- a/app/models/pet_type.rb +++ b/app/models/pet_type.rb @@ -23,7 +23,15 @@ class PetType < ActiveRecord::Base lambda { includes({:color => :translations, :species => :translations}) } def self.standard_pet_types_by_species_id - PetType.where(:color_id => Color.basic).includes_child_translations. + # If we include Color.basic in the query, rather than getting the ID array + # first, it'll do this as a big fat JOIN unnecessarily. On production, + # there are tons of pet types and translations to send down, so tons of + # duplicate data is sent and must be merged together properly, both of + # which take serious time. So, this is a surprisingly significant + # performance boost - though I was surprised and impressed that Rails + # includes relations as subqueries. That's cool. + basic_color_ids = Color.basic.select([:id]).map(&:id) + PetType.where(color_id: basic_color_ids).includes_child_translations. group_by(&:species_id) end diff --git a/app/views/items/show.html.haml b/app/views/items/show.html.haml index c0c053fd..8ba6f8a4 100644 --- a/app/views/items/show.html.haml +++ b/app/views/items/show.html.haml @@ -79,12 +79,7 @@ #item-preview #item-preview-species - -# Does this cache ever need to be swept? I don't think so. These days we - -# can identify body-specific items instantly, and, if it's instead - -# particular to a single species, we definitely know that the first time - -# we render. - - localized_cache "items/#{@item.id} item_preview_species" do - = standard_species_images_for(@item) + = standard_species_images_for(@item) #item-preview-error #item-preview-swf= t '.preview.requirements_not_met'