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
This commit is contained in:
Samuel
2019-06-25 12:00:36 -04:00
parent b3b63a683f
commit fd5e3e87c7
9 changed files with 208 additions and 9 deletions

View File

@@ -18,6 +18,6 @@ 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

View File

@@ -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
#

View File

@@ -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

View File

@@ -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

View File

@@ -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 = ''

View File

@@ -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

View File

@@ -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%}')

View File

@@ -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

View File

@@ -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