WIP: more careful IF block merger

Rakefile

@@ -81,10 +81,21 @@ namespace :benchmark do
   end
 
   desc "Run unit benchmarks"
-  task :unit do
-    Dir["./performance/unit/*_benchmark.rb"].each do |file|
-      puts "🧪 Running #{file}"
-      ruby file
+  namespace :unit do
+    desc "Run all unit benchmarks"
+    task :all do
+      Dir["./performance/unit/*_benchmark.rb"].each do |file|
+        puts "🧪 Running #{file}"
+        ruby file
+      end
+    end
+
+    %w[lexer loom].each do |benchmark|
+      desc "Run the #{benchmark} benchmark"
+      task benchmark.to_sym do
+        puts "🧪 Running #{benchmark}"
+        ruby "./performance/unit/#{benchmark}_benchmark.rb"
+      end
     end
   end
 end

lib/liquid.rb

@@ -86,3 +86,4 @@ require 'liquid/partial_cache'
 require 'liquid/usage'
 require 'liquid/registers'
 require 'liquid/template_factory'
+require 'liquid/loom'

lib/liquid/block_body.rb

@@ -31,10 +31,11 @@ module Liquid
       end
     end
 
-    def freeze
-      @nodelist.freeze
-      super
-    end
+    # TODO: Freeze the nodelist after optimization
+    # def freeze
+    #   @nodelist.freeze
+    #   super
+    # end
 
     private def parse_for_liquid_tag(tokenizer, parse_context)
       while (token = tokenizer.shift)
@@ -154,6 +155,12 @@ module Liquid
           end
           new_tag = tag.parse(tag_name, markup, tokenizer, parse_context)
           @blank &&= new_tag.blank?
+
+          if parse_context.eager_optimize
+            next if new_tag.nodelist&.all? { !_1.is_a?(String) && _1.nodelist.empty? } # this is an empty block
+            next if new_tag.is_a?(If) && new_tag.blocks.empty? # this is an empty If block
+          end
+
           @nodelist << new_tag
         when token.start_with?(VARSTART)
           whitespace_handler(token, parse_context)

lib/liquid/loom.rb

@@ -0,0 +1,123 @@
+# frozen_string_literal: true
+
+module Liquid
+  class Loom
+    MERGABLE_IF_OPERATORS = ["==", ">", "<", "!="].freeze
+    EQUAL_OP = "==".freeze
+
+    class << self
+      def optimize(template)
+        new(template).optimize
+      end
+    end
+
+    def initialize template
+      @root = template.root
+    end
+
+    def optimize
+      merge_if_blocks
+    end
+
+    def merge_if_blocks
+      nodelist_list = [@root.nodelist]
+
+      while nodelist_list.any?
+        next_nodelist_list = []
+
+        nodelist_list.each do |nodelist|
+          i = 0
+          while i < nodelist.length
+            node = nodelist[i]
+            chain_if_blocks(nodelist, node, i) if node.is_a?(If)
+            i += 1
+          end
+        end
+
+        nodelist_list = next_nodelist_list
+      end
+    end
+
+    private
+
+    def mergable_if_blocks?(target_if, next_if)
+      target_left = target_if.blocks.first.left
+      target_right = target_if.blocks.first.right
+      next_left = next_if.blocks.first.left
+      next_right = next_if.blocks.first.right
+
+      used_variables = Hash.new { |h, k| h[k] = 0 }
+
+      [
+        target_if.blocks.first.left,
+        target_if.blocks.first.right,
+        next_if.blocks.first.left,
+        next_if.blocks.first.right
+      ].each do |var|
+        if var.is_a?(VariableLookup)
+          used_variables[var.name] += 1
+        end
+      end
+
+      return if used_variables.keys.count > 1
+
+      most_used_variable_name = used_variables.keys[0]
+
+      # TODO: I probably can't do this
+      # It might be possible to get different result between a > b and b < a
+      # Move most commonly used variable to the left side
+      if (target_left.is_a?(VariableLookup) && target_left.name != most_used_variable_name) || (target_right.is_a?(VariableLookup) && target_right.name == most_used_variable_name)
+        target_left, target_right = target_right, target_left
+      end
+
+      if (next_left.is_a?(VariableLookup) && next_left.name != most_used_variable_name) || (next_right.is_a?(VariableLookup) && next_right.name == most_used_variable_name)
+        next_left, next_right = next_right, next_left
+      end
+
+      return false unless target_left.is_a?(VariableLookup) && next_left.is_a?(VariableLookup)
+      return false if target_left.name != next_left.name
+
+      return false if target_right.nil? || next_right.nil?
+
+
+      # we need to be conversative here and only can merge ==, >, <, and != operators
+      target_operator = target_if.blocks.first.operator
+      next_operator = next_if.blocks.first.operator
+
+      return false unless MERGABLE_IF_OPERATORS.include?(target_operator) && MERGABLE_IF_OPERATORS.include?(next_operator)
+
+      return false if target_operator == next_operator && target_right == next_right
+
+      return false if target_right.is_a?(VariableLookup) || next_right.is_a?(VariableLookup)
+
+      true
+    end
+
+    def chain_if_blocks(nodelist, first_if_node, first_if_index)
+      used_variables = Set.new
+
+      # only check the top level Condition (ignore children conditions for now)
+      first_if_node.blocks.each do |condition|
+        used_variables << condition.left
+        used_variables << condition.right if condition.right
+      end
+
+      if_blocks = []
+
+      nodelist[first_if_index + 1..-1].each do |node|
+        break unless node.is_a?(If)
+
+        # check if the variables used in the current block are used in the previous block
+        break unless mergable_if_blocks?(first_if_node, node)
+
+        if_blocks << node
+      end
+
+      nodelist.delete_if { |node| if_blocks.include?(node) }
+
+      if_blocks.each do |if_block|
+        first_if_node.blocks << if_block.blocks.first
+      end
+    end
+  end
+end

