PERF: Improve theme stylesheet compilation performance (#12850)

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.

diff --git a/app/models/theme.rb b/app/models/theme.rb
index 3c5ce54..1f20833 100644
--- a/app/models/theme.rb
+++ b/app/models/theme.rb
@@ -601,11 +601,12 @@ class Theme < ActiveRecord::Base
     find_disable_action_log&.created_at
   end
 
-  def scss_load_paths
-    return if self.extra_scss_fields.empty?
+  def with_scss_load_paths
+    return yield([]) if self.extra_scss_fields.empty?
 
-    @exporter ||= ThemeStore::ZipExporter.new(self)
-    ["#{@exporter.export_dir}/scss", "#{@exporter.export_dir}/stylesheets"]
+    ThemeStore::ZipExporter.new(self).with_export_dir(extra_scss_only: true) do |dir|
+      yield ["#{dir}/stylesheets"]
+    end
   end
 
   def scss_variables
diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb
index ab3d96d..9ed1e64 100644
--- a/app/models/theme_field.rb
+++ b/app/models/theme_field.rb
@@ -375,11 +375,13 @@ class ThemeField < ActiveRecord::Base
   def compile_scss(prepended_scss = nil)
     prepended_scss ||= Stylesheet::Importer.new({}).prepended_scss
 
-    Stylesheet::Compiler.compile("#{prepended_scss} #{self.theme.scss_variables.to_s} #{self.value}",
-      "#{Theme.targets[self.target_id]}.scss",
-      theme: self.theme,
-      load_paths: self.theme.scss_load_paths
-    )
+    self.theme.with_scss_load_paths do |load_paths|
+      Stylesheet::Compiler.compile("#{prepended_scss} #{self.theme.scss_variables.to_s} #{self.value}",
+        "#{Theme.targets[self.target_id]}.scss",
+        theme: self.theme,
+        load_paths: load_paths
+      )
+    end
   end
 
   def compiled_css(prepended_scss)
diff --git a/lib/stylesheet/manager.rb b/lib/stylesheet/manager.rb
index 6a86417..2d92ed8 100644
--- a/lib/stylesheet/manager.rb
+++ b/lib/stylesheet/manager.rb
@@ -246,7 +246,7 @@ class Stylesheet::Manager
     end
 
     rtl = @target.to_s =~ /_rtl$/
-    css, source_map = begin
+    css, source_map = with_load_paths do |load_paths|
       Stylesheet::Compiler.compile_asset(
         @target,
          rtl: rtl,
@@ -254,7 +254,7 @@ class Stylesheet::Manager
          theme_variables: theme&.scss_variables.to_s,
          source_map_file: source_map_filename,
          color_scheme_id: @color_scheme&.id,
-         load_paths: theme&.scss_load_paths
+         load_paths: load_paths
       )
     rescue SassC::SyntaxError => e
       if Stylesheet::Importer::THEME_TARGETS.include?(@target.to_s)
@@ -373,6 +373,14 @@ class Stylesheet::Manager
     @theme == :nil ? nil : @theme
   end
 
+  def with_load_paths
+    if theme
+      theme.with_scss_load_paths { |p| yield p }
+    else
+      yield nil
+    end
+  end
+
   def theme_digest
     if [:mobile_theme, :desktop_theme].include?(@target)
       scss_digest = theme.resolve_baked_field(:common, :scss)
diff --git a/lib/theme_store/zip_exporter.rb b/lib/theme_store/zip_exporter.rb
index 3cc4a94..edc8639 100644
--- a/lib/theme_store/zip_exporter.rb
+++ b/lib/theme_store/zip_exporter.rb
@@ -25,11 +25,22 @@ class ThemeStore::ZipExporter
     FileUtils.rm_rf(@temp_folder)
   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)
     FileUtils.mkdir_p(destination_folder)
 
     @theme.theme_fields.each do |field|
+      next if extra_scss_only && !field.extra_scss_field?
       next unless path = field.file_path
 
       # Belt and braces approach here. All the user input should already be
@@ -50,19 +61,16 @@ class ThemeStore::ZipExporter
       File.write(path, content)
     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
   end
 
-  def export_dir
-    export_to_folder
-
-    File.join(@temp_folder, @export_name)
-  end
-
-  private
-
   def export_package
     export_to_folder
 

GitHub sha: 1fd8f6df

This commit appears in #12850 which was approved by pmusaraj. It was merged by davidtaylorhq.