DEV: use Discourse.cache over Rails.cache

Discourse.cache is a more consistent method to use and offers clean fallback
if you are skipping redis

This is part of a larger change that both optimizes Discoruse.cache and omits
use of setex on $redis in favor of consistently using discourse cache

Bench does reveal that use of Rails.cache and Discourse.cache is 1.25x slower
than redis.setex / get so a re-implementation will follow prior to porting
This commit is contained in:
Sam Saffron 2019-11-27 12:35:14 +11:00
parent 0807751390
commit 0fb497eb23
11 changed files with 102 additions and 15 deletions

View File

@ -418,7 +418,7 @@ class Theme < ActiveRecord::Base
end end
def cached_settings def cached_settings
Rails.cache.fetch("settings_for_theme_#{self.id}", expires_in: 30.minutes) do Discourse.cache.fetch("settings_for_theme_#{self.id}", expires_in: 30.minutes) do
hash = {} hash = {}
self.settings.each do |setting| self.settings.each do |setting|
hash[setting.name] = setting.value hash[setting.name] = setting.value
@ -438,7 +438,7 @@ class Theme < ActiveRecord::Base
end end
def clear_cached_settings! def clear_cached_settings!
Rails.cache.delete("settings_for_theme_#{self.id}") Discourse.cache.delete("settings_for_theme_#{self.id}")
end end
def included_settings def included_settings

View File

@ -228,7 +228,7 @@ class TopicEmbed < ActiveRecord::Base
end end
def self.expanded_for(post) def self.expanded_for(post)
Rails.cache.fetch("embed-topic:#{post.topic_id}", expires_in: 10.minutes) do Discourse.cache.fetch("embed-topic:#{post.topic_id}", expires_in: 10.minutes) do
url = TopicEmbed.where(topic_id: post.topic_id).pluck_first(:embed_url) url = TopicEmbed.where(topic_id: post.topic_id).pluck_first(:embed_url)
response = TopicEmbed.find_remote(url) response = TopicEmbed.find_remote(url)

View File

@ -591,6 +591,7 @@ module Discourse
SiteSetting.after_fork SiteSetting.after_fork
$redis._client.reconnect $redis._client.reconnect
Rails.cache.reconnect Rails.cache.reconnect
Discourse.cache.reconnect
Logster.store.redis.reconnect Logster.store.redis.reconnect
# shuts down all connections in the pool # shuts down all connections in the pool
Sidekiq.redis_pool.shutdown { |c| nil } Sidekiq.redis_pool.shutdown { |c| nil }

View File

@ -14,11 +14,11 @@ class InlineOneboxer
end end
def self.purge(url) def self.purge(url)
Rails.cache.delete(cache_key(url)) Discourse.cache.delete(cache_key(url))
end end
def self.cache_lookup(url) def self.cache_lookup(url)
Rails.cache.read(cache_key(url)) Discourse.cache.read(cache_key(url))
end end
def self.lookup(url, opts = nil) def self.lookup(url, opts = nil)
@ -70,7 +70,7 @@ class InlineOneboxer
title: title && Emoji.gsub_emoji_to_unicode(title) title: title && Emoji.gsub_emoji_to_unicode(title)
} }
unless opts[:skip_cache] unless opts[:skip_cache]
Rails.cache.write(cache_key(url), onebox, expires_in: 1.day) Discourse.cache.write(cache_key(url), onebox, expires_in: 1.day)
end end
onebox onebox

View File

