Skip to content

Commit

Permalink
keep the expression parse caching per document parsing
Browse files Browse the repository at this point in the history
  • Loading branch information
ggmichaelgo committed Nov 22, 2024
1 parent a9d6e13 commit 0e4da50
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 16 deletions.
20 changes: 12 additions & 8 deletions lib/liquid/expression.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class Expression1
# malicious input as described in https://github.com/Shopify/liquid/issues/1357
RANGES_REGEX = /\A\(\s*(?>(\S+)\s*\.\.)\s*(\S+)\s*\)\z/

def self.parse(markup, _ss = nil)
def self.parse(markup, _ss = nil, _cache = nil)
return nil unless markup

markup = markup.strip
Expand Down Expand Up @@ -72,10 +72,8 @@ class Expression2
INTEGER_REGEX = /\A(-?\d+)\z/
FLOAT_REGEX = /\A(-?\d+)\.\d+\z/

CACHE = LruRedux::Cache.new(10_000) # most themes would have less than 2,000 unique expression

class << self
def parse(markup, ss = StringScanner.new(""))
def parse(markup, ss = StringScanner.new(""), cache = nil)
return unless markup

markup = markup.strip # markup can be a frozen string
Expand All @@ -87,24 +85,30 @@ def parse(markup, ss = StringScanner.new(""))
return LITERALS[markup]
end

return CACHE[markup] if CACHE.key?(markup)
# Cache only exists during parsing
if cache
return cache[markup] if cache.key?(markup)

CACHE[markup] = inner_parse(markup, ss)
cache[markup] = inner_parse(markup, ss, cache)
else
inner_parse(markup, ss, nil)
end
end

def inner_parse(markup, ss)
def inner_parse(markup, ss, cache)
if (markup.start_with?("(") && markup.end_with?(")")) && markup =~ RANGES_REGEX
return RangeLookup.parse(
Regexp.last_match(1),
Regexp.last_match(2),
ss,
cache,
)
end

if (num = parse_number(markup, ss))
num
else
VariableLookup.parse(markup, ss)
VariableLookup.parse(markup, ss, cache)
end
end

Expand Down
5 changes: 3 additions & 2 deletions lib/liquid/parse_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Liquid
class ParseContext
attr_accessor :locale, :line_number, :trim_whitespace, :depth
attr_reader :partial, :warnings, :error_mode, :environment, :string_scanner
attr_reader :partial, :warnings, :error_mode, :environment, :string_scanner, :expression_cache

def initialize(options = Const::EMPTY_HASH)
@environment = options.fetch(:environment, Environment.default)
Expand All @@ -15,6 +15,7 @@ def initialize(options = Const::EMPTY_HASH)
# constructing new StringScanner in Lexer, Tokenizer, etc is expensive
# This StringScanner will be shared by all of them
@string_scanner = StringScanner.new("")
@expression_cache = LruRedux::Cache.new(10_000)

self.depth = 0
self.partial = false
Expand Down Expand Up @@ -42,7 +43,7 @@ def new_tokenizer(source, start_line_number: nil, for_liquid_tag: false)
end

def parse_expression(markup)
Expression.parse(markup, string_scanner)
Expression.parse(markup, string_scanner, @expression_cache)
end

def partial=(value)
Expand Down
6 changes: 3 additions & 3 deletions lib/liquid/range_lookup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

module Liquid
class RangeLookup
def self.parse(start_markup, end_markup, string_scanner)
start_obj = Expression.parse(start_markup, string_scanner)
end_obj = Expression.parse(end_markup, string_scanner)
def self.parse(start_markup, end_markup, string_scanner, cache = nil)
start_obj = Expression.parse(start_markup, string_scanner, cache)
end_obj = Expression.parse(end_markup, string_scanner, cache)
if start_obj.respond_to?(:evaluate) || end_obj.respond_to?(:evaluate)
new(start_obj, end_obj)
else
Expand Down
8 changes: 5 additions & 3 deletions lib/liquid/variable_lookup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@ class VariableLookup

attr_reader :name, :lookups

def self.parse(markup, string_scanner)
new(markup, string_scanner)
def self.parse(markup, string_scanner, cache = nil)
new(markup, string_scanner, cache)
end

def initialize(markup, string_scanner = StringScanner.new(""))
def initialize(markup, string_scanner = StringScanner.new(""), cache = nil)
lookups = markup.scan(VariableParser)

name = lookups.shift
if name&.start_with?('[') && name&.end_with?(']')
name = Expression.parse(
name[1..-2],
string_scanner,
cache,
)
end
@name = name
Expand All @@ -31,6 +32,7 @@ def initialize(markup, string_scanner = StringScanner.new(""))
lookups[i] = Expression.parse(
lookup[1..-2],
string_scanner,
cache,
)
elsif COMMAND_METHODS.include?(lookup)
@command_flags |= 1 << i
Expand Down

0 comments on commit 0e4da50

Please sign in to comment.