Optimize range iteration

For loops support a range syntax for iterating a fixed number of times,
which looks like this.

```liquid
{% for i in range (1..100) %}
        ...
{% endfor %}
```

Previously, we converted these ranges to arrays using `to_a`, which
initialized an array containing each number in the range. Since all we
use these ranges for is iteration, this is far less efficient than
using a range iterator.

Doing this means that iterating over ranges now takes O(1) rather than O(n)
memory. See the PR for more benchmarks.

* Remove to_a cast on ranges
* Add ReversableRange iterator
* Add custom range-specific slicing logic

Gemfile

@@ -9,19 +9,15 @@ group :benchmark, :test do
   gem 'benchmark-ips'
   gem 'memory_profiler'
 
-  # install_if -> { RUBY_PLATFORM !~ /mingw|mswin|java/ } do
-  #   gem 'stackprof'
-  # end
+  install_if -> { RUBY_PLATFORM !~ /mingw|mswin|java/ } do
+    gem 'stackprof'
+  end
 end
 
 group :test do
   gem 'rubocop', '~> 0.53.0'
 
   platform :mri do
-    gem 'liquid-c', github: 'Shopify/liquid-c', ref: '9168659de45d6d576fce30c735f857e597fa26f6'
+    gem 'liquid-c', github: 'Shopify/liquid-c', ref: 'reversable-range'
   end
 end
-
-# gem "byebug", "~> 11.0"
-
-gem "pry", "~> 0.12.2"

Rakefile

@@ -100,9 +100,3 @@ end
 task :console do
   exec 'irb -I lib -r liquid'
 end
-
-task :dump_portable_ast, [:filename] do |_task, args|
-  require 'liquid/ast_to_portable_json'
-  ast = Liquid::Template.parse(File.read(args[:filename]))
-  puts(Liquid::ASTToPortableJSON.dump(ast))
-end

lib/liquid.rb

@@ -74,6 +74,7 @@ require 'liquid/condition'
 require 'liquid/utils'
 require 'liquid/tokenizer'
 require 'liquid/parse_context'
+require 'liquid/reversable_range'
 
 # Load all the tags of the standard library
 #

lib/liquid/ast_to_portable_json.rb

@@ -1,27 +0,0 @@
-require 'yaml'
-require 'json'
-require 'liquid'
-
-module Liquid
-  class ASTToPortableJSON
-    def self.dump(ast)
-      yaml = YAML.dump(ast)
-      yaml.gsub!(/---.*/, '')
-      yaml.gsub!(/!ruby\/object:(.+)\n(\s+)/, "\n\\2class_name: \\1\n\\2")
-
-      bare_hash_ast = YAML.load(yaml)
-      delete_parse_context(bare_hash_ast)
-      JSON.pretty_generate(bare_hash_ast)
-    end
-
-    def self.delete_parse_context(bare_hash_ast)
-      if bare_hash_ast.is_a?(Array)
-        bare_hash_ast.each { |v| delete_parse_context(v) }
-      elsif bare_hash_ast.is_a?(Hash)
-        bare_hash_ast.delete('parse_context')
-        bare_hash_ast.each { |k, v| delete_parse_context(v) }
-      end
-    end
-  end
-end
-

lib/liquid/range_lookup.rb

@@ -6,7 +6,7 @@ module Liquid
       if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate)
         new(start_obj, end_obj)
       else
-        start_obj.to_i..end_obj.to_i
+        ReversableRange.new(start_obj.to_i, end_obj.to_i)
       end
     end
 
@@ -18,7 +18,7 @@ module Liquid
     def evaluate(context)
       start_int = to_integer(context.evaluate(@start_obj))
       end_int = to_integer(context.evaluate(@end_obj))
-      start_int..end_int
+      ReversableRange.new(start_int, end_int)
     end
 
     private

lib/liquid/reversable_range.rb