@ -51,7 +51,7 @@ module Oneboxer
end end
def self.cached_onebox(url) def self.cached_onebox(url)
if c = Rails.cache.read(onebox_cache_key(url)) if c = Discourse.cache.read(onebox_cache_key(url))
c[:onebox] c[:onebox]
end end
rescue => e rescue => e
@ -61,7 +61,7 @@ module Oneboxer
end end
def self.cached_preview(url) def self.cached_preview(url)
if c = Rails.cache.read(onebox_cache_key(url)) if c = Discourse.cache.read(onebox_cache_key(url))
c[:preview] c[:preview]
end end
rescue => e rescue => e
@ -71,7 +71,7 @@ module Oneboxer
end end
def self.invalidate(url) def self.invalidate(url)
Rails.cache.delete(onebox_cache_key(url)) Discourse.cache.delete(onebox_cache_key(url))
end end
# Parse URLs out of HTML, returning the document when finished. # Parse URLs out of HTML, returning the document when finished.
@ -281,7 +281,7 @@ module Oneboxer
end end
def self.external_onebox(url) def self.external_onebox(url)
Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day) do Discourse.cache.fetch(onebox_cache_key(url), expires_in: 1.day) do
fd = FinalDestination.new(url, fd = FinalDestination.new(url,
ignore_redirects: ignore_redirects, ignore_redirects: ignore_redirects,
ignore_hostnames: blacklisted_domains, ignore_hostnames: blacklisted_domains,

View File

@ -137,7 +137,7 @@ class Search
return 0 unless SiteSetting.search_prefer_recent_posts? return 0 unless SiteSetting.search_prefer_recent_posts?
# It can be quite slow to count all the posts so let's cache it # It can be quite slow to count all the posts so let's cache it
Rails.cache.fetch("search-min-post-id:#{SiteSetting.search_recent_posts_size}", expires_in: 1.week) do Discourse.cache.fetch("search-min-post-id:#{SiteSetting.search_recent_posts_size}", expires_in: 1.week) do
min_post_id_no_cache min_post_id_no_cache
end end
end end

View File

@ -192,7 +192,7 @@ module SiteSettingExtension
end end
def client_settings_json def client_settings_json
Rails.cache.fetch(SiteSettingExtension.client_settings_cache_key, expires_in: 30.minutes) do Discourse.cache.fetch(SiteSettingExtension.client_settings_cache_key, expires_in: 30.minutes) do
client_settings_json_uncached client_settings_json_uncached
end end
end end
@ -432,7 +432,7 @@ module SiteSettingExtension
protected protected
def clear_cache! def clear_cache!
Rails.cache.delete(SiteSettingExtension.client_settings_cache_key) Discourse.cache.delete(SiteSettingExtension.client_settings_cache_key)
Site.clear_anon_cache! Site.clear_anon_cache!
end end

81
script/benchmarks/cache/bench.rb vendored Normal file
View File

@ -0,0 +1,81 @@
# frozen_string_literal: true
require 'benchmark/ips'
require File.expand_path('../../../../config/environment', __FILE__)
Benchmark.ips do |x|
x.report("redis setex string") do |times|
while times > 0
$redis.setex("test_key", 60, "test")
times -= 1
end
end
x.report("redis setex marshal string") do |times|
while times > 0
$redis.setex("test_keym", 60, Marshal.dump("test"))
times -= 1
end
end
x.report("Discourse cache string") do |times|
while times > 0
Discourse.cache.write("test_key", "test", expires_in: 60)
times -= 1
end
end
x.report("Rails cache string") do |times|
while times > 0
Rails.cache.write("test_key_rails", "test", expires_in: 60)
times -= 1
end
end
x.compare!
end
Benchmark.ips do |x|
x.report("redis get string") do |times|
while times > 0
$redis.get("test_key")
times -= 1
end
end
x.report("redis get string marshal") do |times|
while times > 0
Marshal.load($redis.get("test_keym"))
times -= 1
end
end
x.report("Discourse read cache string") do |times|
while times > 0
Discourse.cache.read("test_key")
times -= 1
end
end
x.report("Rails read cache string") do |times|
while times > 0
Rails.cache.read("test_key_rails")
times -= 1
end
end
x.compare!
end
# Comparison:
# redis setex string: 13250.0 i/s
# redis setex marshal string: 12866.4 i/s - same-ish: difference falls within error
# Discourse cache string: 10443.0 i/s - 1.27x slower
# Rails cache string: 10367.9 i/s - 1.28x slower
# Comparison:
# redis get string: 13147.4 i/s
# redis get string marshal: 12789.2 i/s - same-ish: difference falls within error
# Rails read cache string: 10486.4 i/s - 1.25x slower
# Discourse read cache string: 10457.1 i/s - 1.26x slower

View File

@ -9,6 +9,11 @@ describe Cache do
Cache.new Cache.new
end end
it "supports float" do
cache.write("float", 1.1)
expect(cache.read("float")).to eq(1.1)
end
it "supports fixnum" do it "supports fixnum" do
cache.write("num", 1) cache.write("num", 1)
expect(cache.read("num")).to eq(1) expect(cache.read("num")).to eq(1)

View File

@ -800,7 +800,7 @@ describe SiteSettingExtension do
it 'expires the cache' do it 'expires the cache' do
settings.default_locale = 'zh_CN' settings.default_locale = 'zh_CN'
expect(Rails.cache.exist?(SiteSettingExtension.client_settings_cache_key)).to be_falsey expect(Discourse.cache.exist?(SiteSettingExtension.client_settings_cache_key)).to be_falsey
end end
it 'refreshes the client' do it 'refreshes the client' do

View File

@ -56,7 +56,7 @@ describe OneboxController do
stub_request(:get, url).to_return(status: 200, body: html).then.to_raise stub_request(:get, url).to_return(status: 200, body: html).then.to_raise
bypass_limiting bypass_limiting
Rails.cache.delete("onebox__#{url}") Discourse.cache.delete("onebox__#{url}")
get "/onebox.json", params: { url: url } get "/onebox.json", params: { url: url }
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(response.body).to include("Onebox1") expect(response.body).to include("Onebox1")