diff options
author | José Valim <jose.valim@gmail.com> | 2012-05-21 21:01:02 +0200 |
---|---|---|
committer | José Valim <jose.valim@gmail.com> | 2012-05-21 21:01:02 +0200 |
commit | 776ea1090fb9ba4353035d4983cf1926a67b3294 (patch) | |
tree | a406f0a86b4199fcf21287e7726311ea7918f306 | |
parent | 254c04286c5916ae7f91eb6e173b312e7a74e364 (diff) | |
download | rails-776ea1090fb9ba4353035d4983cf1926a67b3294.tar.gz rails-776ea1090fb9ba4353035d4983cf1926a67b3294.tar.bz2 rails-776ea1090fb9ba4353035d4983cf1926a67b3294.zip |
Revert "Merge pull request #6425 from pinetops/resolver_concurrency_fix"
This reverts commit 254c04286c5916ae7f91eb6e173b312e7a74e364, reversing
changes made to 513a0525c24c2944630acfa465b22cd2f4601adf.
-rw-r--r-- | actionpack/lib/action_view/template/resolver.rb | 87 | ||||
-rw-r--r-- | actionpack/test/template/lookup_context_test.rb | 8 |
2 files changed, 23 insertions, 72 deletions
diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 8f87d6da8b..fa2038f78d 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -2,7 +2,6 @@ require "pathname" require "active_support/core_ext/class" require "active_support/core_ext/class/attribute_accessors" require "action_view/template" -require "thread" module ActionView # = Action View Resolver @@ -25,64 +24,6 @@ module ActionView end end - # Threadsafe template cache - class Cache #:nodoc: - class CacheEntry - attr_accessor :templates - - delegate :synchronize, :to => "@mutex" - - def initialize - @mutex = Mutex.new - end - end - - 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 - 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? - # all templates are cached forever the first time they are accessed - cache_entry.templates ||= yield - else - # templates are still cached, but are only returned if they are - # all still current - fresh = yield - - mtime = cache_entry.templates && cache_entry.templates.map(&:updated_at).max - - newer = !mtime || fresh.empty? || fresh.any? { |t| t.updated_at > mtime } - - if newer - cache_entry.templates = fresh - else - cache_entry.templates - end - end - end - end - - def clear - @mutex.synchronize do - @data.clear - end - end - end - cattr_accessor :caching self.caching = true @@ -91,11 +32,12 @@ module ActionView end def initialize - @cache = Cache.new + @cached = Hash.new { |h1,k1| h1[k1] = Hash.new { |h2,k2| + h2[k2] = Hash.new { |h3,k3| h3[k3] = Hash.new { |h4,k4| h4[k4] = {} } } } } end def clear_cache - @cache.clear + @cached.clear end # Normalizes the arguments and passes it on to find_template. @@ -123,18 +65,27 @@ module ActionView # Handles templates caching. If a key is given and caching is on # always check the cache before hitting the resolver. Otherwise, - # it always hits the resolver but if the key is present, check if the - # resolver is fresher before returning it. + # it always hits the resolver but check if the resolver is fresher + # before returning it. def cached(key, path_info, details, locals) #:nodoc: name, prefix, partial = path_info locals = locals.map { |x| x.to_s }.sort! - if key - @cache.cache(key, name, prefix, partial, locals) do - decorate(yield, path_info, details, locals) - end + if key && caching? + @cached[key][name][prefix][partial][locals] ||= decorate(yield, path_info, details, locals) else - decorate(yield, path_info, details, locals) + fresh = decorate(yield, path_info, details, locals) + return fresh unless key + + scope = @cached[key][name][prefix][partial] + cache = scope[locals] + mtime = cache && cache.map(&:updated_at).max + + if !mtime || fresh.empty? || fresh.any? { |t| t.updated_at > mtime } + scope[locals] = fresh + else + cache + end end end diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb index ef9c5ce10c..96b14a0acd 100644 --- a/actionpack/test/template/lookup_context_test.rb +++ b/actionpack/test/template/lookup_context_test.rb @@ -169,7 +169,7 @@ class LookupContextTest < ActiveSupport::TestCase assert_not_equal template, old_template end - + test "responds to #prefixes" do assert_equal [], @lookup_context.prefixes @lookup_context.prefixes = ["foo"] @@ -180,7 +180,7 @@ end class LookupContextWithFalseCaching < ActiveSupport::TestCase def setup @resolver = ActionView::FixtureResolver.new("test/_foo.erb" => ["Foo", Time.utc(2000)]) - ActionView::Resolver.stubs(:caching?).returns(false) + @resolver.stubs(:caching?).returns(false) @lookup_context = ActionView::LookupContext.new(@resolver, {}) end @@ -247,6 +247,6 @@ class TestMissingTemplate < ActiveSupport::TestCase @lookup_context.view_paths.find("foo", "parent", true, details) end assert_match %r{Missing partial parent/foo with .* Searched in:\n \* "/Path/to/views"\n}, e.message - end - + end + end |