From 0700e9382d551e4948b493e1219f3119e52bcae4 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 1 Jun 2021 14:57:24 +0800 Subject: [PATCH] PERF: Memoize core svgs in memory to avoid expensive XML parsing. The XML parsing of SVGs is done whenever the cache expires or on the first load after a reboot. In one of our production instance, parsing ranges from 30ms to 70ms which is not ideal. Instead, we've decided to make a small memory trade off here by memoizing the core SVGs once on boot to avoid parsing of the SVG files during the duration of a request. The memozied hash will take up 57440 bytes or 0.05744 megabytes in size. --- lib/discourse.rb | 7 ++- lib/svg_sprite/svg_sprite.rb | 83 +++++++++++++++++++++++++++--------- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/lib/discourse.rb b/lib/discourse.rb index 23e5e646d7a..36165eaba80 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -914,9 +914,9 @@ module Discourse schema_cache = ActiveRecord::Base.connection.schema_cache - # load up schema cache for all multisite assuming all dbs have - # an identical schema RailsMultisite::ConnectionManagement.safe_each_connection do + # load up schema cache for all multisite assuming all dbs have + # an identical schema dup_cache = schema_cache.dup # this line is not really needed, but just in case the # underlying implementation changes lets give it a shot @@ -948,6 +948,9 @@ module Discourse }, Thread.new { LetterAvatar.image_magick_version + }, + Thread.new { + SvgSprite.core_svgs } ].each(&:join) ensure diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb index bf5aae7ad55..e2215ddaa81 100644 --- a/lib/svg_sprite/svg_sprite.rb +++ b/lib/svg_sprite/svg_sprite.rb @@ -225,19 +225,21 @@ module SvgSprite get_set_cache("custom_svg_sprites_#{Theme.transform_ids(theme_ids).join(',')}") do custom_sprite_paths = Dir.glob("#{Rails.root}/plugins/*/svg-icons/*.svg") - ThemeField.where(type_id: ThemeField.types[:theme_upload_var], name: THEME_SPRITE_VAR_NAME, theme_id: Theme.transform_ids(theme_ids)) - .pluck(:upload_id).each do |upload_id| + if theme_ids.present? + ThemeField.where(type_id: ThemeField.types[:theme_upload_var], name: THEME_SPRITE_VAR_NAME, theme_id: Theme.transform_ids(theme_ids)) + .pluck(:upload_id).each do |upload_id| - upload = Upload.find(upload_id) rescue nil + upload = Upload.find(upload_id) rescue nil - if Discourse.store.external? - external_copy = Discourse.store.download(upload) rescue nil - original_path = external_copy.try(:path) - else - original_path = Discourse.store.path_for(upload) + if Discourse.store.external? + external_copy = Discourse.store.download(upload) rescue nil + original_path = external_copy.try(:path) + else + original_path = Discourse.store.path_for(upload) + end + + custom_sprite_paths << original_path if original_path.present? end - - custom_sprite_paths << original_path if original_path.present? end custom_sprite_paths @@ -275,7 +277,33 @@ module SvgSprite end def self.sprite_sources(theme_ids) - CORE_SVG_SPRITES | custom_svg_sprites(theme_ids) + sources = CORE_SVG_SPRITES + + if theme_ids.present? + sources = sources + custom_svg_sprites(theme_ids) + end + + sources + end + + def self.core_svgs + @core_svgs ||= begin + symbols = {} + + CORE_SVG_SPRITES.each do |filename| + svg_filename = "#{File.basename(filename, ".svg")}" + + Nokogiri::XML(File.open(filename)) do |config| + config.options = Nokogiri::XML::ParseOptions::NOBLANKS + end.css('symbol').each do |sym| + icon_id = prepare_symbol(sym, svg_filename) + sym.attributes['id'].value = icon_id + symbols[icon_id] = sym.to_xml + end + end + + symbols + end end def self.bundle(theme_ids = []) @@ -288,19 +316,28 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL """.dup - sprite_sources(theme_ids).each do |fname| - svg_file = Nokogiri::XML(File.open(fname)) do |config| - config.options = Nokogiri::XML::ParseOptions::NOBLANKS + core_svgs.each do |icon_id, sym| + if icons.include?(icon_id) + svg_subset << sym end + end - svg_filename = "#{File.basename(fname, ".svg")}" + if theme_ids.present? + custom_svg_sprites(theme_ids).each do |fname| + svg_file = Nokogiri::XML(File.open(fname)) do |config| + config.options = Nokogiri::XML::ParseOptions::NOBLANKS + end - svg_file.css('symbol').each do |sym| - icon_id = prepare_symbol(sym, svg_filename) - if icons.include? icon_id - sym.attributes['id'].value = icon_id - sym.css('title').each(&:remove) - svg_subset << sym.to_xml + svg_filename = "#{File.basename(fname, ".svg")}" + + svg_file.css("symbol").each do |sym| + icon_id = prepare_symbol(sym, svg_filename) + + if icons.include? icon_id + sym.attributes['id'].value = icon_id + sym.css('title').each(&:remove) + svg_subset << sym.to_xml + end end end end @@ -405,6 +442,8 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL end def self.theme_icons(theme_ids) + return [] if theme_ids.blank? + theme_icon_settings = [] # Need to load full records for default values @@ -422,6 +461,8 @@ License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL end def self.custom_icons(theme_ids) + return [] if theme_ids.blank? + # Automatically register icons in sprites added via themes or plugins icons = [] custom_svg_sprites(theme_ids).each do |fname|