lib/liquid/parse_context.rb

@@ -3,7 +3,7 @@
 module Liquid
   class ParseContext
     attr_accessor :locale, :line_number, :trim_whitespace, :depth
-    attr_reader :partial, :warnings, :error_mode, :environment
+    attr_reader :partial, :warnings, :error_mode, :environment, :eager_optimize
 
     def initialize(options = Const::EMPTY_HASH)
       @environment = options.fetch(:environment, Environment.default)
@@ -11,6 +11,7 @@ module Liquid
 
       @locale   = @template_options[:locale] ||= I18n.new
       @warnings = []
+      @eager_optimize = options.fetch(:eager_optimize, ENV["OPTIMIZE"] == "true")
 
       self.depth   = 0
       self.partial = false

lib/liquid/tags/if.rb

@@ -23,6 +23,7 @@ module Liquid
     def initialize(tag_name, markup, options)
       super
       @blocks = []
+      @has_else_block = false
       push_block('if', markup)
     end
 
@@ -33,17 +34,44 @@ module Liquid
     def parse(tokens)
       while parse_body(@blocks.last.attachment, tokens)
       end
-      @blocks.reverse_each do |block|
-        block.attachment.remove_blank_strings if blank?
-        block.attachment.freeze
+
+      if parse_context.eager_optimize && definitive_false_statement?
+        @blocks.clear
+      else
+        @blocks.reverse_each do |block|
+          block.attachment.remove_blank_strings if blank?
+          block.attachment.freeze
+        end
       end
     end
 
+    def definitive_false_statement?
+      # check if any blocks have variable lookups
+      @blocks.each do |condition|
+        return false if condition.left.is_a?(VariableLookup) || condition.right&.is_a?(VariableLookup)
+
+        child_condition = condition.child_condition
+
+        while child_condition
+          return false if child_condition&.left.is_a?(VariableLookup) || child_condition&.right&.is_a?(VariableLookup)
+          child_condition = child_condition.child_condition
+        end
+      end
+
+      # check if all blocks are false
+      @blocks.each do |condition|
+        return false if condition.evaluate
+      end
+
+      true
+    end
+
     ELSE_TAG_NAMES = ['elsif', 'else'].freeze
     private_constant :ELSE_TAG_NAMES
 
     def unknown_tag(tag, markup, tokens)
       if ELSE_TAG_NAMES.include?(tag)
+        @has_else_block = true
         push_block(tag, markup)
       else
         super

lib/liquid/template.rb

@@ -82,7 +82,11 @@ module Liquid
       # See Liquid::Profiler for more information
       def parse(source, options = {})
         environment = options[:environment] || Environment.default
-        new(environment: environment).parse(source, options)
+        template = new(environment: environment).parse(source, options)
+
+        Loom.optimize(template) if options[:eager_optimize]
+
+        template
       end
     end
 

performance/unit/loom_benchmark.rb

@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require "benchmark/ips"
+require 'liquid'
+
+RubyVM::YJIT.enable
+
+TEMPLATE = <<~LIQUID
+{% if false %}
+  {% for i in (1..1000000) %}
+    {{ "Hello world!" }}
+  {% endfor %}
+{% endif %}
+
+{% assign result = 1 %}
+{% if foo == 1 %}{% assign result = 1 %}{% endif %}{% if foo == 2 %}{% assign result = 2 %}{% endif %}{% if foo == 3 %}{% assign result = 3 %}{% endif %}
+Result: {{ result }}
+LIQUID
+
+baseline_template = Liquid::Template.parse(TEMPLATE, eager_optimize: false)
+optimized_template = Liquid::Template.parse(TEMPLATE, eager_optimize: true)
+
+[nil, 1, 2, 3].each do |foo|
+  baseline_output = baseline_template.render('foo' => foo)
+  optimized_output = optimized_template.render('foo' => foo)
+
+  if baseline_output != optimized_output
+    puts "WARNING! Baseline and optimized templates render differently for foo=#{foo}"
+    puts "Baseline: #{baseline_output}"
+    puts "Optimized: #{optimized_output}"
+
+    raise
+  end
+end
+
+def render(template, foo)
+  template.render('foo' => foo)
+end
+
+Benchmark.ips do |x|
+  x.config(time: 20, warmup: 3)
+
+  x.report("baseline") do
+    [nil, 1, 2, 3].each do |foo|
+      render(baseline_template, foo)
+    end
+  end
+
+  x.report("optimized") do
+    [nil, 1, 2, 3].each do |foo|
+      render(optimized_template, foo)
+    end
+  end
+
+  x.compare!
+end

