Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Replace some global Hash usages with the new thread safe cache.

Summary of the changes:
 * Add thread_safe gem.
 * Use thread safe cache for digestor caching.
 * Replace manual synchronization with ThreadSafe::Cache in Relation::Delegation.
 * Replace @attribute_method_matchers_cache Hash with ThreadSafe::Cache.
 * Use TS::Cache to avoid the synchronisation overhead on listener retrieval.
 * Replace synchronisation with TS::Cache usage.
 * Use a preallocated array for performance/memory reasons.
 * Update the controllers cache to the new AS::Dependencies::ClassCache API.
   The original @controllers cache no longer makes much sense after @tenderlove's
   changes in 7b6bfe8 and f345e23.
 * Use TS::Cache in the connection pool to avoid locking overhead.
 * Use TS::Cache in ConnectionHandler.
  • Loading branch information...
commit 45448a578877f6a753492113d72cc3512a6f1720 1 parent d668544
@thedarkone thedarkone authored
View
1  Gemfile
@@ -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'
View
10 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 Http
# 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 @@ def env_filter
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 = '[^&;=]+'
View
12 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 @@ class Dispatcher #:nodoc:
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 @@ def controller(params, default_controller=true)
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)
View
21 actionpack/lib/action_view/digestor.rb
@@ -1,4 +1,4 @@
-require 'mutex_m'
+require 'thread_safe'
module ActionView
class Digestor
@@ -21,23 +21,12 @@ class Digestor
/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 @@ def source
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
View
3  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 @@ class DetailsKey #:nodoc:
alias :object_hash :hash
attr_reader :hash
- @details_keys = Hash.new
+ @details_keys = ThreadSafe::Cache.new
def self.get(details)
@details_keys[details] ||= new
View
6 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
View
59 actionpack/lib/action_view/template/resolver.rb
@@ -3,7 +3,7 @@
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 @@ def to_str
# 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
View
9 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 @@ def instance_method_already_implemented?(method_name) #:nodoc:
# 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
View
25 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 @@ def initialize(spec)
@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 @@ def initialize(spec)
# 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 @@ def insert_connection_for_test!(c) #:nodoc:
# #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 @@ def connected?
# 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 @@ def disconnect!
# 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 @@ def checkout_and_verify(c)
# 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
View
38 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 @@ def method_missing(method, *args, &block)
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 @@ def new(klass, *args)
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)
View
1  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
View
3  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 @@ def remove_unloadable_constants!
class ClassCache
def initialize
- @store = Hash.new
+ @store = ThreadSafe::Cache.new
end
def empty?
View
6 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 Inflector
# 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
View
8 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 @@ def generate_key(salt, key_size=64)
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
View
7 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 @@ class Fanout
def initialize
@subscribers = []
- @listeners_for = {}
+ @listeners_for = ThreadSafe::Cache.new
super
end
@@ -44,7 +45,9 @@ def publish(name, *args)
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
Please sign in to comment.
Something went wrong with that request. Please try again.