Optimize AS::Notifications to remember which subscribers don't match and not run them. This will allow notifications that are only useful in dev or testing to run efficiently in production.

3 files changed, 42 insertions, 6 deletions

diff --git a/activesupport/lib/active_support/notifications.rb b/activesupport/lib/active_support/notifications.rb
index fca2efd969..3f1fe64e9b 100644
--- a/activesupport/lib/active_support/notifications.rb
+++ b/activesupport/lib/active_support/notifications.rb
@@ -44,7 +44,7 @@ module ActiveSupport
 
     class << self
       attr_writer :notifier
-      delegate :publish, :subscribe, :to => :notifier
+      delegate :publish, :subscribe, :unsubscribe, :to => :notifier
       delegate :instrument, :to => :instrumenter
 
       def notifier
diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb
index e08011e23f..cd60054862 100644
--- a/activesupport/lib/active_support/notifications/fanout.rb
+++ b/activesupport/lib/active_support/notifications/fanout.rb
@@ -5,6 +5,7 @@ module ActiveSupport
     class Fanout
       def initialize
         @subscribers = []
+        @listeners_for = {}
       end
 
       def bind(pattern)
@@ -12,16 +13,22 @@ module ActiveSupport
       end
 
       def subscribe(pattern = nil, &block)
+        @listeners_for.clear
         @subscribers << Subscriber.new(pattern, &block)
         @subscribers.last
       end
 
       def unsubscribe(subscriber)
         @subscribers.delete(subscriber)
+        @listeners_for.clear
       end
 
-      def publish(*args)
-        @subscribers.each { |s| s.publish(*args) }
+      def publish(name, *args)
+        if listeners = @listeners_for[name]
+          listeners.each { |s| s.publish(name, *args) }
+        else
+          @listeners_for[name] = @subscribers.select { |s| s.publish(name, *args) }
+        end
       end
 
       # This is a sync queue, so there is not waiting.
@@ -53,7 +60,9 @@ module ActiveSupport
         end
 
         def publish(*args)
-          push(*args) if matches?(args.first)
+          return unless matches?(args.first)
+          push(*args)
+          true
         end
 
         def drained?
diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb
index baee779b8a..67c3527e23 100644
--- a/activesupport/test/notifications_test.rb
+++ b/activesupport/test/notifications_test.rb
@@ -20,15 +20,20 @@ module Notifications
   end
 
   class UnsubscribeTest < TestCase
-    def unsubscribing_removes_a_subscription
+    def test_unsubscribing_removes_a_subscription
       @notifier.publish :foo
       @notifier.wait
       assert_equal [[:foo]], @events
       @notifier.unsubscribe(@subscription)
-      @notifier.publish :bar
+      @notifier.publish :foo
       @notifier.wait
       assert_equal [[:foo]], @events
     end
+
+  private
+    def event(*args)
+      args
+    end
   end
 
   class SyncPubSubTest < TestCase
@@ -38,6 +43,28 @@ module Notifications
       assert_equal [[:foo]], @events
     end
 
+    def test_publishing_multiple_times_works
+      @notifier.publish :foo
+      @notifier.publish :foo
+      @notifier.wait
+      assert_equal [[:foo], [:foo]], @events
+    end
+
+    def test_publishing_after_a_new_subscribe_works
+      @notifier.publish :foo
+      @notifier.publish :foo
+
+      @notifier.subscribe("not_existant") do |*args|
+        @events << ActiveSupport::Notifications::Event.new(*args)
+      end
+
+      @notifier.publish :foo
+      @notifier.publish :foo
+      @notifier.wait
+
+      assert_equal [[:foo]] * 4, @events
+    end
+
     def test_log_subscriber_with_pattern
       events = []
       @notifier.subscribe('1') { |*args| events << args }