From 65e7cd1d1d62a911da716eac37d9e687075f5b02 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 12 Nov 2014 10:01:07 +1100 Subject: [PATCH] Revert "PERF: use distributed cache for site text and category slugs" This reverts commit a97f2eee054bbd5a252e7cc28ea11210b2a8a82d. --- app/models/category.rb | 27 ++++-------------------- app/models/site_text.rb | 6 ++++-- lib/sass/discourse_stylesheets.rb | 34 ++++++++++++++++++++---------- lib/site_setting_extension.rb | 2 +- lib/site_text_class_methods.rb | 35 +++++++++++++++++++++++++------ 5 files changed, 61 insertions(+), 43 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index 9af8a4ca7b4..dad287a2afb 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -1,6 +1,3 @@ -require_dependency 'distributed_cache' -require_dependency 'sass/discourse_stylesheets' - class Category < ActiveRecord::Base include Positionable @@ -352,26 +349,10 @@ SQL id == SiteSetting.uncategorized_category_id end - @@url_cache = DistributedCache.new('category_url') - - after_save do - # parent takes part in url calculation - # any change could invalidate multiples - @@url_cache.clear - end - def url - url = @@url_cache[self.id] - unless url - url = "/category" - url << "/#{parent_category.slug}" if parent_category_id - url << "/#{slug}" - url.freeze - - @@url_cache[self.id] = url - end - - url + url = "/category" + url << "/#{parent_category.slug}" if parent_category_id + url << "/#{slug}" end # If the name changes, try and update the category definition topic too if it's @@ -385,7 +366,7 @@ SQL end def publish_discourse_stylesheet - DiscourseStylesheets.cache.clear + MessageBus.publish("/discourse_stylesheet", self.name) end end diff --git a/app/models/site_text.rb b/app/models/site_text.rb index f3f7559d5cd..03e0dabbdc3 100644 --- a/app/models/site_text.rb +++ b/app/models/site_text.rb @@ -1,16 +1,18 @@ require_dependency 'site_text_type' require_dependency 'site_text_class_methods' -require_dependency 'distributed_cache' class SiteText < ActiveRecord::Base + # needed for site text class methods + @mutex = Mutex.new + @text_for_cache = {} extend SiteTextClassMethods self.primary_key = 'text_type' validates_presence_of :value after_save do - SiteText.text_for_cache.clear + MessageBus.publish '/text_for', self.text_type end def self.formats diff --git a/lib/sass/discourse_stylesheets.rb b/lib/sass/discourse_stylesheets.rb index 35108405028..0c1008e7669 100644 --- a/lib/sass/discourse_stylesheets.rb +++ b/lib/sass/discourse_stylesheets.rb @@ -1,5 +1,4 @@ require_dependency 'sass/discourse_sass_compiler' -require_dependency 'distributed_cache' class DiscourseStylesheets @@ -8,22 +7,35 @@ class DiscourseStylesheets MANIFEST_FULL_PATH = "#{MANIFEST_DIR}/stylesheet-manifest" @lock = Mutex.new + @links = {} - def self.cache - @cache ||= DistributedCache.new("discourse_stylesheet") + def self.ensure_subscribed! + unless @subscribed + @lock.synchronize do + MessageBus.subscribe("/discourse_stylesheet") do |message| + @lock.synchronize do + @links[message.site_id] = nil + end + end + @subscribed = true + end + end end def self.stylesheet_link_tag(target = :desktop) - tag = cache[target] - return tag if tag - + ensure_subscribed! @lock.synchronize do - builder = self.new(target) - builder.compile unless File.exists?(builder.stylesheet_fullpath) - builder.ensure_digestless_file - tag = %[].html_safe + links = (@links[RailsMultisite::ConnectionManagement.current_db] ||= {}) + tag = links[target] - cache[target] = tag + if !tag + builder = self.new(target) + builder.compile unless File.exists?(builder.stylesheet_fullpath) + builder.ensure_digestless_file + tag = %[].html_safe + + links[target] = tag + end tag end diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index ec8b7c16dfa..0fc0ab7eb74 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -289,7 +289,7 @@ module SiteSettingExtension protected def clear_cache! - SiteText.text_for_cache.clear + MessageBus.publish '/text_for', 'site_settings' Rails.cache.delete(SiteSettingExtension.client_settings_cache_key) end diff --git a/lib/site_text_class_methods.rb b/lib/site_text_class_methods.rb index 7ff9b0a1c22..5482ad42b35 100644 --- a/lib/site_text_class_methods.rb +++ b/lib/site_text_class_methods.rb @@ -15,16 +15,39 @@ module SiteTextClassMethods @types << SiteTextType.new(text_type, format, opts) end - def text_for_cache - @text_for_cache ||= DistributedCache.new("text_for_cache") - end - def text_for(text_type, replacements=nil) text = nil - text = text_for_cache[text_type] if replacements.blank? + text = cached_text_for(text_type) if replacements.blank? text ||= uncached_text_for(text_type, replacements) end + def cached_text_for(text_type) + ensure_subscribed! + @mutex.synchronize do + cache = @text_for_cache[RailsMultisite::ConnectionManagement.current_db] + cache[text_type] if cache + end + end + + def store_cached_text_for(text_type, result) + ensure_subscribed! + @mutex.synchronize do + cache = (@text_for_cache[RailsMultisite::ConnectionManagement.current_db] ||= {}) + cache[text_type] = result + end + end + + def ensure_subscribed! + return if @subscribed + @mutex.synchronize do + MessageBus.subscribe("/text_for") do |message| + @mutex.synchronize do + @text_for_cache[message.site_id] = nil + end + end + end + end + def uncached_text_for(text_type, replacements=nil) store_cache = replacements.blank? @@ -48,7 +71,7 @@ module SiteTextClassMethods if store_cache result.freeze - text_for_cache[text_type] = result + store_cached_text_for(text_type, result) end result