From 2af4ea129556bc62deaa67a834f72a7323016967 Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Sun, 12 May 2013 22:06:53 -0400 Subject: [PATCH 1/5] Support benchmarking templates with 'include' tag --- performance/theme_runner.rb | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/performance/theme_runner.rb b/performance/theme_runner.rb index 98406b3c..6982a829 100644 --- a/performance/theme_runner.rb +++ b/performance/theme_runner.rb @@ -14,6 +14,17 @@ require File.dirname(__FILE__) + '/shopify/liquid' require File.dirname(__FILE__) + '/shopify/database.rb' class ThemeRunner + class FileSystem + + def initialize(path) + @path = path + end + + # Called by Liquid to retrieve a template file + def read_template_file(template_path, context) + File.read(@path + '/' + template_path + '.liquid') + end + end # Load all templates into memory, do this now so that # we don't profile IO. @@ -47,7 +58,7 @@ class ThemeRunner # Compute page_tempalte outside of profiler run, uninteresting to profiler page_template = File.basename(template_name, File.extname(template_name)) - compile_and_render(liquid, layout, assigns, page_template) + compile_and_render(liquid, layout, assigns, page_template, template_name) end end @@ -74,7 +85,7 @@ class ThemeRunner html = nil RubyProf.resume - html = compile_and_render(liquid, layout, assigns, page_template) + html = compile_and_render(liquid, layout, assigns, page_template, template_file) RubyProf.pause @@ -88,10 +99,11 @@ class ThemeRunner RubyProf.stop end - def compile_and_render(template, layout, assigns, page_template) + def compile_and_render(template, layout, assigns, page_template, template_file) tmpl = Liquid::Template.new tmpl.assigns['page_title'] = 'Page title' tmpl.assigns['template'] = page_template + tmpl.registers[:file_system] = ThemeRunner::FileSystem.new(File.dirname(template_file)) content_for_layout = tmpl.parse(template).render(assigns) From 485340713acc55110cdabe30a3c1327117f1944b Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Mon, 13 May 2013 02:25:56 -0400 Subject: [PATCH 2/5] Add tests for caching partial includes --- test/liquid/tags/include_tag_test.rb | 29 +++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/test/liquid/tags/include_tag_test.rb b/test/liquid/tags/include_tag_test.rb index 101dd7bc..8bdb19c8 100644 --- a/test/liquid/tags/include_tag_test.rb +++ b/test/liquid/tags/include_tag_test.rb @@ -39,6 +39,15 @@ class OtherFileSystem end end +class CountingFileSystem + attr_reader :count + def read_template_file(template_path, context) + @count ||= 0 + @count += 1 + 'from CountingFileSystem' + end +end + class IncludeTagTest < Test::Unit::TestCase include Liquid @@ -136,4 +145,22 @@ class IncludeTagTest < Test::Unit::TestCase assert_equal "Product: Draft 151cm ", Template.parse("{% include template for product %}").render("template" => 'product', 'product' => { 'title' => 'Draft 151cm'}) end -end # IncludeTagTest \ No newline at end of file + + def test_include_tag_caches_second_read_of_same_partial + file_system = CountingFileSystem.new + assert_equal 'from CountingFileSystemfrom CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}{% include 'pick_a_source' %}").render({}, :registers => {:file_system => file_system}) + assert_equal 1, file_system.count + end + + def test_include_tag_doesnt_cache_partials_across_renders + file_system = CountingFileSystem.new + assert_equal 'from CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}").render({}, :registers => {:file_system => file_system}) + assert_equal 1, file_system.count + + assert_equal 'from CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}").render({}, :registers => {:file_system => file_system}) + assert_equal 2, file_system.count + end +end # IncludeTagTest From 1e309ba74bdd2f3d3d8e05ca2759d97b3142edab Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Sun, 12 May 2013 21:06:41 -0400 Subject: [PATCH 3/5] cache included partial templates --- lib/liquid/tags/include.rb | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index f7400abd..dd0ea9bf 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -2,10 +2,10 @@ module Liquid class Include < Tag Syntax = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?/o - def initialize(tag_name, markup, tokens) + def initialize(tag_name, markup, tokens) if markup =~ Syntax - @template_name = $1 + @template_name = $1 @variable_name = $3 @attributes = {} @@ -24,8 +24,7 @@ module Liquid end def render(context) - source = _read_template_from_file_system(context) - partial = Liquid::Template.parse(source) + partial = _load_cached_partial(context) variable = context[@variable_name || @template_name[1..-2]] context.stack do @@ -46,6 +45,20 @@ module Liquid end private + def _load_cached_partial(context) + cached_partials = context.registers[:cached_partials] || {} + template_name = context[@template_name] + + if cached = cached_partials[template_name] + return cached + end + source = _read_template_from_file_system(context) + partial = Liquid::Template.parse(source) + cached_partials[template_name] = partial + context.registers[:cached_partials] = cached_partials + partial + end + def _read_template_from_file_system(context) file_system = context.registers[:file_system] || Liquid::Template.file_system From ba5a9f2e47a77e6d7386e7397dc073e18303ef7a Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Mon, 13 May 2013 13:45:43 -0400 Subject: [PATCH 4/5] remove _ on private methods --- lib/liquid/tags/include.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/liquid/tags/include.rb b/lib/liquid/tags/include.rb index dd0ea9bf..b109bf3e 100644 --- a/lib/liquid/tags/include.rb +++ b/lib/liquid/tags/include.rb @@ -24,7 +24,7 @@ module Liquid end def render(context) - partial = _load_cached_partial(context) + partial = load_cached_partial(context) variable = context[@variable_name || @template_name[1..-2]] context.stack do @@ -45,21 +45,21 @@ module Liquid end private - def _load_cached_partial(context) + def load_cached_partial(context) cached_partials = context.registers[:cached_partials] || {} template_name = context[@template_name] if cached = cached_partials[template_name] return cached end - source = _read_template_from_file_system(context) + source = read_template_from_file_system(context) partial = Liquid::Template.parse(source) cached_partials[template_name] = partial context.registers[:cached_partials] = cached_partials partial end - def _read_template_from_file_system(context) + def read_template_from_file_system(context) file_system = context.registers[:file_system] || Liquid::Template.file_system # make read_template_file call backwards-compatible. From b8fbd2b4fa2cdde8c90d949ad9f1b12e13e6ab5c Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Thu, 16 May 2013 20:25:31 -0400 Subject: [PATCH 5/5] typo --- performance/theme_runner.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/performance/theme_runner.rb b/performance/theme_runner.rb index 6982a829..802411a9 100644 --- a/performance/theme_runner.rb +++ b/performance/theme_runner.rb @@ -85,7 +85,7 @@ class ThemeRunner html = nil RubyProf.resume - html = compile_and_render(liquid, layout, assigns, page_template, template_file) + html = compile_and_render(liquid, layout, assigns, page_template, template_name) RubyProf.pause