PERF: Cache each theme field value once (#23192)
Previously, theme fields from components would be cached for each of their parent themes.
This commit is contained in:
parent
c9c2a73441
commit
82a56334a3
|
@ -350,12 +350,7 @@ class Theme < ActiveRecord::Base
|
||||||
return "" if theme_id.blank?
|
return "" if theme_id.blank?
|
||||||
|
|
||||||
theme_ids = !skip_transformation ? transform_ids(theme_id) : [theme_id]
|
theme_ids = !skip_transformation ? transform_ids(theme_id) : [theme_id]
|
||||||
cache_key = "#{theme_ids.join(",")}:#{target}:#{field}:#{Theme.compiler_version}"
|
(resolve_baked_field(theme_ids, target.to_sym, field) || "").html_safe
|
||||||
|
|
||||||
get_set_cache(cache_key) do
|
|
||||||
target = target.to_sym
|
|
||||||
resolve_baked_field(theme_ids, target, field) || ""
|
|
||||||
end.html_safe
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.lookup_modifier(theme_ids, modifier_name)
|
def self.lookup_modifier(theme_ids, modifier_name)
|
||||||
|
@ -434,25 +429,71 @@ class Theme < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.resolve_baked_field(theme_ids, target, name)
|
def self.resolve_baked_field(theme_ids, target, name)
|
||||||
if target == :extra_js
|
target = target.to_sym
|
||||||
|
name = name&.to_sym
|
||||||
|
|
||||||
|
target = :mobile if target == :mobile_theme
|
||||||
|
target = :desktop if target == :desktop_theme
|
||||||
|
|
||||||
|
case target
|
||||||
|
when :extra_js
|
||||||
|
get_set_cache("#{theme_ids.join(",")}:extra_js:#{Theme.compiler_version}") do
|
||||||
require_rebake =
|
require_rebake =
|
||||||
ThemeField.where(theme_id: theme_ids, target_id: Theme.targets[:extra_js]).where(
|
ThemeField.where(theme_id: theme_ids, target_id: Theme.targets[:extra_js]).where(
|
||||||
"compiler_version <> ?",
|
"compiler_version <> ?",
|
||||||
Theme.compiler_version,
|
Theme.compiler_version,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
ActiveRecord::Base.transaction do
|
||||||
require_rebake.each { |tf| tf.ensure_baked! }
|
require_rebake.each { |tf| tf.ensure_baked! }
|
||||||
require_rebake
|
|
||||||
.map(&:theme_id)
|
Theme.where(id: require_rebake.map(&:theme_id)).each(&:update_javascript_cache!)
|
||||||
.uniq
|
end
|
||||||
.each { |theme_id| Theme.find(theme_id).update_javascript_cache! }
|
|
||||||
caches = JavascriptCache.where(theme_id: theme_ids)
|
caches =
|
||||||
caches = caches.sort_by { |cache| theme_ids.index(cache.theme_id) }
|
JavascriptCache
|
||||||
return caches.map { |c| <<~HTML.html_safe }.join("\n")
|
.where(theme_id: theme_ids)
|
||||||
|
.index_by(&:theme_id)
|
||||||
|
.values_at(*theme_ids)
|
||||||
|
.compact
|
||||||
|
|
||||||
|
caches.map { |c| <<~HTML.html_safe }.join("\n")
|
||||||
<link rel="preload" href="#{c.url}" as="script">
|
<link rel="preload" href="#{c.url}" as="script">
|
||||||
<script defer src='#{c.url}' data-theme-id='#{c.theme_id}'></script>
|
<script defer src='#{c.url}' data-theme-id='#{c.theme_id}'></script>
|
||||||
HTML
|
HTML
|
||||||
end
|
end
|
||||||
list_baked_fields(theme_ids, target, name).map { |f| f.value_baked || f.value }.join("\n")
|
when :translations
|
||||||
|
theme_field_values(theme_ids, :translations, I18n.fallbacks[name])
|
||||||
|
.to_a
|
||||||
|
.select(&:second)
|
||||||
|
.uniq { |((theme_id, _, _), _)| theme_id }
|
||||||
|
.flat_map(&:second)
|
||||||
|
.join("\n")
|
||||||
|
else
|
||||||
|
theme_field_values(theme_ids, [:common, target], name).values.compact.flatten.join("\n")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.theme_field_values(theme_ids, targets, names)
|
||||||
|
cache.defer_get_set_bulk(
|
||||||
|
Array(theme_ids).product(Array(targets), Array(names)),
|
||||||
|
lambda do |(theme_id, target, name)|
|
||||||
|
"#{theme_id}:#{target}:#{name}:#{Theme.compiler_version}"
|
||||||
|
end,
|
||||||
|
) do |keys|
|
||||||
|
keys = keys.map { |theme_id, target, name| [theme_id, Theme.targets[target], name.to_s] }
|
||||||
|
|
||||||
|
keys
|
||||||
|
.map do |theme_id, target_id, name|
|
||||||
|
ThemeField.where(theme_id: theme_id, target_id: target_id, name: name)
|
||||||
|
end
|
||||||
|
.inject { |a, b| a.or(b) }
|
||||||
|
.each(&:ensure_baked!)
|
||||||
|
.map { |tf| [[tf.theme_id, tf.target_id, tf.name], tf.value_baked || tf.value] }
|
||||||
|
.group_by(&:first)
|
||||||
|
.transform_values { |x| x.map(&:second) }
|
||||||
|
.values_at(*keys)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def self.list_baked_fields(theme_ids, target, name)
|
def self.list_baked_fields(theme_ids, target, name)
|
||||||
|
|
|
@ -20,6 +20,24 @@ class DistributedCache < MessageBus::DistributedCache
|
||||||
value
|
value
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def defer_get_set_bulk(ks, key_blk, &blk)
|
||||||
|
found_keys, missing_keys = ks.partition { |k| hash.key?(key_blk.call(k)) }
|
||||||
|
found_hash = found_keys.map { |key| [key, self[key_blk.call(key)]] }.to_h
|
||||||
|
|
||||||
|
if missing_keys.present?
|
||||||
|
missing_values = blk.call(missing_keys.freeze)
|
||||||
|
missing_hash = missing_keys.zip(missing_values).to_h
|
||||||
|
|
||||||
|
Scheduler::Defer.later("#{@key}_bulk_set") do
|
||||||
|
missing_hash.each { |key, value| self[key_blk.call(key)] = value }
|
||||||
|
end
|
||||||
|
|
||||||
|
ks.zip(missing_hash.merge(found_hash).values_at(*ks)).to_h
|
||||||
|
else
|
||||||
|
found_hash
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def clear(after_commit: true)
|
def clear(after_commit: true)
|
||||||
if after_commit && !GlobalSetting.skip_db?
|
if after_commit && !GlobalSetting.skip_db?
|
||||||
DB.after_commit { super() }
|
DB.after_commit { super() }
|
||||||
|
|
|
@ -1119,4 +1119,104 @@ HTML
|
||||||
expect(messages).to include(["development-mode-theme-changed"])
|
expect(messages).to include(["development-mode-theme-changed"])
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "#lookup_field when a theme component is used in multiple themes" do
|
||||||
|
fab!(:theme_1) { Fabricate(:theme, user: user) }
|
||||||
|
fab!(:theme_2) { Fabricate(:theme, user: user) }
|
||||||
|
fab!(:child) { Fabricate(:theme, user: user, component: true) }
|
||||||
|
|
||||||
|
before_all do
|
||||||
|
theme_1.add_relative_theme!(:child, child)
|
||||||
|
theme_2.add_relative_theme!(:child, child)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "efficiently caches fields of theme component by only caching the fields once across multiple themes" do
|
||||||
|
child.set_field(target: :common, name: "header", value: "World")
|
||||||
|
child.save!
|
||||||
|
|
||||||
|
expect(Theme.lookup_field(theme_1.id, :desktop, "header")).to eq("World")
|
||||||
|
expect(Theme.lookup_field(theme_2.id, :desktop, "header")).to eq("World")
|
||||||
|
|
||||||
|
expect(
|
||||||
|
Theme.cache.defer_get_set("#{child.id}:common:header:#{Theme.compiler_version}") { raise },
|
||||||
|
).to eq(["World"])
|
||||||
|
expect(
|
||||||
|
Theme.cache.defer_get_set("#{child.id}:desktop:header:#{Theme.compiler_version}") { raise },
|
||||||
|
).to eq(nil)
|
||||||
|
|
||||||
|
expect(
|
||||||
|
Theme
|
||||||
|
.cache
|
||||||
|
.defer_get_set("#{theme_1.id}:common:header:#{Theme.compiler_version}") { raise },
|
||||||
|
).to eq(nil)
|
||||||
|
expect(
|
||||||
|
Theme
|
||||||
|
.cache
|
||||||
|
.defer_get_set("#{theme_1.id}:desktop:header:#{Theme.compiler_version}") { raise },
|
||||||
|
).to eq(nil)
|
||||||
|
|
||||||
|
expect(
|
||||||
|
Theme
|
||||||
|
.cache
|
||||||
|
.defer_get_set("#{theme_2.id}:common:header:#{Theme.compiler_version}") { raise },
|
||||||
|
).to eq(nil)
|
||||||
|
expect(
|
||||||
|
Theme
|
||||||
|
.cache
|
||||||
|
.defer_get_set("#{theme_2.id}:desktop:header:#{Theme.compiler_version}") { raise },
|
||||||
|
).to eq(nil)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "puts the parent value ahead of the child" do
|
||||||
|
theme_1.set_field(target: :common, name: "header", value: "theme_1")
|
||||||
|
theme_1.save!
|
||||||
|
|
||||||
|
child.set_field(target: :common, name: "header", value: "child")
|
||||||
|
child.save!
|
||||||
|
|
||||||
|
expect(Theme.lookup_field(theme_1.id, :desktop, "header")).to eq("theme_1\nchild")
|
||||||
|
end
|
||||||
|
|
||||||
|
it "puts parent translations ahead of child translations" do
|
||||||
|
theme_1.set_field(target: :translations, name: "en", value: <<~YAML)
|
||||||
|
en:
|
||||||
|
theme_1: "test"
|
||||||
|
YAML
|
||||||
|
theme_1.save!
|
||||||
|
theme_field = ThemeField.order(:id).last
|
||||||
|
|
||||||
|
child.set_field(target: :translations, name: "en", value: <<~YAML)
|
||||||
|
en:
|
||||||
|
child: "test"
|
||||||
|
YAML
|
||||||
|
child.save!
|
||||||
|
child_field = ThemeField.order(:id).last
|
||||||
|
|
||||||
|
expect(theme_field.value_baked).not_to eq(child_field.value_baked)
|
||||||
|
expect(Theme.lookup_field(theme_1.id, :translations, :en)).to eq(
|
||||||
|
[theme_field, child_field].map(&:value_baked).join("\n"),
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "prioritizes a locale over its fallback" do
|
||||||
|
theme_1.set_field(target: :translations, name: "en", value: <<~YAML)
|
||||||
|
en:
|
||||||
|
theme_1: "hello"
|
||||||
|
YAML
|
||||||
|
theme_1.save!
|
||||||
|
en_field = ThemeField.order(:id).last
|
||||||
|
|
||||||
|
theme_1.set_field(target: :translations, name: "es", value: <<~YAML)
|
||||||
|
es:
|
||||||
|
theme_1: "hola"
|
||||||
|
YAML
|
||||||
|
theme_1.save!
|
||||||
|
es_field = ThemeField.order(:id).last
|
||||||
|
|
||||||
|
expect(es_field.value_baked).not_to eq(en_field.value_baked)
|
||||||
|
expect(Theme.lookup_field(theme_1.id, :translations, :en)).to eq(en_field.value_baked)
|
||||||
|
expect(Theme.lookup_field(theme_1.id, :translations, :es)).to eq(es_field.value_baked)
|
||||||
|
expect(Theme.lookup_field(theme_1.id, :translations, :fr)).to eq(en_field.value_baked)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue