From 547713b4c91e5c232ac3b2a8288b72001c6dfc46 Mon Sep 17 00:00:00 2001 From: Mike Perham Date: Tue, 5 Jan 2016 14:31:16 -0800 Subject: [PATCH] Move async execution from celluloid to concurrent-ruby This removes 8 runtime gem dependencies from Rails: ``` Using hitimes 1.2.3 Using timers 4.1.1 Using celluloid-essentials 0.20.5 Using celluloid-extras 0.20.5 Using celluloid-fsm 0.20.5 Using celluloid-pool 0.20.5 Using celluloid-supervision 0.20.5 Using celluloid 0.17.2 ``` --- Gemfile.lock | 2 +- actioncable/actioncable.gemspec | 2 +- .../action_cable/channel/periodic_timers.rb | 2 +- .../lib/action_cable/connection/base.rb | 2 +- .../lib/action_cable/process/logging.rb | 3 - actioncable/lib/action_cable/server/base.rb | 3 - actioncable/lib/action_cable/server/worker.rb | 59 ++++++++++++++----- actioncable/test/test_helper.rb | 5 -- actioncable/test/worker_test.rb | 2 - 9 files changed, 49 insertions(+), 31 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 7c6622dbed..baa8dc15c4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -31,8 +31,8 @@ PATH specs: actioncable (5.0.0.beta1) actionpack (= 5.0.0.beta1) - celluloid (~> 0.17.2) coffee-rails (~> 4.1.0) + concurrent-ruby (~> 1.0.0) em-hiredis (~> 0.3.0) faye-websocket (~> 0.10.0) redis (~> 3.0) diff --git a/actioncable/actioncable.gemspec b/actioncable/actioncable.gemspec index 74c21bd24d..b6e529e879 100644 --- a/actioncable/actioncable.gemspec +++ b/actioncable/actioncable.gemspec @@ -23,7 +23,7 @@ Gem::Specification.new do |s| s.add_dependency 'coffee-rails', '~> 4.1.0' s.add_dependency 'faye-websocket', '~> 0.10.0' s.add_dependency 'websocket-driver', '~> 0.6.1' - s.add_dependency 'celluloid', '~> 0.17.2' + s.add_dependency 'concurrent-ruby', '~> 1.0.0' s.add_dependency 'em-hiredis', '~> 0.3.0' s.add_dependency 'redis', '~> 3.0' diff --git a/actioncable/lib/action_cable/channel/periodic_timers.rb b/actioncable/lib/action_cable/channel/periodic_timers.rb index 25fe8e5e54..7f0fb37afc 100644 --- a/actioncable/lib/action_cable/channel/periodic_timers.rb +++ b/actioncable/lib/action_cable/channel/periodic_timers.rb @@ -28,7 +28,7 @@ module ActionCable def start_periodic_timers self.class.periodic_timers.each do |callback, options| active_periodic_timers << EventMachine::PeriodicTimer.new(options[:every]) do - connection.worker_pool.async.run_periodic_timer(self, callback) + connection.worker_pool.async_run_periodic_timer(self, callback) end end end diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb index 977856d656..a8cfdf90f3 100644 --- a/actioncable/lib/action_cable/connection/base.rb +++ b/actioncable/lib/action_cable/connection/base.rb @@ -103,7 +103,7 @@ module ActionCable # Invoke a method on the connection asynchronously through the pool of thread workers. def send_async(method, *arguments) - worker_pool.async.invoke(self, method, *arguments) + worker_pool.async_invoke(self, method, *arguments) end # Return a basic hash of statistics for the connection keyed with `identifier`, `started_at`, and `subscriptions`. diff --git a/actioncable/lib/action_cable/process/logging.rb b/actioncable/lib/action_cable/process/logging.rb index 72b1a080d1..dce637b3ca 100644 --- a/actioncable/lib/action_cable/process/logging.rb +++ b/actioncable/lib/action_cable/process/logging.rb @@ -1,10 +1,7 @@ require 'action_cable/server' require 'eventmachine' -require 'celluloid' EM.error_handler do |e| puts "Error raised inside the event loop: #{e.message}" puts e.backtrace.join("\n") end - -Celluloid.logger = ActionCable.server.logger diff --git a/actioncable/lib/action_cable/server/base.rb b/actioncable/lib/action_cable/server/base.rb index 740e4b301e..cfd0a65f0f 100644 --- a/actioncable/lib/action_cable/server/base.rb +++ b/actioncable/lib/action_cable/server/base.rb @@ -1,6 +1,3 @@ -# FIXME: Cargo culted fix from https://github.com/celluloid/celluloid-pool/issues/10 -require 'celluloid/current' - require 'em-hiredis' module ActionCable diff --git a/actioncable/lib/action_cable/server/worker.rb b/actioncable/lib/action_cable/server/worker.rb index e063b2a2e1..6cddb4e7a5 100644 --- a/actioncable/lib/action_cable/server/worker.rb +++ b/actioncable/lib/action_cable/server/worker.rb @@ -1,39 +1,70 @@ -require 'celluloid' require 'active_support/callbacks' +require 'concurrent' module ActionCable module Server # Worker used by Server.send_async to do connection work in threads. Only for internal use. class Worker include ActiveSupport::Callbacks - include Celluloid - attr_reader :connection define_callbacks :work include ActiveRecordConnectionManagement - def invoke(receiver, method, *args) - @connection = receiver + def initialize(max_size=5) + @pool = Concurrent::ThreadPoolExecutor.new( + min_threads: 1, + max_threads: max_size, + max_queue: 0, + ) + end - run_callbacks :work do - receiver.send method, *args + def connection + Thread.current[:connection] || raise("No connection set") + end + + def async_invoke(receiver, method, *args) + @pool.post do + invoke(receiver, method, *args) end - rescue Exception => e - logger.error "There was an exception - #{e.class}(#{e.message})" - logger.error e.backtrace.join("\n") + end - receiver.handle_exception if receiver.respond_to?(:handle_exception) + def invoke(receiver, method, *args) + begin + Thread.current[:connection] = receiver + + run_callbacks :work do + receiver.send method, *args + end + rescue Exception => e + logger.error "There was an exception - #{e.class}(#{e.message})" + logger.error e.backtrace.join("\n") + + receiver.handle_exception if receiver.respond_to?(:handle_exception) + ensure + Thread.current[:connection] = nil + end + end + + def async_run_periodic_timer(channel, callback) + @pool.post do + run_periodic_timer(channel, callback) + end end def run_periodic_timer(channel, callback) - @connection = channel.connection + begin + Thread.current[:connection] = channel.connection - run_callbacks :work do - callback.respond_to?(:call) ? channel.instance_exec(&callback) : channel.send(callback) + run_callbacks :work do + callback.respond_to?(:call) ? channel.instance_exec(&callback) : channel.send(callback) + end + ensure + Thread.current[:connection] = nil end end private + def logger ActionCable.server.logger end diff --git a/actioncable/test/test_helper.rb b/actioncable/test/test_helper.rb index 12dcd98402..325305939f 100644 --- a/actioncable/test/test_helper.rb +++ b/actioncable/test/test_helper.rb @@ -14,11 +14,6 @@ require 'rack/mock' # Require all the stubs and models Dir[File.dirname(__FILE__) + '/stubs/*.rb'].each {|file| require file } -$CELLULOID_DEBUG = false -$CELLULOID_TEST = false -require 'celluloid' -Celluloid.logger = Logger.new(StringIO.new) - require 'faye/websocket' class << Faye::WebSocket remove_method :ensure_reactor_running diff --git a/actioncable/test/worker_test.rb b/actioncable/test/worker_test.rb index 69c4b6529d..9911a3b98b 100644 --- a/actioncable/test/worker_test.rb +++ b/actioncable/test/worker_test.rb @@ -17,8 +17,6 @@ class WorkerTest < ActiveSupport::TestCase end setup do - Celluloid.boot - @worker = ActionCable::Server::Worker.new @receiver = Receiver.new end