Merge pull request #383 from Shopify/optimize_variable_lookup

Cache parsed markup parts to avoid repeated calls during template render

History.md

@@ -3,6 +3,7 @@
 ## 3.0.0 / not yet released / branch "master"
 
 * ...
+* Optimize variable parsing to avoid repeated regex evaluation during template rendering #383 [Jason Hiltz-Laforge, jasonhl]
 * Optimize checking for block interrupts to reduce object allocation #380 [Jason Hiltz-Laforge, jasonhl] 
 * Properly set context rethrow_errors on render! #349 [Thierry Joyal, tjoyal]
 * Fix broken rendering of variables which are equal to false, see #345 [Florian Weingarten, fw42]

lib/liquid/context.rb

@@ -17,6 +17,8 @@ module Liquid
 
     attr_accessor :rethrow_errors
 
+    SQUARE_BRACKETED = /\A\[(.*)\]\z/m
+
     def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = {})
       @environments    = [environments].flatten
       @scopes          = [(outer_scope || {})]
@@ -28,6 +30,7 @@ module Liquid
 
       @interrupts = []
       @filters = []
+      @parsed_variables = Hash.new{ |cache, markup| cache[markup] = variable_parse(markup) } 
     end
 
     def increment_used_resources(key, obj)
@@ -213,6 +216,16 @@ module Liquid
         return variable
       end
 
+      def variable_parse(markup)
+        parts = markup.scan(VariableParser)
+        needs_resolution = false
+        if parts.first =~ SQUARE_BRACKETED 
+          needs_resolution = true
+          parts[0] = $1
+        end
+        {:first => parts.shift, :needs_resolution => needs_resolution, :rest => parts} 
+      end
+
       # Resolves namespaced queries gracefully.
       #
       # Example
@@ -220,19 +233,17 @@ module Liquid
       #  assert_equal 'tobi', @context['hash.name']
       #  assert_equal 'tobi', @context['hash["name"]']
       def variable(markup)
-        parts = markup.scan(VariableParser)
+        parts = @parsed_variables[markup]
-        square_bracketed = /\A\[(.*)\]\z/m
 
-        first_part = parts.shift
+        first_part = parts[:first] 
-
+        if parts[:needs_resolution]
-        if first_part =~ square_bracketed
+          first_part = resolve(parts[:first])
-          first_part = resolve($1)
         end
 
         if object = find_variable(first_part)
 
-          parts.each do |part|
+          parts[:rest].each do |part|
-            part = resolve($1) if part_resolved = (part =~ square_bracketed)
+            part = resolve($1) if part_resolved = (part =~ SQUARE_BRACKETED)
 
             # If object is a hash- or array-like object we look for the
             # presence of the key and if its available we return it

test/unit/context_unit_test.rb

@@ -473,4 +473,14 @@ class ContextUnitTest < Test::Unit::TestCase
     assert mock_empty.has_been_called?
   end 
 
+  def test_variable_lookup_caches_markup
+    mock_scan = Spy.on_instance_method(String, :scan).and_return(["string"])
+
+    @context['string'] = 'string'
+    @context['string']
+    @context['string']
+
+    assert_equal 1, mock_scan.calls.size
+  end
+
 end # ContextTest