WIP: Add column number

Resource counting perf

.travis.yml

@@ -1,13 +1,16 @@
+language: ruby
+
 rvm:
-  - 1.9
   - 2.0
   - 2.1
-  - jruby-19mode
+  - ruby-head
   - jruby-head
   - rbx-2
+
+sudo: false
+
 matrix:
   allow_failures:
-    - rvm: rbx-2
     - rvm: jruby-head
 
 script: "rake test"

lib/liquid.rb

@@ -63,6 +63,7 @@ require 'liquid/variable'
 require 'liquid/variable_lookup'
 require 'liquid/range_lookup'
 require 'liquid/file_system'
+require 'liquid/resource_limits'
 require 'liquid/template'
 require 'liquid/standardfilters'
 require 'liquid/condition'
@@ -73,6 +74,3 @@ require 'liquid/token'
 # Load all the tags of the standard library
 #
 Dir[File.dirname(__FILE__) + '/liquid/tags/*.rb'].each { |f| require f }
-
-require 'liquid/profiler'
-require 'liquid/profiler/hooks'

lib/liquid/block_body.rb

@@ -62,15 +62,14 @@ module Liquid
     def warnings
       all_warnings = []
       nodelist.each do |node|
-        all_warnings.concat(node.warnings) if node.respond_to?(:warnings) && node.warnings
+        all_warnings.concat(node.warnings || []) if node.respond_to?(:warnings)
       end
       all_warnings
     end
 
     def render(context)
       output = []
-      context.resource_limits[:render_length_current] = 0
+      context.resource_limits.render_score += @nodelist.length
-      context.resource_limits[:render_score_current] += @nodelist.length
 
       @nodelist.each do |token|
         # Break out if we have any unhanded interrupts.
@@ -104,12 +103,13 @@ module Liquid
 
     def render_token(token, context)
       token_output = (token.respond_to?(:render) ? token.render(context) : token)
-      context.increment_used_resources(:render_length_current, token_output)
+      token_str = token_output.is_a?(Array) ? token_output.join : token_output.to_s
-      if context.resource_limits_reached?
+
-        context.resource_limits[:reached] = true
+      context.resource_limits.render_length += token_str.length
+      if context.resource_limits.reached?
         raise MemoryError.new("Memory limits exceeded".freeze)
       end
-      token_output
+      token_str
     end
 
     def create_variable(token, options)

lib/liquid/context.rb

@@ -21,9 +21,7 @@ module Liquid
       @scopes           = [(outer_scope || {})]
       @registers        = registers
       @errors           = []
-      @resource_limits  = resource_limits || Template.default_resource_limits.dup
+      @resource_limits  = resource_limits || ResourceLimits.new(Template.default_resource_limits)
-      @resource_limits[:render_score_current] = 0
-      @resource_limits[:assign_score_current] = 0
       squash_instance_assigns_with_environments
 
       @this_stack_used = false
@@ -36,20 +34,6 @@ module Liquid
       @filters = []
     end
 
-    def increment_used_resources(key, obj)
-      @resource_limits[key] += if obj.kind_of?(String) || obj.kind_of?(Array) || obj.kind_of?(Hash)
-        obj.length
-      else
-        1
-      end
-    end
-
-    def resource_limits_reached?
-      (@resource_limits[:render_length_limit] && @resource_limits[:render_length_current] > @resource_limits[:render_length_limit]) ||
-      (@resource_limits[:render_score_limit]  && @resource_limits[:render_score_current]  > @resource_limits[:render_score_limit] ) ||
-      (@resource_limits[:assign_score_limit]  && @resource_limits[:assign_score_current]  > @resource_limits[:assign_score_limit] )
-    end
-
     def strainer
       @strainer ||= Strainer.create(self, @filters)
     end

lib/liquid/profiler.rb

@@ -1,9 +1,12 @@
+require 'liquid/profiler/hooks'
+
 module Liquid
 
   # Profiler enables support for profiling template rendering to help track down performance issues.
   #
-  # To enable profiling, pass the <tt>profile: true</tt> option to <tt>Liquid::Template.parse</tt>. Then, after
+  # To enable profiling, first require 'liquid/profiler'.
-  # <tt>Liquid::Template#render</tt> is called, the template object makes available an instance of this
+  # Then, to profile a parse/render cycle, pass the <tt>profile: true</tt> option to <tt>Liquid::Template.parse</tt>.
+  # After <tt>Liquid::Template#render</tt> is called, the template object makes available an instance of this
   # class via the <tt>Liquid::Template#profiler</tt> method.
   #
   #   template = Liquid::Template.parse(template_content, profile: true)

lib/liquid/resource_limits.rb

