From f89ef944be06a5b0c8946be93a88fa1b0fcb7a78 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Sun, 2 Feb 2020 12:09:59 +0530 Subject: [PATCH 1/3] Cache URLFilter results of string inputs per site --- lib/jekyll/filters/url_filters.rb | 26 ++++++++++++++++++-------- lib/jekyll/site.rb | 3 ++- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/jekyll/filters/url_filters.rb b/lib/jekyll/filters/url_filters.rb index 5d984096160..305e9fb65d6 100644 --- a/lib/jekyll/filters/url_filters.rb +++ b/lib/jekyll/filters/url_filters.rb @@ -9,8 +9,15 @@ module URLFilters # # Returns the absolute URL as a String. def absolute_url(input) - cache = (@context.registers[:cached_absolute_urls] ||= {}) - cache[input] ||= compute_absolute_url(input) + return if input.nil? + + if input.is_a?(String) + cache = (@context.registers[:site].filter_cache[:absolute_url] ||= {}) + cache[input.freeze] ||= compute_absolute_url(input).freeze + else + cache = (@context.registers[:cached_absolute_url] ||= {}) + cache[input] ||= compute_absolute_url(input) + end end # Produces a URL relative to the domain root based on site.baseurl @@ -20,8 +27,15 @@ def absolute_url(input) # # Returns a URL relative to the domain root as a String. def relative_url(input) - cache = (@context.registers[:cached_relative_urls] ||= {}) - cache[input] ||= compute_relative_url(input) + return if input.nil? + + if input.is_a?(String) + cache = (@context.registers[:site].filter_cache[:relative_url] ||= {}) + cache[input.freeze] ||= compute_relative_url(input).freeze + else + cache = (@context.registers[:cached_relative_url] ||= {}) + cache[input] ||= compute_relative_url(input) + end end # Strips trailing `/index.html` from URLs to create pretty permalinks @@ -38,8 +52,6 @@ def strip_index(input) private def compute_absolute_url(input) - return if input.nil? - input = input.url if input.respond_to?(:url) return input if Addressable::URI.parse(input.to_s).absolute? @@ -53,8 +65,6 @@ def compute_absolute_url(input) end def compute_relative_url(input) - return if input.nil? - input = input.url if input.respond_to?(:url) return input if Addressable::URI.parse(input.to_s).absolute? diff --git a/lib/jekyll/site.rb b/lib/jekyll/site.rb index 8aca2d655a0..7a3d267d12e 100644 --- a/lib/jekyll/site.rb +++ b/lib/jekyll/site.rb @@ -10,7 +10,7 @@ class Site :gems, :plugin_manager, :theme attr_accessor :converters, :generators, :reader - attr_reader :regenerator, :liquid_renderer, :includes_load_paths + attr_reader :regenerator, :liquid_renderer, :includes_load_paths, :filter_cache # Public: Initialize a new Site. # @@ -23,6 +23,7 @@ def initialize(config) self.config = config @cache_dir = in_source_dir(config["cache_dir"]) + @filter_cache = {} @reader = Reader.new(self) @regenerator = Regenerator.new(self) From 56bcc62c1e7ec4252ff02c3b072395d0a0e7c508 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Wed, 12 Feb 2020 19:38:04 +0530 Subject: [PATCH 2/3] Refactor to ensure that results are mutable --- lib/jekyll/filters/url_filters.rb | 28 ++++++++++++++-------------- test/test_filters.rb | 1 + 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/jekyll/filters/url_filters.rb b/lib/jekyll/filters/url_filters.rb index 305e9fb65d6..2ba1792f1c1 100644 --- a/lib/jekyll/filters/url_filters.rb +++ b/lib/jekyll/filters/url_filters.rb @@ -11,13 +11,13 @@ module URLFilters def absolute_url(input) return if input.nil? - if input.is_a?(String) - cache = (@context.registers[:site].filter_cache[:absolute_url] ||= {}) - cache[input.freeze] ||= compute_absolute_url(input).freeze - else - cache = (@context.registers[:cached_absolute_url] ||= {}) - cache[input] ||= compute_absolute_url(input) - end + cache = if input.is_a?(String) + (@context.registers[:site].filter_cache[:absolute_url] ||= {}) + else + (@context.registers[:cached_absolute_url] ||= {}) + end + cache[input] ||= compute_absolute_url(input) + cache[input].dup end # Produces a URL relative to the domain root based on site.baseurl @@ -29,13 +29,13 @@ def absolute_url(input) def relative_url(input) return if input.nil? - if input.is_a?(String) - cache = (@context.registers[:site].filter_cache[:relative_url] ||= {}) - cache[input.freeze] ||= compute_relative_url(input).freeze - else - cache = (@context.registers[:cached_relative_url] ||= {}) - cache[input] ||= compute_relative_url(input) - end + cache = if input.is_a?(String) + (@context.registers[:site].filter_cache[:relative_url] ||= {}) + else + (@context.registers[:cached_relative_url] ||= {}) + end + cache[input] ||= compute_relative_url(input) + cache[input].dup end # Strips trailing `/index.html` from URLs to create pretty permalinks diff --git a/test/test_filters.rb b/test/test_filters.rb index c9358a610a1..1ea309f17ec 100644 --- a/test/test_filters.rb +++ b/test/test_filters.rb @@ -596,6 +596,7 @@ def select; end assert_equal "/front_matter.erb", page.url url = filter.relative_url(page.url) url << "foo" + assert_equal "/front_matter.erb", filter.relative_url(page.url) assert_equal "/front_matter.erb", page.url end From 28b09e69639a709dd5245876a460f8277e0a91c4 Mon Sep 17 00:00:00 2001 From: Ashwin Maroli Date: Wed, 12 Feb 2020 20:23:02 +0530 Subject: [PATCH 3/3] Add clarifications for duplicating cached values --- lib/jekyll/filters/url_filters.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/jekyll/filters/url_filters.rb b/lib/jekyll/filters/url_filters.rb index 2ba1792f1c1..a7dd4dbb37b 100644 --- a/lib/jekyll/filters/url_filters.rb +++ b/lib/jekyll/filters/url_filters.rb @@ -17,6 +17,9 @@ def absolute_url(input) (@context.registers[:cached_absolute_url] ||= {}) end cache[input] ||= compute_absolute_url(input) + + # Duplicate cached string so that the cached value is never mutated by + # a subsequent filter. cache[input].dup end @@ -35,6 +38,9 @@ def relative_url(input) (@context.registers[:cached_relative_url] ||= {}) end cache[input] ||= compute_relative_url(input) + + # Duplicate cached string so that the cached value is never mutated by + # a subsequent filter. cache[input].dup end