Permalink
Browse files

Refactor logger.

- When no logger is set, avoid the computational
  work of `log`, `request_summary` and `response_summary`.
- This can have a noticable impact for suites that
  do lots of VCR logging (see #311).
- This is an alternate form of the improvement
  provided in #311, with a couple further improvements:
  - Retain benefits of null object pattern, rather
    than littering these methods with `if logger`
    conditionals.
  - Provide the perf improvement by default, rather
    than only if `debug_logger` is set to `nil`.
  • Loading branch information...
1 parent 8be9de6 commit d0a70e15326e95c937106e5c89004516dff311a9 @myronmarston myronmarston committed Sep 22, 2013
View
@@ -13,6 +13,12 @@ Bug Fixes:
requiring special handling. (James Bence)
* Explicitly support the latest WebMock (1.13). (Ron Smith)
+Enhancements:
+
+* Improve perf when no logger is used by having it short-circuit
+ and not bother formatting a logger message that won't be
+ printed, anyway (Luan Santos and Matt Parker).
+
## 2.5.0 (May 18, 2013)
[Full Changelog](http://github.com/vcr/vcr/compare/v2.4.0...v2.5.0)
@@ -52,4 +52,11 @@ def prepare_cassette
# 1.480000 0.020000 1.500000 ( 1.500136)
# 1.390000 0.000000 1.390000 ( 1.395503)
# 1.400000 0.010000 1.410000 ( 1.403931)
+#
+# After applying my alternate fix:
+#
+# Ruby ruby 1.9.3p448 (2013-06-27 revision 41675) [x86_64-darwin12.4.0]
+# 1.400000 0.010000 1.410000 ( 1.410103)
+# 1.380000 0.010000 1.390000 ( 1.388467)
+# 1.360000 0.010000 1.370000 ( 1.364418)
View
@@ -5,7 +5,7 @@
module VCR
# The media VCR uses to store HTTP interactions for later re-use.
class Cassette
- include Logger
+ include Logger::Mixin
# The supported record modes.
#
@@ -2,7 +2,7 @@ module VCR
class Cassette
# @private
class HTTPInteractionList
- include Logger
+ include Logger::Mixin
# @private
module NullList
@@ -7,7 +7,7 @@ module VCR
class Configuration
include Hooks
include VariableArgsBlockCaller
- include Logger
+ include Logger::Mixin
# Gets the directory to read cassettes from and write cassettes to.
#
@@ -430,7 +430,21 @@ def configure_rspec_metadata!
# VCR.configure do |c|
# c.debug_logger = File.open('vcr.log', 'w')
# end
- attr_accessor :debug_logger
+ attr_reader :debug_logger
+ # @private (documented above)
+ def debug_logger=(value)
+ @debug_logger = value
+
+ if value
+ @logger = Logger.new(value)
+ else
+ @logger = Logger::Null
+ end
+ end
+
+ # @private
+ # Logger object that provides logging APIs and helper methods.
+ attr_reader :logger
# Sets a callback that determines whether or not to base64 encode
# the bytes of a request or response body during serialization in
@@ -479,7 +493,7 @@ def initialize
self.uri_parser = URI
self.query_parser = CGI.method(:parse)
- self.debug_logger = NullDebugLogger
+ self.debug_logger = nil
register_built_in_hooks
end
@@ -537,11 +551,5 @@ def log_prefix
# @private
define_hook :after_library_hooks_loaded
end
-
- # @private
- module NullDebugLogger
- extend self
- def puts(*); end
- end
end
@@ -1,7 +1,7 @@
module VCR
# @private
class RequestHandler
- include Logger
+ include Logger::Mixin
def handle
log "Handling request: #{request_summary} (disabled: #{disabled?})"
@@ -1,14 +1,15 @@
module VCR
# @private
- module Logger
- def log(message, indentation_level = 0)
- indentation = ' ' * indentation_level
- log_message = indentation + log_prefix + message
- VCR.configuration.debug_logger.puts log_message
+ # Provides log message formatting helper methods.
+ class Logger
+ def initialize(stream)
+ @stream = stream
end
- def log_prefix
- ''
+ def log(message, log_prefix, indentation_level = 0)
+ indentation = ' ' * indentation_level
+ log_message = indentation + log_prefix + message
+ @stream.puts log_message
end
def request_summary(request, request_matchers)
@@ -21,5 +22,38 @@ def request_summary(request, request_matchers)
def response_summary(response)
"[#{response.status.code} #{response.body[0, 80].inspect}]"
end
+
+ # @private
+ # A null-object version of the Logger. Used when
+ # a `debug_logger` has not been set.
+ #
+ # @note We used to use a null object for the `debug_logger` itself,
+ # but some users noticed a negative perf impact from having the
+ # logger formatting logic still executing in that case, so we
+ # moved the null object interface up a layer to here.
+ module Null
+ module_function
+
+ def log(*); end
+ def request_summary(*); end
+ def response_summary(*); end
+ end
+
+ # @private
+ # Provides common logger helper methods that simply delegate to
+ # the underlying logger object.
+ module Mixin
+ def log(message, indentation_level = 0)
+ VCR.configuration.logger.log(message, log_prefix, indentation_level)
+ end
+
+ def request_summary(*args)
+ VCR.configuration.logger.request_summary(*args)
+ end
+
+ def response_summary(*args)
+ VCR.configuration.logger.response_summary(*args)
+ end
+ end
end
end

0 comments on commit d0a70e1

Please sign in to comment.