diff options
| -rw-r--r-- | Gemfile | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/http/filter_parameters.rb | 10 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 12 | ||||
| -rw-r--r-- | actionpack/lib/action_view/digestor.rb | 21 | ||||
| -rw-r--r-- | actionpack/lib/action_view/lookup_context.rb | 3 | ||||
| -rw-r--r-- | actionpack/lib/action_view/renderer/partial_renderer.rb | 6 | ||||
| -rw-r--r-- | actionpack/lib/action_view/template/resolver.rb | 59 | ||||
| -rw-r--r-- | activemodel/lib/active_model/attribute_methods.rb | 9 | ||||
| -rw-r--r-- | activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb | 25 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/delegation.rb | 38 | ||||
| -rw-r--r-- | activesupport/activesupport.gemspec | 1 | ||||
| -rw-r--r-- | activesupport/lib/active_support/dependencies.rb | 3 | ||||
| -rw-r--r-- | activesupport/lib/active_support/inflector/inflections.rb | 6 | ||||
| -rw-r--r-- | activesupport/lib/active_support/key_generator.rb | 8 | ||||
| -rw-r--r-- | activesupport/lib/active_support/notifications/fanout.rb | 7 |
15 files changed, 101 insertions, 108 deletions
@@ -12,6 +12,7 @@ gem 'jquery-rails', '~> 2.1.4', github: 'rails/jquery-rails' gem 'turbolinks' gem 'coffee-rails', github: 'rails/coffee-rails' +gem 'thread_safe', '~> 0.1' gem 'journey', github: 'rails/journey', branch: 'master' gem 'activerecord-deprecated_finders', github: 'rails/activerecord-deprecated_finders', branch: 'master' diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 4a7df6b657..4fea690862 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -1,4 +1,4 @@ -require 'mutex_m' +require 'thread_safe' require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/object/duplicable' @@ -21,7 +21,7 @@ module ActionDispatch # end # => reverses the value to all keys matching /secret/i module FilterParameters - @@parameter_filter_for = {}.extend(Mutex_m) + @@parameter_filter_for = ThreadSafe::Cache.new ENV_MATCH = [/RAW_POST_DATA/, "rack.request.form_vars"] # :nodoc: NULL_PARAM_FILTER = ParameterFilter.new # :nodoc: @@ -65,11 +65,7 @@ module ActionDispatch end def parameter_filter_for(filters) - @@parameter_filter_for.synchronize do - # Do we *actually* need this cache? Constructing ParameterFilters - # doesn't seem too expensive. - @@parameter_filter_for[filters] ||= ParameterFilter.new(filters) - end + @@parameter_filter_for[filters] ||= ParameterFilter.new(filters) end KV_RE = '[^&;=]+' diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index c11e66d110..eb9d4b24f1 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -1,5 +1,6 @@ require 'journey' require 'forwardable' +require 'thread_safe' require 'active_support/core_ext/object/to_query' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/module/remove_method' @@ -20,7 +21,7 @@ module ActionDispatch def initialize(options={}) @defaults = options[:defaults] @glob_param = options.delete(:glob) - @controllers = {} + @controller_class_names = ThreadSafe::Cache.new end def call(env) @@ -68,13 +69,8 @@ module ActionDispatch private def controller_reference(controller_param) - controller_name = "#{controller_param.camelize}Controller" - - unless controller = @controllers[controller_param] - controller = @controllers[controller_param] = - ActiveSupport::Dependencies.reference(controller_name) - end - controller.get(controller_name) + const_name = @controller_class_names[controller_param] ||= "#{controller_param.camelize}Controller" + ActiveSupport::Dependencies.constantize(const_name) end def dispatch(controller, action, env) diff --git a/actionpack/lib/action_view/digestor.rb b/actionpack/lib/action_view/digestor.rb index 1c6eaf36f7..8bc69b9246 100644 --- a/actionpack/lib/action_view/digestor.rb +++ b/actionpack/lib/action_view/digestor.rb @@ -1,4 +1,4 @@ -require 'mutex_m' +require 'thread_safe' module ActionView class Digestor @@ -21,23 +21,12 @@ module ActionView /x cattr_reader(:cache) - @@cache = Hash.new.extend Mutex_m + @@cache = ThreadSafe::Cache.new def self.digest(name, format, finder, options = {}) - cache.synchronize do - unsafe_digest name, format, finder, options - end - end - - ### - # This method is NOT thread safe. DO NOT CALL IT DIRECTLY, instead call - # Digestor.digest - def self.unsafe_digest(name, format, finder, options = {}) # :nodoc: - key = "#{name}.#{format}" - - cache.fetch(key) do + @@cache["#{name}.#{format}"] ||= begin klass = options[:partial] || name.include?("/_") ? PartialDigestor : Digestor - cache[key] = klass.new(name, format, finder).digest + klass.new(name, format, finder).digest end end @@ -93,7 +82,7 @@ module ActionView def dependency_digest dependencies.collect do |template_name| - Digestor.unsafe_digest(template_name, format, finder, partial: true) + Digestor.digest(template_name, format, finder, partial: true) end.join("-") end diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 76f4dea7b8..4e4816d983 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -1,3 +1,4 @@ +require 'thread_safe' require 'active_support/core_ext/module/remove_method' module ActionView @@ -51,7 +52,7 @@ module ActionView alias :object_hash :hash attr_reader :hash - @details_keys = Hash.new + @details_keys = ThreadSafe::Cache.new def self.get(details) @details_keys[details] ||= new diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index 8fb9b6ff18..37f93a13fc 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -1,3 +1,5 @@ +require 'thread_safe' + module ActionView # = Action View Partials # @@ -247,7 +249,9 @@ module ActionView # <%- end -%> # <% end %> class PartialRenderer < AbstractRenderer - PREFIXED_PARTIAL_NAMES = Hash.new { |h,k| h[k] = {} } + PREFIXED_PARTIAL_NAMES = ThreadSafe::Cache.new do |h, k| + h[k] = ThreadSafe::Cache.new + end def initialize(*) super diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index fc77c1485d..8b23029bbc 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -3,7 +3,7 @@ require "active_support/core_ext/class" require "active_support/core_ext/class/attribute_accessors" require "action_view/template" require "thread" -require "mutex_m" +require "thread_safe" module ActionView # = Action View Resolver @@ -35,52 +35,51 @@ module ActionView # Threadsafe template cache class Cache #:nodoc: - class CacheEntry - include Mutex_m - - attr_accessor :templates + class SmallCache < ThreadSafe::Cache + def initialize(options = {}) + super(options.merge(:initial_capacity => 2)) + end end + # preallocate all the default blocks for performance/memory consumption reasons + PARTIAL_BLOCK = lambda {|cache, partial| cache[partial] = SmallCache.new} + PREFIX_BLOCK = lambda {|cache, prefix| cache[prefix] = SmallCache.new(&PARTIAL_BLOCK)} + NAME_BLOCK = lambda {|cache, name| cache[name] = SmallCache.new(&PREFIX_BLOCK)} + KEY_BLOCK = lambda {|cache, key| cache[key] = SmallCache.new(&NAME_BLOCK)} + + # usually a majority of template look ups return nothing, use this canonical preallocated array to safe memory + NO_TEMPLATES = [].freeze + def initialize - @data = Hash.new { |h1,k1| h1[k1] = Hash.new { |h2,k2| - h2[k2] = Hash.new { |h3,k3| h3[k3] = Hash.new { |h4,k4| h4[k4] = {} } } } } - @mutex = Mutex.new + @data = SmallCache.new(&KEY_BLOCK) end # Cache the templates returned by the block def cache(key, name, prefix, partial, locals) - cache_entry = nil - - # first obtain a lock on the main data structure to create the cache entry - @mutex.synchronize do - cache_entry = @data[key][name][prefix][partial][locals] ||= CacheEntry.new - end - - # then to avoid a long lasting global lock, obtain a more granular lock - # on the CacheEntry itself - cache_entry.synchronize do - if Resolver.caching? - cache_entry.templates ||= yield + if Resolver.caching? + @data[key][name][prefix][partial][locals] ||= canonical_no_templates(yield) + else + fresh_templates = yield + cached_templates = @data[key][name][prefix][partial][locals] + + if templates_have_changed?(cached_templates, fresh_templates) + @data[key][name][prefix][partial][locals] = canonical_no_templates(fresh_templates) else - fresh_templates = yield - - if templates_have_changed?(cache_entry.templates, fresh_templates) - cache_entry.templates = fresh_templates - else - cache_entry.templates ||= [] - end + cached_templates || NO_TEMPLATES end end end def clear - @mutex.synchronize do - @data.clear - end + @data.clear end private + def canonical_no_templates(templates) + templates.empty? ? NO_TEMPLATES : templates + end + def templates_have_changed?(cached_templates, fresh_templates) # if either the old or new template list is empty, we don't need to (and can't) # compare modification times, and instead just check whether the lists are different diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index af11da1351..db5759ada9 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -1,3 +1,4 @@ +require 'thread_safe' module ActiveModel # Raised when an attribute is not defined. @@ -337,17 +338,17 @@ module ActiveModel # significantly (in our case our test suite finishes 10% faster with # this cache). def attribute_method_matchers_cache #:nodoc: - @attribute_method_matchers_cache ||= {} + @attribute_method_matchers_cache ||= ThreadSafe::Cache.new(:initial_capacity => 4) end def attribute_method_matcher(method_name) #:nodoc: - attribute_method_matchers_cache.fetch(method_name) do |name| + attribute_method_matchers_cache.compute_if_absent(method_name) do # Must try to match prefixes/suffixes first, or else the matcher with no prefix/suffix # will match every time. matchers = attribute_method_matchers.partition(&:plain?).reverse.flatten(1) match = nil - matchers.detect { |method| match = method.match(name) } - attribute_method_matchers_cache[name] = match + matchers.detect { |method| match = method.match(method_name) } + match end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index b5a8011ca4..82d0cf7e2e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -1,4 +1,5 @@ require 'thread' +require 'thread_safe' require 'monitor' require 'set' require 'active_support/deprecation' @@ -236,9 +237,6 @@ module ActiveRecord @spec = spec - # The cache of reserved connections mapped to threads - @reserved_connections = {} - @checkout_timeout = spec.config[:checkout_timeout] || 5 @dead_connection_timeout = spec.config[:dead_connection_timeout] @reaper = Reaper.new self, spec.config[:reaping_frequency] @@ -247,6 +245,9 @@ module ActiveRecord # default max pool size to 5 @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5 + # The cache of reserved connections mapped to threads + @reserved_connections = ThreadSafe::Cache.new(:initial_capacity => @size) + @connections = [] @automatic_reconnect = true @@ -267,7 +268,9 @@ module ActiveRecord # #connection can be called any number of times; the connection is # held in a hash keyed by the thread id. def connection - synchronize do + # this is correctly done double-checked locking + # (ThreadSafe::Cache's lookups have volatile semantics) + @reserved_connections[current_connection_id] || synchronize do @reserved_connections[current_connection_id] ||= checkout end end @@ -310,7 +313,7 @@ module ActiveRecord # Disconnects all connections in the pool, and clears the pool. def disconnect! synchronize do - @reserved_connections = {} + @reserved_connections.clear @connections.each do |conn| checkin conn conn.disconnect! @@ -323,7 +326,7 @@ module ActiveRecord # Clears the cache which maps classes. def clear_reloadable_connections! synchronize do - @reserved_connections = {} + @reserved_connections.clear @connections.each do |conn| checkin conn conn.disconnect! if conn.requires_reloading? @@ -490,11 +493,15 @@ module ActiveRecord # determine the connection pool that they should use. class ConnectionHandler def initialize - # These hashes are keyed by klass.name, NOT klass. Keying them by klass + # These caches are keyed by klass.name, NOT klass. Keying them by klass # alone would lead to memory leaks in development mode as all previous # instances of the class would stay in memory. - @owner_to_pool = Hash.new { |h,k| h[k] = {} } - @class_to_pool = Hash.new { |h,k| h[k] = {} } + @owner_to_pool = ThreadSafe::Cache.new(:initial_capacity => 2) do |h,k| + h[k] = ThreadSafe::Cache.new(:initial_capacity => 2) + end + @class_to_pool = ThreadSafe::Cache.new(:initial_capacity => 2) do |h,k| + h[k] = ThreadSafe::Cache.new + end end def connection_pool_list diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index 2184625e22..431d083f21 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,5 +1,6 @@ require 'active_support/concern' -require 'mutex_m' +require 'thread' +require 'thread_safe' module ActiveRecord module Delegation # :nodoc: @@ -73,8 +74,7 @@ module ActiveRecord end module ClassMethods - # This hash is keyed by klass.name to avoid memory leaks in development mode - @@subclasses = Hash.new { |h, k| h[k] = {} }.extend(Mutex_m) + @@subclasses = ThreadSafe::Cache.new(:initial_capacity => 2) def new(klass, *args) relation = relation_class_for(klass).allocate @@ -82,33 +82,27 @@ module ActiveRecord relation end + # This doesn't have to be thread-safe. relation_class_for guarantees that this will only be + # called exactly once for a given const name. + def const_missing(name) + const_set(name, Class.new(self) { include ClassSpecificRelation }) + end + + private # Cache the constants in @@subclasses because looking them up via const_get # make instantiation significantly slower. def relation_class_for(klass) - if klass && klass.name - if subclass = @@subclasses.synchronize { @@subclasses[self][klass.name] } - subclass - else - subclass = const_get("#{name.gsub('::', '_')}_#{klass.name.gsub('::', '_')}", false) - @@subclasses.synchronize { @@subclasses[self][klass.name] = subclass } - subclass + if klass && (klass_name = klass.name) + my_cache = @@subclasses.compute_if_absent(self) { ThreadSafe::Cache.new } + # This hash is keyed by klass.name to avoid memory leaks in development mode + my_cache.compute_if_absent(klass_name) do + # Cache#compute_if_absent guarantees that the block will only executed once for the given klass_name + const_get("#{name.gsub('::', '_')}_#{klass_name.gsub('::', '_')}", false) end else ActiveRecord::Relation end end - - # Check const_defined? in case another thread has already defined the constant. - # I am not sure whether this is strictly necessary. - def const_missing(name) - @@subclasses.synchronize { - if const_defined?(name) - const_get(name) - else - const_set(name, Class.new(self) { include ClassSpecificRelation }) - end - } - end end def respond_to?(method, include_private = false) diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index a4216d2cb4..4c9e59dbd2 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -24,4 +24,5 @@ Gem::Specification.new do |s| s.add_dependency 'multi_json', '~> 1.3' s.add_dependency 'tzinfo', '~> 0.3.33' s.add_dependency 'minitest', '~> 4.1' + s.add_dependency 'thread_safe','~> 0.1' end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index efd351d741..fff4c776a9 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -1,5 +1,6 @@ require 'set' require 'thread' +require 'thread_safe' require 'pathname' require 'active_support/core_ext/module/aliasing' require 'active_support/core_ext/module/attribute_accessors' @@ -517,7 +518,7 @@ module ActiveSupport #:nodoc: class ClassCache def initialize - @store = Hash.new + @store = ThreadSafe::Cache.new end def empty? diff --git a/activesupport/lib/active_support/inflector/inflections.rb b/activesupport/lib/active_support/inflector/inflections.rb index 6f259a093b..9cf4b2b2ba 100644 --- a/activesupport/lib/active_support/inflector/inflections.rb +++ b/activesupport/lib/active_support/inflector/inflections.rb @@ -1,3 +1,4 @@ +require 'thread_safe' require 'active_support/core_ext/array/prepend_and_append' require 'active_support/i18n' @@ -24,9 +25,10 @@ module ActiveSupport # singularization rules that is runs. This guarantees that your rules run # before any of the rules that may already have been loaded. class Inflections + @__instance__ = ThreadSafe::Cache.new + def self.instance(locale = :en) - @__instance__ ||= Hash.new { |h, k| h[k] = new } - @__instance__[locale] + @__instance__[locale] ||= new end attr_reader :plurals, :singulars, :uncountables, :humans, :acronyms, :acronym_regex diff --git a/activesupport/lib/active_support/key_generator.rb b/activesupport/lib/active_support/key_generator.rb index 6beb2b6afa..71654dbb87 100644 --- a/activesupport/lib/active_support/key_generator.rb +++ b/activesupport/lib/active_support/key_generator.rb @@ -1,4 +1,4 @@ -require 'mutex_m' +require 'thread_safe' require 'openssl' module ActiveSupport @@ -28,16 +28,14 @@ module ActiveSupport class CachingKeyGenerator def initialize(key_generator) @key_generator = key_generator - @cache_keys = {}.extend(Mutex_m) + @cache_keys = ThreadSafe::Cache.new end # Returns a derived key suitable for use. The default key_size is chosen # to be compatible with the default settings of ActiveSupport::MessageVerifier. # i.e. OpenSSL::Digest::SHA1#block_length def generate_key(salt, key_size=64) - @cache_keys.synchronize do - @cache_keys["#{salt}#{key_size}"] ||= @key_generator.generate_key(salt, key_size) - end + @cache_keys["#{salt}#{key_size}"] ||= @key_generator.generate_key(salt, key_size) end end diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index 2e5bcf4639..7588fdb67c 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -1,4 +1,5 @@ require 'mutex_m' +require 'thread_safe' module ActiveSupport module Notifications @@ -11,7 +12,7 @@ module ActiveSupport def initialize @subscribers = [] - @listeners_for = {} + @listeners_for = ThreadSafe::Cache.new super end @@ -44,7 +45,9 @@ module ActiveSupport end def listeners_for(name) - synchronize do + # this is correctly done double-checked locking (ThreadSafe::Cache's lookups have volatile semantics) + @listeners_for[name] || synchronize do + # use synchronisation when accessing @subscribers @listeners_for[name] ||= @subscribers.select { |s| s.subscribed_to?(name) } end end |