@@ -0,0 +1,23 @@
+module Liquid
+  class ResourceLimits
+    attr_accessor :render_length, :render_score, :assign_score,
+                  :render_length_limit, :render_score_limit, :assign_score_limit
+
+    def initialize(limits)
+      @render_length_limit = limits[:render_length_limit]
+      @render_score_limit = limits[:render_score_limit]
+      @assign_score_limit = limits[:assign_score_limit]
+      reset
+    end
+
+    def reached?
+      (@render_length_limit && @render_length > @render_length_limit) ||
+      (@render_score_limit  && @render_score  > @render_score_limit ) ||
+      (@assign_score_limit  && @assign_score  > @assign_score_limit )
+    end
+
+    def reset
+      @render_length = @render_score = @assign_score = 0
+    end
+  end
+end

lib/liquid/tags/assign.rb

@@ -25,7 +25,10 @@ module Liquid
     def render(context)
       val = @from.render(context)
       context.scopes.last[@to] = val
-      context.increment_used_resources(:assign_score_current, val)
+
+      inc = val.instance_of?(String) || val.instance_of?(Array) || val.instance_of?(Hash) ? val.length : 1
+      context.resource_limits.assign_score += inc
+
       ''.freeze
     end
 

lib/liquid/tags/capture.rb

@@ -25,7 +25,7 @@ module Liquid
     def render(context)
       output = super
       context.scopes.last[@to] = output
-      context.increment_used_resources(:assign_score_current, output)
+      context.resource_limits.assign_score += output.length
       ''.freeze
     end
 

lib/liquid/tags/unless.rb

@@ -3,7 +3,7 @@ require File.dirname(__FILE__) + '/if'
 module Liquid
   # Unless is a conditional just like 'if' but works on the inverse logic.
   #
-  #   {% unless x < 0 %} x is greater than zero {% end %}
+  #   {% unless x < 0 %} x is greater than zero {% endunless %}
   #
   class Unless < If
     def render(context)

lib/liquid/template.rb

@@ -18,7 +18,9 @@ module Liquid
       :locale => I18n.new
     }
 
-    attr_accessor :root, :resource_limits
+    attr_accessor :root
+    attr_reader :resource_limits
+
     @@file_system = BlankFileSystem.new
 
     class TagRegistry
@@ -110,7 +112,7 @@ module Liquid
     end
 
     def initialize
-      @resource_limits = self.class.default_resource_limits.dup
+      @resource_limits = ResourceLimits.new(self.class.default_resource_limits)
     end
 
     # Parse source code.
@@ -203,6 +205,9 @@ module Liquid
         context.add_filters(args.pop)
       end
 
+      # Retrying a render resets resource usage
+      context.resource_limits.reset
+
       begin
         # render the nodelist.
         # for performance reasons we get an array back here. join will make a string out of it.
@@ -241,15 +246,24 @@ module Liquid
       return raw_tokens unless @line_numbers
 
       current_line = 1
+      current_column = 1
       raw_tokens.map do |token|
-        Token.new(token, current_line).tap do
+        Token.new(token, current_line, current_column).tap do
-          current_line += token.count("\n")
+          new_line_count = token.count("\n")
+          if new_line_count > 0
+            current_line += new_line_count
+            current_column = token.size - token.rindex("\n") + 1
+          else
+            current_column += token.size
+          end
         end
       end
     end
 
     def with_profiling
       if @profiling && !@options[:included]
+        raise "Profiler not loaded, require 'liquid/profiler' first" unless defined?(Liquid::Profiler)
+
         @profiler = Profiler.new
         @profiler.start
 

lib/liquid/token.rb

@@ -1,10 +1,11 @@
 module Liquid
   class Token < String
-    attr_reader :line_number
+    attr_reader :line_number, :column_number
 
-    def initialize(content, line_number)
+    def initialize(content, line_number, column_number=nil)
       super(content)
       @line_number = line_number
+      @column_number = column_number
     end
 
     def raw

test/integration/template_test.rb

@@ -1,4 +1,5 @@
 require 'test_helper'
+require 'timeout'
 
 class TemplateContextDrop < Liquid::Drop
   def before_method(method)
@@ -37,6 +38,16 @@ class TemplateTest < Minitest::Test
     assert_equal 'from instance assigns', t.parse("{{ foo }}").render!
   end
 
+  def test_warnings_is_not_exponential_time
+    str = "false"
+    100.times do
+      str = "{% if true %}true{% else %}#{str}{% endif %}"
+    end
+
+    t = Template.parse(str)
+    assert_equal [], Timeout::timeout(1) { t.warnings }
+  end
+
   def test_instance_assigns_persist_on_same_template_parsing_between_renders
     t = Template.new.parse("{{ foo }}{% assign foo = 'foo' %}{{ foo }}")
     assert_equal 'foo', t.render!
@@ -82,69 +93,92 @@ class TemplateTest < Minitest::Test
 
   def test_resource_limits_works_with_custom_length_method
     t = Template.parse("{% assign foo = bar %}")
-    t.resource_limits = { :render_length_limit => 42 }
+    t.resource_limits.render_length_limit = 42
     assert_equal "", t.render!("bar" => SomethingWithLength.new)
   end
 
   def test_resource_limits_render_length
     t = Template.parse("0123456789")
