Merge pull request rails#43390 from jhawthorn/remove_notification_event_children

Remove child event tracking from ActiveSupport::Subscriber

Author: jhawthorn

actionview/lib/action_view/log_subscriber.rb

@@ -50,28 +50,58 @@ def render_collection(event)
       end
     end
 
-    def start(name, id, payload)
-      log_rendering_start(payload, name)
+    module Utils # :nodoc:
+      def logger
+        ActionView::Base.logger
+      end
 
-      super
-    end
+    private
+      def from_rails_root(string)
+        string = string.sub(rails_root, "")
+        string.sub!(VIEWS_PATTERN, "")
+        string
+      end
 
-    def logger
-      ActionView::Base.logger
+      def rails_root # :doc:
+        @root ||= "#{Rails.root}/"
+      end
     end
 
-  private
-    EMPTY = ""
-    def from_rails_root(string) # :doc:
-      string = string.sub(rails_root, EMPTY)
-      string.sub!(VIEWS_PATTERN, EMPTY)
-      string
+    include Utils
+
+    class Start # :nodoc:
+      include Utils
+
+      def start(name, id, payload)
+        return unless logger
+        logger.debug do
+          qualifier =
+            if name == "render_template.action_view"
+              ""
+            elsif name == "render_layout.action_view"
+              "layout "
+            end
+
+          return unless qualifier
+
+          message = +"  Rendering #{qualifier}#{from_rails_root(payload[:identifier])}"
+          message << " within #{from_rails_root(payload[:layout])}" if payload[:layout]
+          message
+        end
+      end
+
+      def finish(name, id, payload)
+      end
     end
 
-    def rails_root # :doc:
-      @root ||= "#{Rails.root}/"
+    def self.attach_to(*)
+      ActiveSupport::Notifications.subscribe("render_template.action_view", ActionView::LogSubscriber::Start.new)
+      ActiveSupport::Notifications.subscribe("render_layout.action_view", ActionView::LogSubscriber::Start.new)
+
+      super
     end
 
+  private
     def render_count(payload) # :doc:
       if payload[:cache_hits]
         "[#{payload[:cache_hits]} / #{payload[:count]} cache hits]"
@@ -88,23 +118,6 @@ def cache_message(payload) # :doc:
         "[cache miss]"
       end
     end
-
-    def log_rendering_start(payload, name)
-      debug do
-        qualifier =
-          if name == "render_template.action_view"
-            ""
-          elsif name == "render_layout.action_view"
-            "layout "
-          end
-
-        return unless qualifier
-
-        message = +"  Rendering #{qualifier}#{from_rails_root(payload[:identifier])}"
-        message << " within #{from_rails_root(payload[:layout])}" if payload[:layout]
-        message
-      end
-    end
   end
 end
 

activesupport/CHANGELOG.md

@@ -1,3 +1,5 @@
+*   Deprecate `Notification::Event`'s `#children` and `#parent_of?`
+
 *   Change default serialization format of `MessageEncryptor` from `Marshal` to `JSON` for Rails 7.1.
 
     Existing apps are provided with an upgrade path to migrate to `JSON` as described in `guides/source/upgrading_ruby_on_rails.md`

activesupport/lib/active_support/log_subscriber.rb

@@ -107,14 +107,10 @@ def logger
       LogSubscriber.logger
     end
 
-    def start(name, id, payload)
-      super if logger
-    end
-
-    def finish(name, id, payload)
+    def call(event)
       super if logger
     rescue => e
-      log_exception(name, e)
+      log_exception(event.name, e)
     end
 
     def publish_event(event)

activesupport/lib/active_support/notifications/instrumenter.rb

@@ -56,7 +56,7 @@ def unique_id
     end
 
     class Event
-      attr_reader :name, :time, :end, :transaction_id, :children
+      attr_reader :name, :time, :end, :transaction_id
       attr_accessor :payload
 
       def initialize(name, start, ending, transaction_id, payload)