test/unit/eager_optimize_test.rb

@@ -0,0 +1,186 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class EagerOptimizeTest < Minitest::Test
+  include Liquid
+
+  def test_remove_empty_blocks
+    source = <<~LIQUID.gsub(/\n/, '')
+      {% for i in (1..1000000) %}
+      {% endfor %}
+    LIQUID
+
+    template = Liquid::Template.parse(source, eager_optimize: true)
+    assert_equal(0, total_node_count(template))
+  end
+
+  def test_remove_false_if_block
+    source = <<~LIQUID.gsub(/\n/, '')
+      {% if false %}
+        {% if true %}
+          {% if true %}
+            {% if true %}
+              {{ "Hello world!" }}
+            {% endif %}
+          {% endif %}
+        {% endif %}
+      {% endif %}
+    LIQUID
+
+    template = Liquid::Template.parse(source, eager_optimize: true)
+    assert_equal(0, total_node_count(template))
+
+    source = <<~LIQUID.gsub(/\n/, '')
+      {% if false %}
+        {% for i in (1..1000000) %}
+          {{ "Hello world!" }}
+        {% endfor %}
+      {% endif %}
+    LIQUID
+
+    template = Liquid::Template.parse(source, eager_optimize: true)
+    assert_equal(0, total_node_count(template))
+  end
+
+  def test_remove_multiple_false_if_block
+    source = <<~LIQUID.gsub(/\n/, '')
+      {% if false %}
+        {% if true %}
+          {% if true %}
+            {% if true %}
+              {{ "Hello world!" }}
+            {% endif %}
+          {% endif %}
+        {% endif %}
+      {% endif %}
+    LIQUID
+
+    template = Liquid::Template.parse(source, eager_optimize: true)
+    assert_equal(0, total_node_count(template))
+  end
+
+  def test_merge_if_blocks
+    # for now, work with consecutive if blocks without any String nodes in between
+    source = <<~LIQUID.gsub(/\n/, '')
+      {% if foo == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if foo == 2 %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if foo == 3 %}
+        foo: {{ foo }}
+      {% endif %}
+    LIQUID
+
+    assert_optimization([Liquid::If], source, { "foo" => nil })
+    assert_optimization([Liquid::If], source, { "foo" => 1 })
+    assert_optimization([Liquid::If], source, { "foo" => 2 })
+    assert_optimization([Liquid::If], source, { "foo" => 5 })
+
+    source = <<~LIQUID.gsub(/\n/, '')
+      {% assign bar = "application" %}
+      {% if foo == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if foo == 2 and bar contains "app" %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if 3 == foo and bar == "application" %}
+        foo: {{ foo }}
+      {% endif %}
+    LIQUID
+
+    assert_optimization([Liquid::Assign, Liquid::If], source)
+  end
+
+  def test_does_not_merge_if_blocks
+    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
+      {% if foo == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if k == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+    LIQUID
+
+    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
+      {% if foo == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if foo == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+    LIQUID
+
+    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
+      {% if foo == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if a == foo %}
+        foo: {{ foo }}
+      {% endif %}
+    LIQUID
+
+    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
+      {% if foo %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if foo %}
+        foo: {{ foo }}
+      {% endif %}
+    LIQUID
+
+    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
+      {% if foo == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if foo >= 1 %}
+        foo: {{ foo }}
+      {% endif %}
+    LIQUID
+
+    assert_optimization([Liquid::If, Liquid::If], <<~LIQUID.gsub(/\n/, ''))
+      {% if foo == 1 %}
+        foo: {{ foo }}
+      {% endif %}
+      {% if 1  %}
+        foo: {{ foo }}
+      {% endif %}
+    LIQUID
+  end
+
+  private
+
+  def assert_optimization(expected, source, context = { "foo" => 1 })
+    template = Template.parse(source, eager_optimize: true)
+    assert_equal(expected, template.root.nodelist.map(&:class),)
+
+    baseline_template = Template.parse(source, eager_optimize: false)
+
+    assert_equal(
+      baseline_template.render(context),
+      template.render(context),
+    )
+  end
+
+  def total_node_count(template)
+    root = template.root
+    children = root.nodelist
+    count = 0
+
+    while children.any?
+      next_children = []
+
+      children.each do |node|
+        count += 1 unless node.is_a?(Liquid::BlockBody)
+        next_children.concat(node.nodelist) if node.respond_to?(:nodelist) && node.nodelist
+      end
+
+      children = next_children
+    end
+
+    count
+  end
+end