From c588337aac62ed9bbfbabd45c2c3989f7711d49a Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Tue, 1 Mar 2022 12:44:57 +0100
Subject: [PATCH] Eagerly cache global filters

Including a module can cause Ruby's global constant cache to be busted
if the included module contain constants. So that's something you don't
want to happen at "runtime", otherwise it will severely degrade performance
and if you are using YJIT or MJIT most of the compiled code will be invalidated.

To limit the impact of this, we can pre-include the global filters,
as they're generally registered during boot, that limits the problem
to non-global filters.
---
 lib/liquid.rb                            |  2 +-
 lib/liquid/strainer_factory.rb           | 21 +++++++++++----------
 lib/liquid/strainer_template.rb          |  5 +++++
 test/test_helper.rb                      | 20 ++++++++++----------
 test/unit/strainer_factory_unit_test.rb  |  3 ++-
 test/unit/strainer_template_unit_test.rb |  2 +-
 6 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/lib/liquid.rb b/lib/liquid.rb
index 7772bb5f..f6aabb0d 100644
--- a/lib/liquid.rb
+++ b/lib/liquid.rb
@@ -59,8 +59,8 @@ require 'liquid/forloop_drop'
 require 'liquid/extensions'
 require 'liquid/errors'
 require 'liquid/interrupts'
-require 'liquid/strainer_factory'
 require 'liquid/strainer_template'
+require 'liquid/strainer_factory'
 require 'liquid/expression'
 require 'liquid/context'
 require 'liquid/parser_switching'
diff --git a/lib/liquid/strainer_factory.rb b/lib/liquid/strainer_factory.rb
index 7a7bcb8a..739227ff 100644
--- a/lib/liquid/strainer_factory.rb
+++ b/lib/liquid/strainer_factory.rb
@@ -7,25 +7,26 @@ module Liquid
 
     def add_global_filter(filter)
       strainer_class_cache.clear
-      global_filters << filter
+      GlobalCache.add_filter(filter)
     end
 
     def create(context, filters = [])
       strainer_from_cache(filters).new(context)
     end
 
+    GlobalCache = Class.new(StrainerTemplate)
+
     private
 
-    def global_filters
-      @global_filters ||= []
-    end
-
     def strainer_from_cache(filters)
-      strainer_class_cache[filters] ||= begin
-        klass = Class.new(StrainerTemplate)
-        global_filters.each { |f| klass.add_filter(f) }
-        filters.each { |f| klass.add_filter(f) }
-        klass
+      if filters.empty?
+        GlobalCache
+      else
+        strainer_class_cache[filters] ||= begin
+          klass = Class.new(GlobalCache)
+          filters.each { |f| klass.add_filter(f) }
+          klass
+        end
       end
     end
 
diff --git a/lib/liquid/strainer_template.rb b/lib/liquid/strainer_template.rb
index a3ad06a7..c16be3b0 100644
--- a/lib/liquid/strainer_template.rb
+++ b/lib/liquid/strainer_template.rb
@@ -31,6 +31,11 @@ module Liquid
         filter_methods.include?(method.to_s)
       end
 
+      def inherited(subclass)
+        super
+        subclass.instance_variable_set(:@filter_methods, @filter_methods.dup)
+      end
+
       private
 
       def filter_methods
diff --git a/test/test_helper.rb b/test/test_helper.rb
index f5c1c6a6..874dd8f4 100755
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -72,21 +72,21 @@ module Minitest
     end
 
     def with_global_filter(*globals)
-      original_global_filters = Liquid::StrainerFactory.instance_variable_get(:@global_filters)
-      Liquid::StrainerFactory.instance_variable_set(:@global_filters, [])
-      globals.each do |global|
-        Liquid::StrainerFactory.add_global_filter(global)
-      end
-
-      Liquid::StrainerFactory.send(:strainer_class_cache).clear
+      original_global_cache = Liquid::StrainerFactory::GlobalCache
+      Liquid::StrainerFactory.send(:remove_const, :GlobalCache)
+      Liquid::StrainerFactory.const_set(:GlobalCache, Class.new(Liquid::StrainerTemplate))
 
       globals.each do |global|
         Liquid::Template.register_filter(global)
       end
-      yield
-    ensure
       Liquid::StrainerFactory.send(:strainer_class_cache).clear
-      Liquid::StrainerFactory.instance_variable_set(:@global_filters, original_global_filters)
+      begin
+        yield
+      ensure
+        Liquid::StrainerFactory.send(:remove_const, :GlobalCache)
+        Liquid::StrainerFactory.const_set(:GlobalCache, original_global_cache)
+        Liquid::StrainerFactory.send(:strainer_class_cache).clear
+      end
     end
 
     def with_error_mode(mode)
diff --git a/test/unit/strainer_factory_unit_test.rb b/test/unit/strainer_factory_unit_test.rb
index 05a9c834..43eafc1d 100644
--- a/test/unit/strainer_factory_unit_test.rb
+++ b/test/unit/strainer_factory_unit_test.rb
@@ -52,7 +52,8 @@ class StrainerFactoryUnitTest < Minitest::Test
       /\ALiquid error: wrong number of arguments \((1 for 0|given 1, expected 0)\)\z/,
       exception.message
     )
-    assert_equal(exception.backtrace[0].split(':')[0], __FILE__)
+    source = AccessScopeFilters.instance_method(:public_filter).source_location
+    assert_equal(source.map(&:to_s), exception.backtrace[0].split(':')[0..1])
   end
 
   def test_strainer_only_invokes_public_filter_methods
diff --git a/test/unit/strainer_template_unit_test.rb b/test/unit/strainer_template_unit_test.rb
index e72ed44d..fd6a5a3c 100644
--- a/test/unit/strainer_template_unit_test.rb
+++ b/test/unit/strainer_template_unit_test.rb
@@ -57,8 +57,8 @@ class StrainerTemplateUnitTest < Minitest::Test
   end
 
   def test_add_filter_does_not_raise_when_module_overrides_previously_registered_method
-    strainer = Context.new.strainer
     with_global_filter do
+      strainer = Context.new.strainer
       strainer.class.add_filter(PublicMethodOverrideFilter)
       assert(strainer.class.send(:filter_methods).include?('public_filter'))
     end