@@ -65,7 +65,6 @@ def initialize(name, start, ending, transaction_id, payload)
         @time           = start ? start.to_f * 1_000.0 : start
         @transaction_id = transaction_id
         @end            = ending ? ending.to_f * 1_000.0 : ending
-        @children       = []
         @cpu_time_start = 0.0
         @cpu_time_finish = 0.0
         @allocation_count_start = 0
@@ -117,6 +116,23 @@ def allocations
         @allocation_count_finish - @allocation_count_start
       end
 
+      def children # :nodoc:
+        ActiveSupport::Deprecation.warn <<~EOM
+          ActiveSupport::Notifications::Event#children is deprecated and will
+          be removed in Rails 7.2.
+        EOM
+        []
+      end
+
+      def parent_of?(event) # :nodoc:
+        ActiveSupport::Deprecation.warn <<~EOM
+          ActiveSupport::Notifications::Event#parent_of? is deprecated and will
+          be removed in Rails 7.2.
+        EOM
+        start = (time - event.time) * 1000
+        start <= 0 && (start + duration >= event.duration)
+      end
+
       # Returns the difference in milliseconds between when the execution of the
       # event started and when it ended.
       #
@@ -133,14 +149,6 @@ def duration
         self.end - time
       end
 
-      def <<(event)
-        @children << event
-      end
-
-      def parent_of?(event)
-        @children.include? event
-      end
-
       private
         def now
           Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_millisecond)

activesupport/lib/active_support/subscriber.rb

@@ -126,38 +126,18 @@ def fetch_public_methods(subscriber, inherit_all)
     attr_reader :patterns # :nodoc:
 
     def initialize
-      @queue_key = [self.class.name, object_id].join "-"
       @patterns  = {}
       super
     end
 
-    def start(name, id, payload)
-      event = ActiveSupport::Notifications::Event.new(name, nil, nil, id, payload)
-      event.start!
-      parent = event_stack.last
-      parent << event if parent
-
-      event_stack.push event
-    end
-
-    def finish(name, id, payload)
-      event = event_stack.pop
-      event.finish!
-      event.payload.merge!(payload)
-
-      method = name.split(".").first
+    def call(event)
+      method = event.name.split(".").first
       send(method, event)
     end
 
     def publish_event(event) # :nodoc:
       method = event.name.split(".").first
       send(method, event)
     end
-
-    private
-      def event_stack
-        registry = ActiveSupport::IsolatedExecutionState[:active_support_subscriber_queue_registry] ||= {}
-        registry[@queue_key] ||= []
-      end
   end
 end

activesupport/test/log_subscriber_test.rb

@@ -77,7 +77,9 @@ def test_event_is_an_active_support_notifications_event
 
   def test_event_attributes
     ActiveSupport::LogSubscriber.attach_to :my_log_subscriber, @log_subscriber
-    instrument "some_event.my_log_subscriber"
+    instrument "some_event.my_log_subscriber" do
+      [] # Make an allocation
+    end
     wait
     event = @log_subscriber.event
     if defined?(JRUBY_VERSION)

activesupport/test/notifications_test.rb

@@ -462,21 +462,6 @@ def test_events_consumes_information_given_as_payload
       assert_equal Hash[payload: :bar], event.payload
     end
 
-    def test_event_is_parent_based_on_children
-      time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
-
-      parent    = event(:foo, Process.clock_gettime(Process::CLOCK_MONOTONIC), Process.clock_gettime(Process::CLOCK_MONOTONIC) + 100, random_id, {})
-      child     = event(:foo, time, time + 10, random_id, {})
-      not_child = event(:foo, time, time + 100, random_id, {})
-
-      parent.children << child
-
-      assert parent.parent_of?(child)
-      assert_not child.parent_of?(parent)
-      assert_not parent.parent_of?(not_child)
-      assert_not not_child.parent_of?(parent)
-    end
-
     def test_subscribe_raises_error_on_non_supported_arguments
       notifier = ActiveSupport::Notifications::Fanout.new