diff --git a/Gemfile b/Gemfile index 013b1700..2053d5e0 100644 --- a/Gemfile +++ b/Gemfile @@ -24,3 +24,6 @@ group :test do gem 'rubocop-shopify', '~> 2.12.0', require: false gem 'rubocop-performance', require: false end + +gem "strscan", ">= 3.1" +gem "lru_redux" diff --git a/Rakefile b/Rakefile index 21135d1b..6ccd2c86 100755 --- a/Rakefile +++ b/Rakefile @@ -71,7 +71,7 @@ end namespace :benchmark do desc "Run the liquid benchmark with lax parsing" - task :run do + task :lax do ruby "./performance/benchmark.rb lax" end @@ -80,11 +80,30 @@ namespace :benchmark do ruby "./performance/benchmark.rb strict" end + desc "Run the liquid benchmark with both lax and strict parsing" + task run: [:lax, :strict] + desc "Run unit benchmarks" - task :unit do - Dir["./performance/unit/*_benchmark.rb"].each do |file| - puts "🧪 Running #{file}" - ruby file + namespace :unit do + task :all do + Dir["./performance/unit/*_benchmark.rb"].each do |file| + puts "🧪 Running #{file}" + ruby file + end + end + + task :lexer do + Dir["./performance/unit/lexer_benchmark.rb"].each do |file| + puts "🧪 Running #{file}" + ruby file + end + end + + task :expression do + Dir["./performance/unit/expression_benchmark.rb"].each do |file| + puts "🧪 Running #{file}" + ruby file + end end end end diff --git a/lib/liquid/expression.rb b/lib/liquid/expression.rb index a1426732..11a695ad 100644 --- a/lib/liquid/expression.rb +++ b/lib/liquid/expression.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true +require "lru_redux" + module Liquid - class Expression + class Expression1 LITERALS = { nil => nil, 'nil' => nil, @@ -45,4 +47,106 @@ module Liquid end end end + + class Expression2 + LITERALS = { + nil => nil, + 'nil' => nil, + 'null' => nil, + '' => nil, + 'true' => true, + 'false' => false, + 'blank' => '', + 'empty' => '' + }.freeze + + DOT = ".".ord + ZERO = "0".ord + NINE = "9".ord + DASH = "-".ord + + # Use an atomic group (?>...) to avoid pathological backtracing from + # malicious input as described in https://github.com/Shopify/liquid/issues/1357 + RANGES_REGEX = /\A\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\z/ + CACHE = LruRedux::Cache.new(1_000_000) # most themes would have less than 2,000 unique expression + + class << self + def string_scanner + @ss ||= StringScanner.new("") + end + + def parse(markup) + return unless markup + + markup = markup.strip # markup can be a frozen string + + return CACHE[markup] if CACHE.key?(markup) + + CACHE[markup] = inner_parse(markup) + end + + def inner_parse(markup) + if (markup.start_with?('"') && markup.end_with?('"')) || + (markup.start_with?("'") && markup.end_with?("'")) + return markup[1..-2] + elsif (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX + return RangeLookup.parse(Regexp.last_match(1), Regexp.last_match(2)) + end + + return LITERALS[markup] if LITERALS.key?(markup) + + if (num = parse_number(markup)) + num + else + VariableLookup.parse(markup) + end + end + + def parse_number(markup) + ss = string_scanner + ss.string = markup + + is_integer = true + last_dot_pos = nil + num_end_pos = nil + + # the first byte must be a digit, a period, or a dash + byte = ss.scan_byte + + return false if byte != DASH && byte != DOT && (byte < ZERO || byte > NINE) + + while (byte = ss.scan_byte) + return false if byte != DOT && (byte < ZERO || byte > NINE) + + # we found our number and now we are just scanning the rest of the string + next if num_end_pos + + if byte == DOT + if is_integer == false + num_end_pos = ss.pos - 1 + else + is_integer = false + last_dot_pos = ss.pos + end + end + end + + num_end_pos = markup.length if ss.eos? + + return markup.to_i if is_integer + + if num_end_pos + # number ends with a number "123.123" + markup.byteslice(0, num_end_pos).to_f + elsif last_dot_pos + markup.byteslice(0, last_dot_pos).to_f + else + # we should never reach this point + false + end + end + end + end + + Expression = StringScanner.instance_methods.include?(:scan_byte) ? Expression2 : Expression1 end diff --git a/lib/liquid/lexer.rb b/lib/liquid/lexer.rb index b9e5443c..c2adec78 100644 --- a/lib/liquid/lexer.rb +++ b/lib/liquid/lexer.rb @@ -92,6 +92,7 @@ module Liquid SINGLE_COMPARISON_TOKENS = [].tap do |table| table["<".ord] = COMPARISON_LESS_THAN table[">".ord] = COMPARISON_GREATER_THAN + table.freeze end TWO_CHARS_COMPARISON_JUMP_TABLE = [].tap do |table| @@ -103,18 +104,17 @@ module Liquid sub_table["=".ord] = COMPARISION_NOT_EQUAL sub_table.freeze end + table.freeze end COMPARISON_JUMP_TABLE = [].tap do |table| table["<".ord] = [].tap do |sub_table| sub_table["=".ord] = COMPARISON_LESS_THAN_OR_EQUAL sub_table[">".ord] = COMPARISON_NOT_EQUAL_ALT - RUBY_WHITESPACE.each { |c| sub_table[c.ord] = COMPARISON_LESS_THAN } sub_table.freeze end table[">".ord] = [].tap do |sub_table| sub_table["=".ord] = COMPARISON_GREATER_THAN_OR_EQUAL - RUBY_WHITESPACE.each { |c| sub_table[c.ord] = COMPARISON_GREATER_THAN } sub_table.freeze end table.freeze @@ -157,8 +157,15 @@ module Liquid table.freeze end + class << self + def string_scanner + @string_scanner ||= StringScanner.new("") + end + end + def initialize(input) - @ss = StringScanner.new(input) + @ss = Lexer2.string_scanner + @ss.string = input end # rubocop:disable Metrics/BlockNesting @@ -233,5 +240,6 @@ module Liquid end end + # Remove this once we can depend on strscan >= 3.1.1 Lexer = StringScanner.instance_methods.include?(:scan_byte) ? Lexer2 : Lexer1 end diff --git a/lib/liquid/tokenizer.rb b/lib/liquid/tokenizer.rb index 740f8599..0bb05c73 100644 --- a/lib/liquid/tokenizer.rb +++ b/lib/liquid/tokenizer.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true +require "strscan" + module Liquid - class Tokenizer + class Tokenizer1 attr_reader :line_number, :for_liquid_tag def initialize(source, line_numbers = false, line_number: nil, for_liquid_tag: false) @@ -42,4 +44,154 @@ module Liquid tokens end end + + class Tokenizer2 + attr_reader :line_number, :for_liquid_tag + + TAG_END = /%\}/ + TAG_OR_VARIABLE_START = /\{[\{\%]/ + NEWLINE = /\n/ + + OPEN_CURLEY = "{".ord + CLOSE_CURLEY = "}".ord + PERCENTAGE = "%".ord + + class << self + def string_scanner + @string_scanner ||= StringScanner.new("") + end + end + + def initialize(source, line_numbers = false, line_number: nil, for_liquid_tag: false) + @line_number = line_number || (line_numbers ? 1 : nil) + @for_liquid_tag = for_liquid_tag + @source = source + @offset = 0 + @tokens = [] + tokenize + end + + def shift + token = @tokens[@offset] + + return unless token + + @offset += 1 + + if @line_number + @line_number += @for_liquid_tag ? 1 : token.count("\n") + end + + token + end + + private + + def tokenize + if @for_liquid_tag + @tokens = @source.split("\n") + else + @ss = Tokenizer2.string_scanner + @ss.string = @source + @tokens << shift_normal until @ss.eos? + end + + @ss = nil + @source = nil + end + + def shift_normal + token = next_token + + return unless token + + token + end + + def next_token + # possible states: :text, :tag, :variable + byte_a = @ss.peek_byte + + if byte_a == OPEN_CURLEY + @ss.scan_byte + + byte_b = @ss.peek_byte + + if byte_b == PERCENTAGE + @ss.scan_byte + return next_tag_token + elsif byte_b == OPEN_CURLEY + @ss.scan_byte + return next_variable_token + end + + @ss.pos -= 1 + end + + next_text_token + end + + def next_text_token + start = @ss.pos + + unless @ss.skip_until(TAG_OR_VARIABLE_START) + token = @ss.rest + @ss.terminate + return token + end + + pos = @ss.pos -= 2 + @source.byteslice(start, pos - start) + end + + def next_variable_token + start = @ss.pos - 2 + + byte_a = byte_b = @ss.scan_byte + + while byte_b + byte_a = @ss.scan_byte while byte_a && (byte_a != CLOSE_CURLEY && byte_a != OPEN_CURLEY) + + break unless byte_a + + if @ss.eos? + return byte_a == CLOSE_CURLEY ? @source.byteslice(start, @ss.pos - start) : "{{" + end + + byte_b = @ss.scan_byte + + if byte_a == CLOSE_CURLEY + if byte_b == CLOSE_CURLEY + return @source.byteslice(start, @ss.pos - start) + elsif byte_b != CLOSE_CURLEY + @ss.pos -= 1 + return @source.byteslice(start, @ss.pos - start) + end + elsif byte_a == OPEN_CURLEY && byte_b == PERCENTAGE + return next_tag_token_with_start(start) + end + + byte_a = byte_b + end + + "{{" + end + + def next_tag_token + start = @ss.pos - 2 + if (len = @ss.skip_until(TAG_END)) + @source.byteslice(start, len + 2) + else + "{%" + end + end + + def next_tag_token_with_start(start) + @ss.skip_until(TAG_END) + @source.byteslice(start, @ss.pos - start) + end + end + + # Remove this once we can depend on strscan >= 3.1.1 + Tokenizer = StringScanner.instance_methods.include?(:scan_byte) ? Tokenizer2 : Tokenizer1 end diff --git a/performance/benchmark.rb b/performance/benchmark.rb index f897c511..b61e9057 100644 --- a/performance/benchmark.rb +++ b/performance/benchmark.rb @@ -9,14 +9,17 @@ Liquid::Environment.default.error_mode = ARGV.first.to_sym if ARGV.first profiler = ThemeRunner.new Benchmark.ips do |x| - x.time = 10 - x.warmup = 5 + x.time = 20 + x.warmup = 10 puts puts "Running benchmark for #{x.time} seconds (with #{x.warmup} seconds warmup)." puts - x.report("parse:") { profiler.compile } - x.report("render:") { profiler.render } - x.report("parse & render:") { profiler.run } + phase = ENV["PHASE"] || "all" + + x.report("tokenize:") { profiler.tokenize } if phase == "all" || phase == "tokenize" + x.report("parse:") { profiler.compile } if phase == "all" || phase == "parse" + x.report("render:") { profiler.render } if phase == "all" || phase == "render" + x.report("parse & render:") { profiler.run } if phase == "all" || phase == "run" end diff --git a/performance/theme_runner.rb b/performance/theme_runner.rb index 3dd5548e..c158f877 100644 --- a/performance/theme_runner.rb +++ b/performance/theme_runner.rb @@ -48,6 +48,14 @@ class ThemeRunner end end + # `tokenize` will just test the tokenizen portion of liquid without any templates + def tokenize + @tests.each do |test_hash| + tokenizer = Liquid::Tokenizer.new(test_hash[:liquid], true) + while tokenizer.shift; end + end + end + # `run` is called to benchmark rendering and compiling at the same time def run each_test do |liquid, layout, assigns, page_template, template_name| diff --git a/performance/unit/expression_benchmark.rb b/performance/unit/expression_benchmark.rb new file mode 100644 index 00000000..ae4eeaa5 --- /dev/null +++ b/performance/unit/expression_benchmark.rb @@ -0,0 +1,183 @@ +# frozen_string_literal: true + +require "benchmark/ips" + +# benchmark liquid lexing + +require 'liquid' + +RubyVM::YJIT.enable + +STRING_MARKUPS = [ + "\"foo\"", + "\"fooooooooooo\"", + "\"foooooooooooooooooooooooooooooo\"", + "'foo'", + "'fooooooooooo'", + "'foooooooooooooooooooooooooooooo'", +] + +VARIABLE_MARKUPS = [ + "article", + "article.title", + "article.title.size", + "very_long_variable_name_2024_11_05", + "very_long_variable_name_2024_11_05.size", +] + +NUMBER_MARKUPS = [ + "0", + "35", + "1241891024912849", + "3.5", + "3.51214128409128", + "12381902839.123819283910283", + "123.456.789", + "-123", + "-12.33", + "-405.231", + "-0", + "0", + "0.0", + "0.0000000000000000000000", + "0.00000000001", +] + +RANGE_MARKUPS = [ + "(1..30)", + "(1...30)", + "(1..30..5)", + "(1.0...30.0)", + "(1.........30)", + "(1..foo)", + "(foo..30)", + "(foo..bar)", + "(foo...bar...100)", + "(foo...bar...100.0)", +] + +LITERAL_MARKUPS = [ + nil, + 'nil', + 'null', + '', + 'true', + 'false', + 'blank', + 'empty', +] + +MARKUPS = { + "string" => STRING_MARKUPS, + "literal" => LITERAL_MARKUPS, + "variable" => VARIABLE_MARKUPS, + "number" => NUMBER_MARKUPS, + "range" => RANGE_MARKUPS, +} + +def compare_objects(object_1, object_2) + if object_1.is_a?(Liquid::VariableLookup) && object_2.is_a?(Liquid::VariableLookup) + return false if object_1.name != object_2.name + elsif object_1 != object_2 + return false + end + + true +end + +def compare_range_lookup(expression_1_result, expression_2_result) + return false unless expression_1_result.is_a?(Liquid::RangeLookup) && expression_2_result.is_a?(Liquid::RangeLookup) + + start_obj_1 = expression_1_result.start_obj + start_obj_2 = expression_2_result.start_obj + + return false unless compare_objects(start_obj_1, start_obj_2) + + end_obj_1 = expression_1_result.end_obj + end_obj_2 = expression_2_result.end_obj + + return false unless compare_objects(end_obj_1, end_obj_2) + + true +end + +MARKUPS.values.flatten.each do |markup| + expression_1_result = Liquid::Expression1.parse(markup) + expression_2_result = Liquid::Expression2.parse(markup) + + next if expression_1_result == expression_2_result + + if expression_1_result.is_a?(Liquid::RangeLookup) && expression_2_result.is_a?(Liquid::RangeLookup) + next if compare_range_lookup(expression_1_result, expression_2_result) + end + + warn "Expression1 and Expression2 results are different for markup: #{markup}" + warn "expected: #{expression_1_result}" + warn "got: #{expression_2_result}" + abort +end + +warmed_up = false + +MARKUPS.each do |type, markups| + Benchmark.ips do |x| + if warmed_up + x.config(time: 10, warmup: 5) + warmed_up = true + else + x.config(time: 10) + end + + x.report("Liquid::Expression1#parse: #{type}") do + if Liquid::Expression != Liquid::Expression1 + Liquid.send(:remove_const, :Expression) + Liquid.const_set(:Expression, Liquid::Expression1) + end + + markups.each do |markup| + Liquid::Expression1.parse(markup) + end + end + + x.report("Liquid::Expression2#parse: #{type}") do + if Liquid::Expression != Liquid::Expression2 + Liquid.send(:remove_const, :Expression) + Liquid.const_set(:Expression, Liquid::Expression2) + end + + markups.each do |markup| + Liquid::Expression2.parse(markup) + end + end + + x.compare! + end +end + +Benchmark.ips do |x| + x.config(time: 10) + + x.report("Liquid::Expression1#parse: all") do + if Liquid::Expression != Liquid::Expression1 + Liquid.send(:remove_const, :Expression) + Liquid.const_set(:Expression, Liquid::Expression1) + end + + MARKUPS.values.flatten.each do |markup| + Liquid::Expression1.parse(markup) + end + end + + x.report("Liquid::Expression2#parse: all") do + if Liquid::Expression != Liquid::Expression2 + Liquid.send(:remove_const, :Expression) + Liquid.const_set(:Expression, Liquid::Expression2) + end + + MARKUPS.values.flatten.each do |markup| + Liquid::Expression2.parse(markup) + end + end + + x.compare! +end diff --git a/test/integration/expression_test.rb b/test/integration/expression_test.rb index 80f6f417..7f451afd 100644 --- a/test/integration/expression_test.rb +++ b/test/integration/expression_test.rb @@ -13,6 +13,7 @@ class ExpressionTest < Minitest::Test assert_template_result("double quoted", '{{"double quoted"}}') assert_template_result("spaced", "{{ 'spaced' }}") assert_template_result("spaced2", "{{ 'spaced2' }}") + assert_template_result("emoji🔥", "{{ 'emoji🔥' }}") end def test_int @@ -22,6 +23,7 @@ class ExpressionTest < Minitest::Test end def test_float + assert_template_result("-17.42", "{{ -17.42 }}") assert_template_result("2.5", "{{ 2.5 }}") assert_expression_result(1.5, "1.5") end diff --git a/test/integration/parsing_quirks_test.rb b/test/integration/parsing_quirks_test.rb index 48f91463..f5b483fc 100644 --- a/test/integration/parsing_quirks_test.rb +++ b/test/integration/parsing_quirks_test.rb @@ -134,6 +134,14 @@ class ParsingQuirksTest < Minitest::Test def test_incomplete_expression with_error_mode(:lax) do + assert_template_result("false", "{{ false - }}") + assert_template_result("false", "{{ false > }}") + assert_template_result("false", "{{ false < }}") + assert_template_result("false", "{{ false = }}") + assert_template_result("false", "{{ false ! }}") + assert_template_result("false", "{{ false 1 }}") + assert_template_result("false", "{{ false a }}") + assert_template_result("false", "{% liquid assign foo = false -\n%}{{ foo }}") assert_template_result("false", "{% liquid assign foo = false >\n%}{{ foo }}") assert_template_result("false", "{% liquid assign foo = false <\n%}{{ foo }}") diff --git a/test/integration/tags/raw_tag_test.rb b/test/integration/tags/raw_tag_test.rb index 3629f39c..2ca43d3b 100644 --- a/test/integration/tags/raw_tag_test.rb +++ b/test/integration/tags/raw_tag_test.rb @@ -16,6 +16,7 @@ class RawTagTest < Minitest::Test assert_template_result('>{{ test }}<', '> {%- raw -%}{{ test }}{%- endraw -%} <') assert_template_result("> inner <", "> {%- raw -%} inner {%- endraw %} <") assert_template_result("> inner <", "> {%- raw -%} inner {%- endraw -%} <") + assert_template_result("{Hello}", "{% raw %}{{% endraw %}Hello{% raw %}}{% endraw %}") end def test_open_tag_in_raw diff --git a/test/unit/tokenizer_unit_test.rb b/test/unit/tokenizer_unit_test.rb index 5e0496e6..76d379d3 100644 --- a/test/unit/tokenizer_unit_test.rb +++ b/test/unit/tokenizer_unit_test.rb @@ -6,6 +6,7 @@ class TokenizerTest < Minitest::Test def test_tokenize_strings assert_equal([' '], tokenize(' ')) assert_equal(['hello world'], tokenize('hello world')) + assert_equal(['{}'], tokenize('{}')) end def test_tokenize_variables @@ -34,6 +35,19 @@ class TokenizerTest < Minitest::Test assert_equal([], tokenize(nil)) end + def test_incomplete_curly_braces + assert_equal(["{{.}", " "], tokenize('{{.} ')) + assert_equal(["{{}", "%}"], tokenize('{{}%}')) + assert_equal(["{{}}", "}"], tokenize('{{}}}')) + end + + def test_unmatching_start_and_end + assert_equal(["{{%}"], tokenize('{{%}')) + assert_equal(["{{%%%}}"], tokenize('{{%%%}}')) + assert_equal(["{%", "}}"], tokenize('{%}}')) + assert_equal(["{%%}", "}"], tokenize('{%%}}')) + end + private def new_tokenizer(source, parse_context: Liquid::ParseContext.new, start_line_number: nil)