Merge pull request #681 from Shopify/save-some-loop-allocations

Reuse 'forloop' hash to save memory allocations

lib/liquid.rb

@@ -48,6 +48,7 @@ require 'liquid/lexer'
 require 'liquid/parser'
 require 'liquid/i18n'
 require 'liquid/drop'
+require 'liquid/forloop_drop'
 require 'liquid/extensions'
 require 'liquid/errors'
 require 'liquid/interrupts'

lib/liquid/forloop_drop.rb

@@ -0,0 +1,42 @@
+module Liquid
+  class ForloopDrop < Drop
+    def initialize(name, length, parentloop)
+      @name = name
+      @length = length
+      @parentloop = parentloop
+      @index = 0
+    end
+
+    attr_reader :name, :length, :parentloop
+
+    def index
+      @index + 1
+    end
+
+    def index0
+      @index
+    end
+
+    def rindex
+      @length - @index
+    end
+
+    def rindex0
+      @length - @index - 1
+    end
+
+    def first
+      @index == 0
+    end
+
+    def last
+      @index == @length - 1
+    end
+
+    protected
+
+    def increment!
+      @index += 1
+    end
+  end
+end

lib/liquid/tags/for.rb

@@ -67,69 +67,13 @@ module Liquid
     end
 
     def render(context)
-      for_offsets = context.registers[:for] ||= Hash.new(0)
-      for_stack = context.registers[:for_stack] ||= []
+      segment = collection_segment(context)
 
-      parent_loop = for_stack.last
-      for_stack.push(nil)
-
-      collection = context.evaluate(@collection_name)
-      collection = collection.to_a if collection.is_a?(Range)
-
-      from = if @from == :continue
-        for_offsets[@name].to_i
+      if segment.empty?
+        render_else(context)
       else
-        context.evaluate(@from).to_i
+        render_segment(context, segment)
       end
-
-      limit = context.evaluate(@limit)
-      to    = limit ? limit.to_i + from : nil
-
-      segment = Utils.slice_collection(collection, from, to)
-
-      return render_else(context) if segment.empty?
-
-      segment.reverse! if @reversed
-
-      result = ''
-
-      length = segment.length
-
-      # Store our progress through the collection for the continue flag
-      for_offsets[@name] = from + segment.length
-
-      context.stack do
-        segment.each_with_index do |item, index|
-          context[@variable_name] = item
-          loop_vars = {
-            'name'.freeze       => @name,
-            'length'.freeze     => length,
-            'index'.freeze      => index + 1,
-            'index0'.freeze     => index,
-            'rindex'.freeze     => length - index,
-            'rindex0'.freeze    => length - index - 1,
-            'first'.freeze      => (index == 0),
-            'last'.freeze       => (index == length - 1),
-            'parentloop'.freeze => parent_loop
-          }
-
-          context['forloop'.freeze] = loop_vars
-          for_stack[-1] = loop_vars
-
-          result << @for_block.render(context)
-
-          # Handle any interrupts if they exist.
-          if context.interrupt?
-            interrupt = context.pop_interrupt
-            break if interrupt.is_a? BreakInterrupt
-            next if interrupt.is_a? ContinueInterrupt
-          end
-        end
-      end
-
-      result
-    ensure
-      for_stack.pop
     end
 
     protected
@@ -170,6 +114,63 @@ module Liquid
 
     private
 
+    def collection_segment(context)
+      offsets = context.registers[:for] ||= Hash.new(0)
+
+      from = if @from == :continue
+        offsets[@name].to_i
+      else
+        context.evaluate(@from).to_i
+      end
+
+      collection = context.evaluate(@collection_name)
+      collection = collection.to_a if collection.is_a?(Range)
+
+      limit = context.evaluate(@limit)
+      to = limit ? limit.to_i + from : nil
+
+      segment = Utils.slice_collection(collection, from, to)
+      segment.reverse! if @reversed
+
+      offsets[@name] = from + segment.length
+
+      segment
+    end
+
+    def render_segment(context, segment)
+      for_stack = context.registers[:for_stack] ||= []
+      length = segment.length
+
+      result = ''
+
+      context.stack do
+        loop_vars = Liquid::ForloopDrop.new(@name, length, for_stack[-1])
+
+        for_stack.push(loop_vars)
+
+        begin
+          context['forloop'.freeze] = loop_vars
+
+          segment.each_with_index do |item, index|
+            context[@variable_name] = item
+            result << @for_block.render(context)
+            loop_vars.send(:increment!)
+
+            # Handle any interrupts if they exist.
+            if context.interrupt?
+              interrupt = context.pop_interrupt
+              break if interrupt.is_a? BreakInterrupt
+              next if interrupt.is_a? ContinueInterrupt
+            end
+          end
+        ensure
+          for_stack.pop
+        end
+      end
+
+      result
+    end
+
     def set_attribute(key, expr)
       case key
       when 'offset'.freeze