-    t.resource_limits = { :render_length_limit => 5 }
+    t.resource_limits.render_length_limit = 5
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
-    t.resource_limits = { :render_length_limit => 10 }
+
+    t.resource_limits.render_length_limit = 10
     assert_equal "0123456789", t.render!()
-    refute_nil t.resource_limits[:render_length_current]
+    refute_nil t.resource_limits.render_length
   end
 
   def test_resource_limits_render_score
     t = Template.parse("{% for a in (1..10) %} {% for a in (1..10) %} foo {% endfor %} {% endfor %}")
-    t.resource_limits = { :render_score_limit => 50 }
+    t.resource_limits.render_score_limit = 50
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
+
     t = Template.parse("{% for a in (1..100) %} foo {% endfor %}")
-    t.resource_limits = { :render_score_limit => 50 }
+    t.resource_limits.render_score_limit = 50
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
-    t.resource_limits = { :render_score_limit => 200 }
+
+    t.resource_limits.render_score_limit = 200
     assert_equal (" foo " * 100), t.render!()
-    refute_nil t.resource_limits[:render_score_current]
+    refute_nil t.resource_limits.render_score
   end
 
   def test_resource_limits_assign_score
     t = Template.parse("{% assign foo = 42 %}{% assign bar = 23 %}")
-    t.resource_limits = { :assign_score_limit => 1 }
+    t.resource_limits.assign_score_limit = 1
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
-    t.resource_limits = { :assign_score_limit => 2 }
+
+    t.resource_limits.assign_score_limit = 2
     assert_equal "", t.render!()
-    refute_nil t.resource_limits[:assign_score_current]
+    refute_nil t.resource_limits.assign_score
   end
 
   def test_resource_limits_aborts_rendering_after_first_error
     t = Template.parse("{% for a in (1..100) %} foo1 {% endfor %} bar {% for a in (1..100) %} foo2 {% endfor %}")
-    t.resource_limits = { :render_score_limit => 50 }
+    t.resource_limits.render_score_limit = 50
     assert_equal "Liquid error: Memory limits exceeded", t.render()
-    assert t.resource_limits[:reached]
+    assert t.resource_limits.reached?
   end
 
   def test_resource_limits_hash_in_template_gets_updated_even_if_no_limits_are_set
     t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")
     t.render!()
-    assert t.resource_limits[:assign_score_current] > 0
+    assert t.resource_limits.assign_score > 0
-    assert t.resource_limits[:render_score_current] > 0
+    assert t.resource_limits.render_score > 0
-    assert t.resource_limits[:render_length_current] > 0
+    assert t.resource_limits.render_length > 0
+  end
+
+  def test_render_length_persists_between_blocks
+    t = Template.parse("{% if true %}aaaa{% endif %}")
+    t.resource_limits.render_length_limit = 7
+    assert_equal "Liquid error: Memory limits exceeded", t.render()
+    t.resource_limits.render_length_limit = 8
+    assert_equal "aaaa", t.render()
+
+    t = Template.parse("{% if true %}aaaa{% endif %}{% if true %}bbb{% endif %}")
+    t.resource_limits.render_length_limit = 13
+    assert_equal "Liquid error: Memory limits exceeded", t.render()
+    t.resource_limits.render_length_limit = 14
+    assert_equal "aaaabbb", t.render()
+
+    t = Template.parse("{% if true %}a{% endif %}{% if true %}b{% endif %}{% if true %}a{% endif %}{% if true %}b{% endif %}{% if true %}a{% endif %}{% if true %}b{% endif %}")
+    t.resource_limits.render_length_limit = 5
+    assert_equal "Liquid error: Memory limits exceeded", t.render()
+    t.resource_limits.render_length_limit = 11
+    assert_equal "Liquid error: Memory limits exceeded", t.render()
+    t.resource_limits.render_length_limit = 12
+    assert_equal "ababab", t.render()
   end
 
   def test_default_resource_limits_unaffected_by_render_with_context
     context = Context.new
     t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")
     t.render!(context)
-    assert context.resource_limits[:assign_score_current] > 0
+    assert context.resource_limits.assign_score > 0
-    assert context.resource_limits[:render_score_current] > 0
+    assert context.resource_limits.render_score > 0
-    assert context.resource_limits[:render_length_current] > 0
+    assert context.resource_limits.render_length > 0
-    refute Template.default_resource_limits.key?(:assign_score_current)
-    refute Template.default_resource_limits.key?(:render_score_current)
-    refute Template.default_resource_limits.key?(:render_length_current)
   end
 
   def test_can_use_drop_as_context

test/test_helper.rb

@@ -5,6 +5,7 @@ require 'spy/integration'
 
 $:.unshift(File.join(File.expand_path(File.dirname(__FILE__)), '..', 'lib'))
 require 'liquid.rb'
+require 'liquid/profiler'
 
 mode = :strict
 if env_mode = ENV['LIQUID_PARSER_MODE']