PERF: Improve theme stylesheet compilation performance (#12850)

When building the `scss_load_paths`, we were creating a full export of the theme (including uploads), and not cleaning it up. With many uploads, this can be extremely slow (because it downloads every upload from S3), and the lack of cleanup could cause a disk to fill up over time.

This commit updates the ZipExporter to provide a `with_export_dir` API, which takes care of cleanup. It also adds a kwarg which allows exporting only extra_scss fields. This should make things much faster for themes with many uploads.
This commit is contained in:
David Taylor 2021-04-27 14:33:43 +01:00 committed by GitHub
parent 657dff3544
commit 1fd8f6df5f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 40 additions and 21 deletions

View File

@ -601,11 +601,12 @@ class Theme < ActiveRecord::Base
find_disable_action_log&.created_at find_disable_action_log&.created_at
end end
def scss_load_paths def with_scss_load_paths
return if self.extra_scss_fields.empty? return yield([]) if self.extra_scss_fields.empty?
@exporter ||= ThemeStore::ZipExporter.new(self) ThemeStore::ZipExporter.new(self).with_export_dir(extra_scss_only: true) do |dir|
["#{@exporter.export_dir}/scss", "#{@exporter.export_dir}/stylesheets"] yield ["#{dir}/stylesheets"]
end
end end
def scss_variables def scss_variables

View File

@ -375,12 +375,14 @@ class ThemeField < ActiveRecord::Base
def compile_scss(prepended_scss = nil) def compile_scss(prepended_scss = nil)
prepended_scss ||= Stylesheet::Importer.new({}).prepended_scss prepended_scss ||= Stylesheet::Importer.new({}).prepended_scss
self.theme.with_scss_load_paths do |load_paths|
Stylesheet::Compiler.compile("#{prepended_scss} #{self.theme.scss_variables.to_s} #{self.value}", Stylesheet::Compiler.compile("#{prepended_scss} #{self.theme.scss_variables.to_s} #{self.value}",
"#{Theme.targets[self.target_id]}.scss", "#{Theme.targets[self.target_id]}.scss",
theme: self.theme, theme: self.theme,
load_paths: self.theme.scss_load_paths load_paths: load_paths
) )
end end
end
def compiled_css(prepended_scss) def compiled_css(prepended_scss)
css, _source_map = begin css, _source_map = begin

View File

@ -246,7 +246,7 @@ class Stylesheet::Manager
end end
rtl = @target.to_s =~ /_rtl$/ rtl = @target.to_s =~ /_rtl$/
css, source_map = begin css, source_map = with_load_paths do |load_paths|
Stylesheet::Compiler.compile_asset( Stylesheet::Compiler.compile_asset(
@target, @target,
rtl: rtl, rtl: rtl,
@ -254,7 +254,7 @@ class Stylesheet::Manager
theme_variables: theme&.scss_variables.to_s, theme_variables: theme&.scss_variables.to_s,
source_map_file: source_map_filename, source_map_file: source_map_filename,
color_scheme_id: @color_scheme&.id, color_scheme_id: @color_scheme&.id,
load_paths: theme&.scss_load_paths load_paths: load_paths
) )
rescue SassC::SyntaxError => e rescue SassC::SyntaxError => e
if Stylesheet::Importer::THEME_TARGETS.include?(@target.to_s) if Stylesheet::Importer::THEME_TARGETS.include?(@target.to_s)
@ -373,6 +373,14 @@ class Stylesheet::Manager
@theme == :nil ? nil : @theme @theme == :nil ? nil : @theme
end end
def with_load_paths
if theme
theme.with_scss_load_paths { |p| yield p }
else
yield nil
end
end
def theme_digest def theme_digest
if [:mobile_theme, :desktop_theme].include?(@target) if [:mobile_theme, :desktop_theme].include?(@target)
scss_digest = theme.resolve_baked_field(:common, :scss) scss_digest = theme.resolve_baked_field(:common, :scss)

View File

@ -25,11 +25,22 @@ class ThemeStore::ZipExporter
FileUtils.rm_rf(@temp_folder) FileUtils.rm_rf(@temp_folder)
end end
def export_to_folder def with_export_dir(**kwargs)
export_to_folder(**kwargs)
yield File.join(@temp_folder, @export_name)
ensure
cleanup!
end
private
def export_to_folder(extra_scss_only: false)
destination_folder = File.join(@temp_folder, @export_name) destination_folder = File.join(@temp_folder, @export_name)
FileUtils.mkdir_p(destination_folder) FileUtils.mkdir_p(destination_folder)
@theme.theme_fields.each do |field| @theme.theme_fields.each do |field|
next if extra_scss_only && !field.extra_scss_field?
next unless path = field.file_path next unless path = field.file_path
# Belt and braces approach here. All the user input should already be # Belt and braces approach here. All the user input should already be
@ -50,19 +61,16 @@ class ThemeStore::ZipExporter
File.write(path, content) File.write(path, content)
end end
File.write(File.join(destination_folder, "about.json"), JSON.pretty_generate(@theme.generate_metadata_hash)) if !extra_scss_only
File.write(
File.join(destination_folder, "about.json"),
JSON.pretty_generate(@theme.generate_metadata_hash)
)
end
@temp_folder @temp_folder
end end
def export_dir
export_to_folder
File.join(@temp_folder, @export_name)
end
private
def export_package def export_package
export_to_folder export_to_folder