From d7ce956dacd0b772273d39b8ed31a30cff7ecf38 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Wed, 4 Sep 2024 21:16:32 +0200 Subject: [PATCH] Remove dependency on logger * To fix the Ruby 3.3.5 warnings: https://github.com/ruby-concurrency/concurrent-ruby/issues/1061 * concurrent-ruby only uses 7 constants from Logger, so just copy those over. --- README.md | 2 +- docs-source/actor/celluloid_benchmark.rb | 6 +--- docs-source/actor/io.in.rb | 3 +- docs-source/actor/io.out.rb | 3 +- docs-source/actor/main.md | 8 ++--- examples/init.rb | 2 +- .../concurrent/actor/context.rb | 4 +-- .../concurrent/actor/internal_delegations.rb | 4 +-- .../concurrent/edge/erlang_actor.rb | 18 ++++++------ .../concurrent/concern/logging.rb | 29 +++++++++++-------- spec/spec_helper.rb | 2 +- 11 files changed, 40 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index 66a6983b0..63ee743c5 100644 --- a/README.md +++ b/README.md @@ -284,7 +284,7 @@ To use the tools in the Edge gem it must be required separately: require 'concurrent-edge' ``` -If the library does not behave as expected, `Concurrent.use_stdlib_logger(Logger::DEBUG)` could +If the library does not behave as expected, `Concurrent.use_simple_logger(:DEBUG)` could help to reveal the problem. ## Installation diff --git a/docs-source/actor/celluloid_benchmark.rb b/docs-source/actor/celluloid_benchmark.rb index dea20e553..30ca1e0d6 100644 --- a/docs-source/actor/celluloid_benchmark.rb +++ b/docs-source/actor/celluloid_benchmark.rb @@ -7,11 +7,7 @@ # require 'stackprof' # require 'profiler' -logger = Logger.new($stderr) -logger.level = Logger::INFO -Concurrent.configuration.logger = lambda do |level, progname, message = nil, &block| - logger.add level, message, progname, &block -end +Concurrent.use_simple_logger(:INFO) scale = 1 ADD_TO = (100 * scale).to_i diff --git a/docs-source/actor/io.in.rb b/docs-source/actor/io.in.rb index 6bbc1a07c..3c6a1bdb5 100644 --- a/docs-source/actor/io.in.rb +++ b/docs-source/actor/io.in.rb @@ -1,7 +1,6 @@ require 'concurrent' -# logger = Logger.new(STDOUT) -# Concurrent.configuration.logger = logger.method(:add) +# Concurrent.use_simple_logger(:WARN, STDOUT) # First option is to use operation pool diff --git a/docs-source/actor/io.out.rb b/docs-source/actor/io.out.rb index b96cfc72a..b879c4fb8 100644 --- a/docs-source/actor/io.out.rb +++ b/docs-source/actor/io.out.rb @@ -1,7 +1,6 @@ require 'concurrent' # => false -# logger = Logger.new(STDOUT) -# Concurrent.configuration.logger = logger.method(:add) +# Concurrent.use_simple_logger(:WARN, STDOUT) # First option is to use operation pool diff --git a/docs-source/actor/main.md b/docs-source/actor/main.md index 43cf72798..e3e67f62c 100644 --- a/docs-source/actor/main.md +++ b/docs-source/actor/main.md @@ -124,12 +124,12 @@ Spawned actor cannot be garbage-collected until it's terminated. There is a refe Actors are running on shared thread poll which allows user to create many actors cheaply. Downside is that these actors cannot be directly used to do IO or other blocking operations. -Blocking operations could starve the `default_task_pool`. However there are two options: +Blocking operations could starve the `global_fast_executor`. However there are two options: -- Create an regular actor which will schedule blocking operations in `global_operation_pool` +- Create an regular actor which will schedule blocking operations in `global_io_executor` (which is intended for blocking operations) sending results back to self in messages. -- Create an actor using `global_operation_pool` instead of `global_task_pool`, e.g. - `AnIOActor.spawn name: :blocking, executor: Concurrent.configuration.global_operation_pool`. +- Create an actor using `global_io_executor` instead of `global_fast_executor`, e.g. + `AnIOActor.spawn name: :blocking, executor: Concurrent.global_io_executor`. ### Example diff --git a/examples/init.rb b/examples/init.rb index 03c91f7c1..92d2c63af 100644 --- a/examples/init.rb +++ b/examples/init.rb @@ -4,4 +4,4 @@ def do_stuff(*args) :stuff end -Concurrent.use_simple_logger Logger::DEBUG +Concurrent.use_simple_logger :DEBUG diff --git a/lib/concurrent-ruby-edge/concurrent/actor/context.rb b/lib/concurrent-ruby-edge/concurrent/actor/context.rb index 19ee14ab7..96252901e 100644 --- a/lib/concurrent-ruby-edge/concurrent/actor/context.rb +++ b/lib/concurrent-ruby-edge/concurrent/actor/context.rb @@ -84,7 +84,7 @@ def default_reference_class Reference end - # override to se different default executor, e.g. to change it to global_operation_pool + # override to se different default executor, e.g. to change it to global_fast_executor # @return [Executor] def default_executor Concurrent.global_io_executor @@ -109,7 +109,7 @@ def ask(message) # @example by option hash # inc2 = AdHoc.spawn(name: 'increment by 2', # args: [2], - # executor: Concurrent.configuration.global_task_pool) do |increment_by| + # executor: Concurrent.global_fast_executor) do |increment_by| # lambda { |number| number + increment_by } # end # inc2.ask!(2) # => 4 diff --git a/lib/concurrent-ruby-edge/concurrent/actor/internal_delegations.rb b/lib/concurrent-ruby-edge/concurrent/actor/internal_delegations.rb index ddca34883..9ed051b5a 100644 --- a/lib/concurrent-ruby-edge/concurrent/actor/internal_delegations.rb +++ b/lib/concurrent-ruby-edge/concurrent/actor/internal_delegations.rb @@ -1,11 +1,11 @@ -require 'logger' +require 'concurrent/concern/logging' require 'concurrent/actor/public_delegations' module Concurrent module Actor module InternalDelegations include PublicDelegations - include Logger::Severity + include Concurrent::Concern::Logging # @see Core#children def children diff --git a/lib/concurrent-ruby-edge/concurrent/edge/erlang_actor.rb b/lib/concurrent-ruby-edge/concurrent/edge/erlang_actor.rb index 2277a977c..6690e9e26 100644 --- a/lib/concurrent-ruby-edge/concurrent/edge/erlang_actor.rb +++ b/lib/concurrent-ruby-edge/concurrent/edge/erlang_actor.rb @@ -674,7 +674,7 @@ def initialize(mailbox, environment, name, executor) end def tell_op(message) - log Logger::DEBUG, @Pid, told: message + log DEBUG, @Pid, told: message if (mailbox = @Mailbox) mailbox.push_op(message).then { @Pid } else @@ -683,7 +683,7 @@ def tell_op(message) end def tell(message, timeout = nil) - log Logger::DEBUG, @Pid, told: message + log DEBUG, @Pid, told: message if (mailbox = @Mailbox) timed_out = mailbox.push message, timeout timeout ? timed_out : @Pid @@ -693,7 +693,7 @@ def tell(message, timeout = nil) end def ask(message, timeout, timeout_value) - log Logger::DEBUG, @Pid, asked: message + log DEBUG, @Pid, asked: message if @Terminated.resolved? raise NoActor.new(@Pid) else @@ -724,7 +724,7 @@ def ask(message, timeout, timeout_value) end def ask_op(message, probe) - log Logger::DEBUG, @Pid, asked: message + log DEBUG, @Pid, asked: message if @Terminated.resolved? probe.reject NoActor.new(@Pid), false else @@ -1029,7 +1029,7 @@ def terminate_self(reason, value) end def after_termination(final_reason) - log Logger::DEBUG, @Pid, terminated: final_reason + log DEBUG, @Pid, terminated: final_reason clean_reply NoActor.new(@Pid) while true message = @Mailbox.try_pop NOTHING @@ -1071,7 +1071,7 @@ def run(*args, &body) inner_run(*args, &body). run(Run::TEST). then(&method(:after_termination)). - rescue { |e| log Logger::ERROR, e } + rescue { |e| log ERROR, e } end def receive(*rules, timeout: nil, timeout_value: nil, keep: false, &given_block) @@ -1163,7 +1163,7 @@ def internal_receive end message_future.then(start, self) do |message, s, _actor| - log Logger::DEBUG, pid, got: message + log DEBUG, pid, got: message catch(JUMP) do if (message = consume_signal(message)) == NOTHING @timeout = [@timeout + s - Concurrent.monotonic_time, 0].max if s @@ -1230,7 +1230,7 @@ def receive(*rules, timeout: nil, timeout_value: nil, &given_block) matcher = -> m { m.is_a?(Ask) ? rules_matcher === m.message : rules_matcher === m } while true message = @Mailbox.pop_matching(matcher, timeout, TIMEOUT) - log Logger::DEBUG, pid, got: message + log DEBUG, pid, got: message unless (message = consume_signal(message)) == NOTHING rules.each do |rule, job| return eval_task(message, job) if rule === message @@ -1535,7 +1535,7 @@ class NoReply < Error def self.create(type, channel, environment, name, executor) actor = KLASS_MAP.fetch(type).new(channel, environment, name, executor) ensure - log Logger::DEBUG, actor.pid, created: caller[1] if actor + log Concern::Logging::DEBUG, actor.pid, created: caller[1] if actor end KLASS_MAP = { diff --git a/lib/concurrent-ruby/concurrent/concern/logging.rb b/lib/concurrent-ruby/concurrent/concern/logging.rb index 568a539eb..d1aae81ae 100644 --- a/lib/concurrent-ruby/concurrent/concern/logging.rb +++ b/lib/concurrent-ruby/concurrent/concern/logging.rb @@ -1,4 +1,3 @@ -require 'logger' require 'concurrent/atomic/atomic_reference' module Concurrent @@ -8,10 +7,12 @@ module Concern # # @!visibility private module Logging - include Logger::Severity + # The same as Logger::Severity but we copy it here to avoid a dependency on the logger gem just for these 7 constants + DEBUG, INFO, WARN, ERROR, FATAL, UNKNOWN = 0, 1, 2, 3, 4, 5 + SEV_LABEL = %w[DEBUG INFO WARN ERROR FATAL ANY].freeze # Logs through {Concurrent.global_logger}, it can be overridden by setting @logger - # @param [Integer] level one of Logger::Severity constants + # @param [Integer] level one of Concurrent::Concern::Logging constants # @param [String] progname e.g. a path of an Actor # @param [String, nil] message when nil block is used to generate the message # @yieldreturn [String] a message @@ -23,7 +24,7 @@ def log(level, progname, message = nil, &block) end logger.call level, progname, message, &block rescue => error - $stderr.puts "`Concurrent.configuration.logger` failed to log #{[level, progname, message, block]}\n" + + $stderr.puts "`Concurrent.global_logger` failed to log #{[level, progname, message, block]}\n" + "#{error.message} (#{error.class})\n#{error.backtrace.join "\n"}" end end @@ -33,8 +34,10 @@ def log(level, progname, message = nil, &block) module Concurrent extend Concern::Logging - # @return [Logger] Logger with provided level and output. - def self.create_simple_logger(level = Logger::FATAL, output = $stderr) + # Create a simple logger with provided level and output. + def self.create_simple_logger(level = :FATAL, output = $stderr) + level = Concern::Logging.const_get(level) unless level.is_a?(Integer) + # TODO (pitr-ch 24-Dec-2016): figure out why it had to be replaced, stdlogger was deadlocking lambda do |severity, progname, message = nil, &block| return false if severity < level @@ -52,7 +55,7 @@ def self.create_simple_logger(level = Logger::FATAL, output = $stderr) output.print format "[%s] %5s -- %s: %s\n", Time.now.strftime('%Y-%m-%d %H:%M:%S.%L'), - Logger::SEV_LABEL[severity], + Concern::Logging::SEV_LABEL[severity], progname, formatted_message true @@ -60,13 +63,15 @@ def self.create_simple_logger(level = Logger::FATAL, output = $stderr) end # Use logger created by #create_simple_logger to log concurrent-ruby messages. - def self.use_simple_logger(level = Logger::FATAL, output = $stderr) + def self.use_simple_logger(level = :FATAL, output = $stderr) Concurrent.global_logger = create_simple_logger level, output end - # @return [Logger] Logger with provided level and output. + # Create a stdlib logger with provided level and output. + # If you use this deprecated method you might need to add logger to your Gemfile to avoid warnings from Ruby 3.3.5+. # @deprecated - def self.create_stdlib_logger(level = Logger::FATAL, output = $stderr) + def self.create_stdlib_logger(level = :FATAL, output = $stderr) + require 'logger' logger = Logger.new(output) logger.level = level logger.formatter = lambda do |severity, datetime, progname, msg| @@ -93,7 +98,7 @@ def self.create_stdlib_logger(level = Logger::FATAL, output = $stderr) # Use logger created by #create_stdlib_logger to log concurrent-ruby messages. # @deprecated - def self.use_stdlib_logger(level = Logger::FATAL, output = $stderr) + def self.use_stdlib_logger(level = :FATAL, output = $stderr) Concurrent.global_logger = create_stdlib_logger level, output end @@ -103,7 +108,7 @@ def self.use_stdlib_logger(level = Logger::FATAL, output = $stderr) NULL_LOGGER = lambda { |level, progname, message = nil, &block| } # @!visibility private - GLOBAL_LOGGER = AtomicReference.new(create_simple_logger(Logger::WARN)) + GLOBAL_LOGGER = AtomicReference.new(create_simple_logger(:WARN)) private_constant :GLOBAL_LOGGER def self.global_logger diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d191183f6..1f0048647 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -42,7 +42,7 @@ def requires=(paths) config.before :all do # Only configure logging if it has been required, to make sure the necessary require's are in place if Concurrent.respond_to? :use_simple_logger - Concurrent.use_simple_logger Logger::FATAL + Concurrent.use_simple_logger :FATAL end end