From 33a0ad1b69b1b8556f28c5c87c746fafb382d996 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 16 Feb 2022 12:46:17 +1100 Subject: [PATCH] PERF: introduce site/global emoji cache (#15899) Previously calls such as `Emoji["smile"]` would force a full dehydration of objects from Redis. This introduces a version safe site and global emoji cache so lookups are cheap. It eliminates iterating through the list of emojis and pulling from redis. Distributed cache uses a normalized name as the key and stores an Array tuple with version and Emoji. Successful hits always confirm version matches. Interface to Emoji object remains unchanged. We opted for 2 caches to improve reuse on multisites. misses though will be stored in both caches. If there is a hit on the global cache we can avoid looking up in site local cache and storing a miss there. --- app/models/emoji.rb | 40 +++++++++++++++++++++++++++++++++++---- spec/models/emoji_spec.rb | 31 ++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/app/models/emoji.rb b/app/models/emoji.rb index 36bdea93be3..c940f317964 100644 --- a/app/models/emoji.rb +++ b/app/models/emoji.rb @@ -12,6 +12,14 @@ class Emoji attr_accessor :name, :url, :tonable, :group + def self.global_emoji_cache + @global_emoji_cache ||= DistributedCache.new("global_emoji_cache", namespace: false) + end + + def self.site_emoji_cache + @site_emoji_cache ||= DistributedCache.new("site_emoji_cache") + end + def self.all Discourse.cache.fetch(cache_key("all_emojis")) { standard | custom } end @@ -54,10 +62,28 @@ class Emoji is_toned = name.match?(/.+:t[1-6]/) normalized_name = name.gsub(/(.+):t[1-6]/, '\1') - Emoji.all.detect do |e| - e.name == normalized_name && - (!is_toned || (is_toned && e.tonable)) + found_emoji = nil + + [[global_emoji_cache, :standard], [site_emoji_cache, :custom]].each do |cache, list_key| + cache_postfix, found_emoji = cache.defer_get_set(normalized_name) do + emoji = Emoji.public_send(list_key).detect do |e| + e.name == normalized_name && + (!is_toned || (is_toned && e.tonable)) + end + [self.cache_postfix, emoji] + end + + if found_emoji && (cache_postfix != self.cache_postfix) + cache.delete(normalized_name) + redo + end + + if found_emoji + break + end end + + found_emoji end def self.create_from_db_item(emoji) @@ -81,13 +107,19 @@ class Emoji end def self.cache_key(name) - "#{name}:v#{EMOJI_VERSION}:#{Plugin::CustomEmoji.cache_key}" + "#{name}#{cache_postfix}" + end + + def self.cache_postfix + ":v#{EMOJI_VERSION}:#{Plugin::CustomEmoji.cache_key}" end def self.clear_cache %w{custom standard aliases search_aliases translations all tonable}.each do |key| Discourse.cache.delete(cache_key("#{key}_emojis")) end + global_emoji_cache.clear + site_emoji_cache.clear end def self.db_file diff --git a/spec/models/emoji_spec.rb b/spec/models/emoji_spec.rb index 25c1c5ffbee..505c322f9ca 100644 --- a/spec/models/emoji_spec.rb +++ b/spec/models/emoji_spec.rb @@ -87,6 +87,37 @@ describe Emoji do end end + describe 'version updates' do + it 'should correct cache when global emojis cache is stale' do + Emoji.global_emoji_cache["blonde_man"] = [ + "invalid", + Emoji.new + ] + + emoji = Emoji[":blonde_man:t3"] + + expect(emoji.name).to eq('blonde_man') + expect(emoji.tonable).to eq(true) + end + + it 'should correct cache when site emojis cache is stale' do + CustomEmoji.create!(name: 'test123', upload_id: 9999) + Emoji.clear_cache + + Emoji.site_emoji_cache["test123"] = [ + "invalid", + Emoji.new + ] + + emoji = Emoji[":test123:"] + + expect(emoji.name).to eq('test123') + expect(emoji.tonable).to be_falsey + + Emoji.clear_cache + end + end + describe '.codes_to_img' do before { Plugin::CustomEmoji.clear_cache } after { Plugin::CustomEmoji.clear_cache }