@@ -0,0 +1,77 @@
+module Liquid
+  class ReversableRange
+    include Enumerable
+
+    def initialize(min, max)
+      @min = min
+      @max = max
+      @reversed = false
+    end
+
+    def each
+      if reversed
+        index = max
+        while index >= min
+          yield index
+          index -= 1
+        end
+      else
+        index = min
+        while index <= max
+          yield index
+          index += 1
+        end
+      end
+    end
+
+    def reverse!
+      @reversed = !reversed
+      self
+    end
+
+    def empty?
+      max < min
+    end
+
+    def size
+      difference = max - min
+      if difference > 0
+        difference + 1
+      else
+        0
+      end
+    end
+
+    def load_slice(from, to = nil)
+      to ||= max
+      slice_max = [max, to].min
+      slice_min = [min, from].max
+      range = ReversableRange.new(slice_min, slice_max)
+      range.reverse! if reversed
+      range
+    end
+
+    def ==(other)
+      other.is_a?(self.class) &&
+        other.min == min &&
+        other.max == max &&
+        other.reversed == reversed
+    end
+
+    def to_s
+      if reversed
+        "#{max}..#{min}"
+      else
+        "#{min}..#{max}"
+      end
+    end
+
+    def to_liquid
+      self
+    end
+
+    protected
+
+    attr_reader :min, :max, :reversed
+  end
+end

lib/liquid/tags/for.rb

@@ -133,7 +133,6 @@ module Liquid
       end
 
       collection = context.evaluate(@collection_name)
-      collection = collection.to_a if collection.is_a?(Range)
 
       limit_value = context.evaluate(@limit)
       to = if limit_value.nil?
@@ -145,14 +144,14 @@ module Liquid
       segment = Utils.slice_collection(collection, from, to)
       segment.reverse! if @reversed
 
-      offsets[@name] = from + segment.length
+      offsets[@name] = from + segment.size
 
       segment
     end
 
     def render_segment(context, segment)
       for_stack = context.registers[:for_stack] ||= []
-      length = segment.length
+      length = segment.size
 
       result = ''
 

lib/liquid/template.rb

@@ -1,4 +1,3 @@
-require 'liquid/ast_to_portable_json'
 module Liquid
   # Templates are central to liquid.
   # Interpretating templates is a two step process. First you compile the
@@ -131,8 +130,6 @@ module Liquid
       @line_numbers = options[:line_numbers] || @profiling
       parse_context = options.is_a?(ParseContext) ? options : ParseContext.new(options)
       @root = Document.parse(tokenize(source), parse_context)
-      @json = ASTToPortableJSON.dump(self)
-
       @warnings = parse_context.warnings
       self
     end
@@ -201,15 +198,6 @@ module Liquid
         context.add_filters(args.pop)
       end
 
-      unless @truffle_context
-        environment = context.environments.first
-        @truffle_context = Polyglot.eval('liquid', @json)
-        environment.each do |key, value|
-          @truffle_context[key] = value
-        end
-      end
-      return @truffle_context[:render].call
-
       # Retrying a render resets resource usage
       context.resource_limits.reset
 

lib/liquid/utils.rb

@@ -1,6 +1,8 @@
 module Liquid
   module Utils
     def self.slice_collection(collection, from, to)
+      return collection.load_slice(from, to) if collection.is_a?(ReversableRange)
+
       if (from != 0 || !to.nil?) && collection.respond_to?(:load_slice)
         collection.load_slice(from, to)
       else

performance/benchmark.rb

@@ -1,4 +1,3 @@
-require 'bundler/setup'
 require 'benchmark/ips'
 require_relative 'theme_runner'
 
@@ -6,15 +5,14 @@ Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first
 profiler = ThemeRunner.new
 
 Benchmark.ips do |x|
-  x.time = 60
-  x.warmup = 1
+  x.time = 10
+  x.warmup = 5
 
   puts
   puts "Running benchmark for #{x.time} seconds (with #{x.warmup} seconds warmup)."
   puts
 
-  profiler.compile
-  # x.report("parse:") { profiler.compile }
+  x.report("parse:") { profiler.compile }
   x.report("render:") { profiler.render }
   x.report("parse & render:") { profiler.run }
 end

performance/theme_runner.rb

@@ -24,7 +24,7 @@ class ThemeRunner
   # Initialize a new liquid ThemeRunner instance
   # Will load all templates into memory, do this now so that we don't profile IO.
   def initialize
-    @tests = Dir[__dir__ + '/tests/ripen/product.liquid'].collect do |test|
+    @tests = Dir[__dir__ + '/tests/**/*.liquid'].collect do |test|
       next if File.basename(test) == 'theme.liquid'
 
       theme_path = File.dirname(test) + '/theme.liquid'

test/integration/tags/for_tag_test.rb

