From ac4d5a5229d757e171b086199b2dc17aba99e75e Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Tue, 12 Nov 2024 18:01:43 -0600 Subject: [PATCH] Faster initializer tsort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Iterating initializers for each node child is expensive. Instead, we can use a hash to speed up tsorting children to iterate less often. This can save a lot of time on larger apps: Before: ``` Warming up -------------------------------------- tsort initializers 24.000 i/100ms Calculating ------------------------------------- tsort initializers 244.212 (± 2.5%) i/s (4.09 ms/i) - 1.224k in 5.015190s ``` After: ``` Warming up -------------------------------------- tsort initializers 179.000 i/100ms Calculating ------------------------------------- tsort initializers 1.791k (± 1.1%) i/s (558.27 μs/i) - 9.129k in 5.097031s ``` --- railties/lib/rails/application.rb | 2 +- railties/lib/rails/initializable.rb | 70 +++++++++++++++++++++-------- railties/test/initializable_test.rb | 44 ++++++++++++++++++ 3 files changed, 96 insertions(+), 20 deletions(-) diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 06ec110af86f8..195af88cf583c 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -609,7 +609,7 @@ def ordered_railties # :nodoc: end def railties_initializers(current) # :nodoc: - initializers = [] + initializers = Initializable::Collection.new ordered_railties.reverse.flatten.each do |r| if r == self initializers += current diff --git a/railties/lib/rails/initializable.rb b/railties/lib/rails/initializable.rb index 45547540f1e2b..59270cdfb41f8 100644 --- a/railties/lib/rails/initializable.rb +++ b/railties/lib/rails/initializable.rb @@ -9,23 +9,15 @@ def self.included(base) # :nodoc: end class Initializer - attr_reader :name, :block + attr_reader :name, :block, :before, :after - def initialize(name, context, options, &block) - options[:group] ||= :default - @name, @context, @options, @block = name, context, options, block - end - - def before - @options[:before] - end - - def after - @options[:after] + def initialize(name, context, before:, after:, group: nil, &block) + @group = group || :default + @name, @before, @after, @context, @block = name, before, after, context, block end def belongs_to?(group) - @options[:group] == group || @options[:group] == :all + @group == group || @group == :all end def run(*args) @@ -34,7 +26,7 @@ def run(*args) def bind(context) return self if @context - Initializer.new(@name, context, @options, &block) + Initializer.new(@name, context, before:, after:, group: @group, &block) end def context_class @@ -42,16 +34,54 @@ def context_class end end - class Collection < Array + class Collection + include Enumerable include TSort + def initialize(initializers = nil) + @order = Hash.new { |hash, key| hash[key] = Set.new } + @resolve = Hash.new { |hash, key| hash[key] = Set.new } + @collection = [] + concat(initializers) if initializers + end + + def to_a + @collection + end + + def last + @collection.last + end + + def each(&block) + @collection.each(&block) + end + alias :tsort_each_node :each def tsort_each_child(initializer, &block) - select { |i| i.before == initializer.name || i.name == initializer.after }.each(&block) + @order[initializer.name].each do |name| + @resolve[name].each(&block) + end end def +(other) - Collection.new(to_a + other.to_a) + dup.concat(other.to_a) + end + + def <<(initializer) + @collection << initializer + @order[initializer.before] << initializer.name if initializer.before + @order[initializer.name] << initializer.after if initializer.after + @resolve[initializer.name] << initializer + end + + def concat(initializers) + initializers.each(&method(:<<)) + self + end + + def has?(name) + @resolve.key?(name) end end @@ -87,8 +117,10 @@ def initializers_for(binding) def initializer(name, opts = {}, &blk) raise ArgumentError, "A block must be passed when defining an initializer" unless blk - opts[:after] ||= initializers.last.name unless initializers.empty? || initializers.find { |i| i.name == opts[:before] } - initializers << Initializer.new(name, nil, opts, &blk) + opts[:after] ||= initializers.last&.name unless initializers.has?(opts[:before]) + initializers << Initializer.new( + name, nil, before: opts[:before], after: opts[:after], group: opts[:group], &blk + ) end end end diff --git a/railties/test/initializable_test.rb b/railties/test/initializable_test.rb index 59fee245f9b4d..b32124bccc665 100644 --- a/railties/test/initializable_test.rb +++ b/railties/test/initializable_test.rb @@ -144,6 +144,44 @@ def self.initializers end end + module Duplicate + class PluginA + include Rails::Initializable + + initializer "plugin_a.startup" do + $arr << 1 + end + + initializer "plugin_a.terminate" do + $arr << 4 + end + end + + class PluginB + include Rails::Initializable + + initializer "plugin_b.startup", after: "plugin_a.startup" do + $arr << 2 + end + + initializer "plugin_b.terminate", before: "plugin_a.terminate" do + $arr << 3 + end + end + + class Application + include Rails::Initializable + + def self.initializers + @initializers ||= (PluginA.initializers + PluginB.initializers + PluginB.initializers) + end + + initializer "root" do + $arr << 5 + end + end + end + class Basic < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation @@ -200,6 +238,12 @@ class BeforeAfter < ActiveSupport::TestCase Interdependent::Application.new.run_initializers assert_equal [1, 2, 3, 4], $arr end + + test "handles duplicate initializers" do + $arr = [] + Duplicate::Application.new.run_initializers + assert_equal [1, 2, 2, 3, 3, 4, 5], $arr + end end class InstanceTest < ActiveSupport::TestCase