Some optimizations on AS::Notifications. This does not change the public-facing API.

2 files changed, 51 insertions, 40 deletions

diff --git a/activesupport/lib/active_support/notifications.rb b/activesupport/lib/active_support/notifications.rb
index 7e9ffca13f..8e705a4eaa 100644
--- a/activesupport/lib/active_support/notifications.rb
+++ b/activesupport/lib/active_support/notifications.rb
@@ -62,15 +62,14 @@ module ActiveSupport
     class Instrumenter
       def initialize(publisher)
         @publisher = publisher
+        @id        = SecureRandom.hex(10)
       end
 
       def instrument(name, payload={})
-        payload[:time]      = Time.now
-        payload[:thread_id] = Thread.current.object_id
-        payload[:result]    = yield if block_given?
+        time   = Time.now
+        result = yield if block_given?
       ensure
-        payload[:duration] = 1000 * (Time.now.to_f - payload[:time].to_f)
-        @publisher.publish(name, payload)
+        @publisher.publish(name, time, Time.now, result, @id, payload)
       end
     end
 
@@ -79,8 +78,8 @@ module ActiveSupport
         @queue = queue
       end
 
-      def publish(name, payload)
-        @queue.publish(name, payload)
+      def publish(*args)
+        @queue.publish(*args)
       end
     end
 
@@ -95,27 +94,31 @@ module ActiveSupport
       end
 
       def subscribe
-        @queue.subscribe(@pattern) do |name, payload|
-          yield Event.new(name, payload)
+        @queue.subscribe(@pattern) do |*args|
+          yield Event.new(*args)
         end
       end
     end
 
     class Event
-      attr_reader :name, :time, :duration, :thread_id, :result, :payload
+      attr_reader :name, :time, :end, :thread_id, :result, :payload
 
-      def initialize(name, payload)
+      def initialize(name, start, ending, result, thread_id, payload)
         @name      = name
         @payload   = payload.dup
-        @time      = @payload.delete(:time)
-        @thread_id = @payload.delete(:thread_id)
-        @result    = @payload.delete(:result)
-        @duration  = @payload.delete(:duration)
+        @time      = start
+        @thread_id = thread_id
+        @end       = ending
+        @result    = result
+      end
+
+      def duration
+        @duration ||= 1000.0 * (@end - @time)
       end
 
       def parent_of?(event)
         start = (self.time - event.time) * 1000
-        start <= 0 && (start + self.duration >= event.duration)
+        start <= 0 && (start + duration >= event.duration)
       end
     end
 
@@ -124,12 +127,13 @@ module ActiveSupport
     #
     class LittleFanout
       def initialize
-        @listeners, @stream = [], Queue.new
-        @thread = Thread.new { consume }
+        @listeners = []
+        @stream    = Queue.new
+        Thread.new { consume }
       end
 
-      def publish(*event)
-        @stream.push(event)
+      def publish(*args)
+        @stream.push(args)
       end
 
       def subscribe(pattern=nil, &block)
@@ -137,30 +141,30 @@ module ActiveSupport
       end
 
       def consume
-        while event = @stream.shift
-          @listeners.each { |l| l.publish(*event) }
+        while args = @stream.shift
+          @listeners.each { |l| l.publish(*args) }
         end
       end
 
       class Listener
-        attr_reader :thread
+        # attr_reader :thread
 
         def initialize(pattern, &block)
           @pattern = pattern
           @subscriber = block
           @queue = Queue.new
-          @thread = Thread.new { consume }
+          Thread.new { consume }
         end
 
-        def publish(name, payload)
-          unless @pattern && !(@pattern === name.to_s)
-            @queue << [name, payload]
+        def publish(name, *args)
+          if !@pattern || @pattern === name.to_s
+            @queue << args.unshift(name)
           end
         end
 
         def consume
-          while event = @queue.shift
-            @subscriber.call(*event)
+          while args = @queue.shift
+            @subscriber.call(*args)
           end
         end
       end
diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb
index 561ee2b0ba..7d2bdf5ccf 100644
--- a/activesupport/test/notifications_test.rb
+++ b/activesupport/test/notifications_test.rb
@@ -8,25 +8,28 @@ class ActiveSupport::Notifications::LittleFanout
 end
 
 class NotificationsEventTest < Test::Unit::TestCase
-  def test_events_are_initialized_with_name_and_payload
-    event = event(:foo, :payload => :bar)
+  def test_events_are_initialized_with_details
+    event = event(:foo, Time.now, Time.now + 1, 1, random_id, :payload => :bar)
     assert_equal :foo, event.name
     assert_equal Hash[:payload => :bar], event.payload
   end
 
   def test_events_consumes_information_given_as_payload
-    event = event(:foo, :time => (time = Time.now), :result => 1, :duration => 10)
+    time = Time.now
+    event = event(:foo, time, time + 0.01, 1, random_id, {})
 
     assert_equal Hash.new, event.payload
     assert_equal time, event.time
     assert_equal 1, event.result
-    assert_equal 10, event.duration
+    assert_equal 10.0, event.duration
   end
 
   def test_event_is_parent_based_on_time_frame
-    parent    = event(:foo, :time => Time.utc(2009), :duration => 10000)
-    child     = event(:foo, :time => Time.utc(2009, 01, 01, 0, 0, 1), :duration => 1000)
-    not_child = event(:foo, :time => Time.utc(2009, 01, 01, 0, 0, 1), :duration => 10000)
+    time = Time.utc(2009, 01, 01, 0, 0, 1)
+
+    parent    = event(:foo, Time.utc(2009), Time.utc(2009) + 100, nil, random_id, {})
+    child     = event(:foo, time, time + 10, nil, random_id, {})
+    not_child = event(:foo, time, time + 100, nil, random_id, {})
 
     assert parent.parent_of?(child)
     assert !child.parent_of?(parent)
@@ -34,11 +37,15 @@ class NotificationsEventTest < Test::Unit::TestCase
     assert !not_child.parent_of?(parent)
   end
 
-  protected
+protected
 
-    def event(*args)
-      ActiveSupport::Notifications::Event.new(*args)
-    end
+  def random_id
+    @random_id ||= ActiveSupport::SecureRandom.hex(10)
+  end
+
+  def event(*args)
+    ActiveSupport::Notifications::Event.new(*args)
+  end
 end
 
 class NotificationsMainTest < Test::Unit::TestCase