Permalink
Browse files

Revert "Merge pull request #6425 from pinetops/resolver_concurrency_fix"

This reverts commit 254c042, reversing
changes made to 513a052.
  • Loading branch information...
1 parent 254c042 commit 776ea1090fb9ba4353035d4983cf1926a67b3294 @josevalim josevalim committed May 21, 2012
Showing with 23 additions and 72 deletions.
  1. +19 −68 actionpack/lib/action_view/template/resolver.rb
  2. +4 −4 actionpack/test/template/lookup_context_test.rb
View
87 actionpack/lib/action_view/template/resolver.rb
@@ -2,7 +2,6 @@
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 @@ def initialize(name, prefix, partial, virtual)
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 @@ class << self
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 @@ def build_path(name, prefix, partial)
# 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
View
8 actionpack/test/template/lookup_context_test.rb
@@ -169,7 +169,7 @@ def teardown
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 @@ def teardown
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 @@ def setup
@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

0 comments on commit 776ea10

Please sign in to comment.