@@ -36,6 +36,10 @@ HERE
     assert_template_result('321', '{%for item in array reversed %}{{item}}{%endfor%}', assigns)
   end
 
+  def test_for_range_reversed
+    assert_template_result('321', '{%for item in (1..3) reversed %}{{item}}{%endfor%}', {})
+  end
+
   def test_for_with_range
     assert_template_result(' 1  2  3 ', '{%for item in (1..3) %} {{item}} {%endfor%}')
 

test/unit/context_unit_test.rb

@@ -349,9 +349,9 @@ class ContextUnitTest < Minitest::Test
 
   def test_ranges
     @context.merge("test" => '5')
-    assert_equal (1..5), @context['(1..5)']
-    assert_equal (1..5), @context['(1..test)']
-    assert_equal (5..5), @context['(test..test)']
+    assert_equal ReversableRange.new(1, 5), @context['(1..5)']
+    assert_equal ReversableRange.new(1, 5), @context['(1..test)']
+    assert_equal ReversableRange.new(5, 5), @context['(test..test)']
   end
 
   def test_cents_through_drop_nestedly

test/unit/reversable_range_unit_test.rb

@@ -0,0 +1,116 @@
+require 'test_helper'
+
+class ReversableRangeTest < Minitest::Test
+  include Liquid
+
+  def test_each_iterates_through_items_in_the_range
+    actual_items = []
+    ReversableRange.new(1, 10).each { |item| actual_items << item }
+
+    expected_items = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_implements_enumerable
+    actual_items = ReversableRange.new(1, 10).select(&:even?)
+
+    expected_items = [2, 4, 6, 8, 10]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_is_not_empty_max_greater_than_min
+    range = ReversableRange.new(9, 10)
+
+    refute_predicate range, :empty?
+  end
+
+  def test_is_not_empty_max_equal_to_min
+    range = ReversableRange.new(10, 10)
+
+    refute_predicate range, :empty?
+  end
+
+  def test_is_empty_if_not_reversed_and_max_less_than_min
+    range = ReversableRange.new(10, 9)
+
+    assert_predicate range, :empty?
+  end
+
+  def test_ranges_with_the_same_max_and_min_have_one_element
+    actual_items = ReversableRange.new(1337, 1337).to_a
+    expected_items = [1337]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_load_slice_returns_a_sub_range
+    actual_items = ReversableRange.new(1, 10).load_slice(5, 8).to_a
+
+    expected_items = [5, 6, 7, 8]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_load_slice_returns_a_reversed_sub_range_if_reversed
+    range = ReversableRange.new(1, 10)
+    range.reverse!
+    actual_items = range.load_slice(5, 8).to_a
+
+    expected_items = [8, 7, 6, 5]
+    assert_equal expected_items, actual_items
+  end
+
+  def test_is_equal_to_other_if_also_a_reversable_range_and_has_same_properties
+    one = ReversableRange.new(1, 10)
+    one.reverse!
+
+    two = ReversableRange.new(1, 10)
+    two.reverse!
+
+    assert_equal one, two
+  end
+
+  def test_is_not_equal_to_a_non_reversable_range
+    range = ReversableRange.new(1, 10)
+    range.reverse!
+
+    refute_equal range, :something_else
+  end
+
+  def test_is_not_equal_if_ranges_have_different_mins
+    one = ReversableRange.new(1, 10)
+    two = ReversableRange.new(2, 10)
+
+    refute_equal one, two
+  end
+
+  def test_is_not_equal_if_ranges_have_different_maxes
+    one = ReversableRange.new(1, 10)
+    two = ReversableRange.new(1, 11)
+
+    refute_equal one, two
+  end
+
+  def test_is_not_equal_if_only_one_is_reversed
+    one = ReversableRange.new(1, 10)
+
+    two = ReversableRange.new(1, 10)
+    two.reverse!
+
+    refute_equal one, two
+  end
+
+  def test_to_s_mirrors_rubys_range_syntax
+    range = ReversableRange.new(1, 10)
+    assert_equal '1..10', range.to_s
+  end
+
+  def test_to_s_reverses_when_reversed
+    range = ReversableRange.new(1, 10)
+    range.reverse!
+    assert_equal '10..1', range.to_s
+  end
+
+  def test_size
+    range = ReversableRange.new(1, 10)
+    assert_equal 10, range.size
+  end
+end