From ff0d842454571d78addd1fe9d4f232b600881b1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 25 Jul 2010 20:46:42 +0200 Subject: Revert the previous three commits. * AS::Notifications#instrument should not measure anything, it is not its responsibility; * Adding another argument to AS::Notifications#instrument API needs to be properly discussed; --- .../connection_adapters/abstract_adapter.rb | 18 ++---------------- activerecord/lib/active_record/log_subscriber.rb | 16 +++++++++++++++- .../lib/active_record/railties/controller_runtime.rb | 5 ++--- activerecord/test/cases/log_subscriber_test.rb | 4 ++++ activesupport/lib/active_support/notifications.rb | 16 +++------------- .../lib/active_support/notifications/instrumenter.rb | 11 ++++------- activesupport/test/notifications_test.rb | 9 --------- 7 files changed, 30 insertions(+), 49 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 618560087e..c07ba2ac74 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -36,15 +36,11 @@ module ActiveRecord define_callbacks :checkout, :checkin - @@row_even = true - def initialize(connection, logger = nil) #:nodoc: @active = nil @connection, @logger = connection, logger - @runtime = 0 @query_cache_enabled = false @query_cache = {} - @instrumenter = ActiveSupport::Notifications.instrumenter end # Returns the human-readable name of the adapter. Use mixed case - one @@ -93,11 +89,6 @@ module ActiveRecord false end - def reset_runtime #:nodoc: - rt, @runtime = @runtime, 0 - rt - end - # QUOTING ================================================== # Override to return the quoted table name. Defaults to column quoting. @@ -200,15 +191,10 @@ module ActiveRecord def log(sql, name) name ||= "SQL" - info = {} - - result = @instrumenter.instrument("sql.active_record", - {:sql => sql, :name => name, :connection_id => object_id}, info) do + ActiveSupport::Notifications.instrument("sql.active_record", + :sql => sql, :name => name, :connection_id => object_id) do yield end - @runtime += info[:elapsed] - - result rescue Exception => e message = "#{e.class.name}: #{e.message}: #{sql}" @logger.debug message if @logger diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 278e192e59..c7ae12977a 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -1,11 +1,25 @@ module ActiveRecord class LogSubscriber < ActiveSupport::LogSubscriber + def self.runtime=(value) + Thread.current["active_record_sql_runtime"] = value + end + + def self.runtime + Thread.current["active_record_sql_runtime"] ||= 0 + end + + def self.reset_runtime + rt, self.runtime = runtime, 0 + rt + end + def initialize super @odd_or_even = false end def sql(event) + self.class.runtime += event.duration return unless logger.debug? name = '%s (%.1fms)' % [event.payload[:name], event.duration] @@ -31,4 +45,4 @@ module ActiveRecord end end -ActiveRecord::LogSubscriber.attach_to :active_record +ActiveRecord::LogSubscriber.attach_to :active_record \ No newline at end of file diff --git a/activerecord/lib/active_record/railties/controller_runtime.rb b/activerecord/lib/active_record/railties/controller_runtime.rb index cf74fa1655..bc6ca936c0 100644 --- a/activerecord/lib/active_record/railties/controller_runtime.rb +++ b/activerecord/lib/active_record/railties/controller_runtime.rb @@ -11,10 +11,9 @@ module ActiveRecord def cleanup_view_runtime if ActiveRecord::Base.connected? - connection = ActiveRecord::Base.connection - db_rt_before_render = connection.reset_runtime + db_rt_before_render = ActiveRecord::LogSubscriber.reset_runtime runtime = super - db_rt_after_render = connection.reset_runtime + db_rt_after_render = ActiveRecord::LogSubscriber.reset_runtime self.db_runtime = db_rt_before_render + db_rt_after_render runtime - db_rt_after_render else diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index 342daa19df..cbaaca764b 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -57,4 +57,8 @@ class LogSubscriberTest < ActiveRecord::TestCase wait assert_equal 0, @logger.logged(:debug).size end + + def test_initializes_runtime + Thread.new { assert_equal 0, ActiveRecord::LogSubscriber.runtime }.join + end end diff --git a/activesupport/lib/active_support/notifications.rb b/activesupport/lib/active_support/notifications.rb index d65431a943..886d7183eb 100644 --- a/activesupport/lib/active_support/notifications.rb +++ b/activesupport/lib/active_support/notifications.rb @@ -47,21 +47,11 @@ module ActiveSupport attr_writer :notifier delegate :publish, :unsubscribe, :to => :notifier - def instrument(name, payload = {}, info = nil) + def instrument(name, payload = {}) if @instrumenters[name] - instrumenter.instrument(name, payload, info) { - yield payload if block_given? - } + instrumenter.instrument(name, payload) { yield payload if block_given? } else - value = nil - if block_given? - if info - info[:elapsed] = Benchmark.ms { value = yield payload } - else - value = yield payload - end - end - value + yield payload if block_given? end end diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb index 4fc446fb2b..441fefb491 100644 --- a/activesupport/lib/active_support/notifications/instrumenter.rb +++ b/activesupport/lib/active_support/notifications/instrumenter.rb @@ -14,19 +14,16 @@ module ActiveSupport # Instrument the given block by measuring the time taken to execute it # and publish it. Notice that events get sent even if an error occurs # in the passed-in block - def instrument(name, payload={}, info = nil) + def instrument(name, payload={}) + started = Time.now + begin - started = Time.now yield rescue Exception => e payload[:exception] = [e.class.name, e.message] raise e ensure - finished = Time.now - if info - info[:elapsed] = 1000.0 * (finished.to_f - started.to_f) - end - @notifier.publish(name, started, finished, @id, payload) + @notifier.publish(name, started, Time.now, @id, payload) end end diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index 41e8ca4ae7..9faa11efbc 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -172,15 +172,6 @@ module Notifications :exception => ["RuntimeError", "FAIL"]], @events.last.payload end - def test_elapsed - instrument(:something) do - sleep(0.001) - end - - # Elapsed returns duration in ms - assert_in_delta 1, ActiveSupport::Notifications.instrumenter.elapsed, 100 - end - def test_event_is_pushed_even_without_block instrument(:awesome, :payload => "notifications") assert_equal 1, @events.size -- cgit v1.2.3