From 23bb50c71cdff0c63f9d4402364f6d662a25ffa7 Mon Sep 17 00:00:00 2001 From: ashmaroli Date: Wed, 28 Feb 2018 21:36:08 +0530 Subject: [PATCH] Bypass rendering via Liquid unless required (#6735) Merge pull request 6735 --- benchmark/conditional_liquid.rb | 101 ++++++++++++++++++++++++++++++++ lib/jekyll/convertible.rb | 4 +- lib/jekyll/document.rb | 3 +- lib/jekyll/renderer.rb | 2 +- lib/jekyll/utils.rb | 8 +++ 5 files changed, 114 insertions(+), 4 deletions(-) create mode 100755 benchmark/conditional_liquid.rb diff --git a/benchmark/conditional_liquid.rb b/benchmark/conditional_liquid.rb new file mode 100755 index 00000000..49d08271 --- /dev/null +++ b/benchmark/conditional_liquid.rb @@ -0,0 +1,101 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "liquid" +require "benchmark/ips" + +# Test if processing content string without any Liquid constructs, via Liquid, +# is slower than checking whether constructs exist ( using `String#include?` ) +# and return-ing the "plaintext" content string as is.. +# +# Ref: https://github.com/jekyll/jekyll/pull/6735 + +# Sample contents +WITHOUT_LIQUID = <<-TEXT.freeze +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce auctor libero at +pharetra tempus. Etiam bibendum magna et metus fermentum, eu cursus lorem +mattis. Curabitur vel dui et lacus rutrum suscipit et eget neque. + +Nullam luctus fermentum est id blandit. Phasellus consectetur ullamcorper +ligula, at finibus eros laoreet id. Etiam sit amet est in libero efficitur +tristique. Ut nec magna augue. Quisque ut fringilla lacus, ac dictum enim. +Aliquam vel ornare mauris. Suspendisse ornare diam tempor nulla facilisis +aliquet. Sed ultrices placerat ultricies. +TEXT + +WITH_LIQUID = <<-LIQUID.freeze +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce auctor libero at +pharetra tempus. {{ author }} et metus fermentum, eu cursus lorem +mattis. Curabitur vel dui et lacus rutrum suscipit et eget neque. + +Nullam luctus fermentum est id blandit. Phasellus consectetur ullamcorper +ligula, {% if author == "Jane Doe" %} at finibus eros laoreet id. {% else %} +Etiam sit amet est in libero efficitur.{% endif %} +tristique. Ut nec magna augue. Quisque ut fringilla lacus, ac dictum enim. +Aliquam vel ornare mauris. Suspendisse ornare diam tempor nulla facilisis +aliquet. Sed ultrices placerat ultricies. +LIQUID + +WITH_JUST_LIQUID_VAR = <<-LIQUID.freeze +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce auctor libero at +pharetra tempus. et metus fermentum, eu cursus lorem, ac dictum enim. +mattis. Curabitur vel dui et lacus rutrum suscipit et {{ title }} neque. + +Nullam luctus fermentum est id blandit. Phasellus consectetur ullamcorper +ligula, at finibus eros laoreet id. Etiam sit amet est in libero efficitur. +tristique. Ut nec magna augue. {{ author }} Quisque ut fringilla lacus +Aliquam vel ornare mauris. Suspendisse ornare diam tempor nulla facilisis +aliquet. Sed ultrices placerat ultricies. +LIQUID + +SUITE = { + :"plain text" => WITHOUT_LIQUID, + :"tags n vars" => WITH_LIQUID, + :"just vars" => WITH_JUST_LIQUID_VAR, +}.freeze + +# Mimic how Jekyll's LiquidRenderer would process a non-static file, with +# some dummy payload +def always_liquid(content) + Liquid::Template.error_mode = :warn + Liquid::Template.parse(content, :line_numbers => true).render( + "author" => "John Doe", + "title" => "FooBar" + ) +end + +# Mimic how the proposed change would first execute a couple of checks and +# proceed to process with Liquid if necessary +def conditional_liquid(content) + return content if content.nil? || content.empty? + return content unless content.include?("{%") || content.include?("{{") + always_liquid(content) +end + +# Test https://github.com/jekyll/jekyll/pull/6735#discussion_r165499868 +# ------------------------------------------------------------------------ +def check_with_regex(content) + !content.to_s.match?(%r!{[{%]!) +end + +def check_with_builtin(content) + content.include?("{%") || content.include?("{{") +end + +SUITE.each do |key, text| + Benchmark.ips do |x| + x.report("regex-check - #{key}") { check_with_regex(text) } + x.report("builtin-check - #{key}") { check_with_builtin(text) } + x.compare! + end +end +# ------------------------------------------------------------------------ + +# Let's roll! +SUITE.each do |key, text| + Benchmark.ips do |x| + x.report("always thru liquid - #{key}") { always_liquid(text) } + x.report("conditional liquid - #{key}") { conditional_liquid(text) } + x.compare! + end +end diff --git a/lib/jekyll/convertible.rb b/lib/jekyll/convertible.rb index d3749a1a..b0308ab3 100644 --- a/lib/jekyll/convertible.rb +++ b/lib/jekyll/convertible.rb @@ -165,9 +165,9 @@ module Jekyll # Determine whether the file should be rendered with Liquid. # - # Always returns true. + # Returns true if the file has Liquid Tags or Variables, false otherwise. def render_with_liquid? - true + Jekyll::Utils.has_liquid_construct?(content) end # Determine whether the file should be placed into layouts. diff --git a/lib/jekyll/document.rb b/lib/jekyll/document.rb index eb0e17ee..a22c67c7 100644 --- a/lib/jekyll/document.rb +++ b/lib/jekyll/document.rb @@ -157,9 +157,10 @@ module Jekyll # Determine whether the file should be rendered with Liquid. # # Returns false if the document is either an asset file or a yaml file, + # or if the document doesn't contain any Liquid Tags or Variables, # true otherwise. def render_with_liquid? - !(coffeescript_file? || yaml_file?) + !(coffeescript_file? || yaml_file? || !Utils.has_liquid_construct?(content)) end # Determine whether the file should be rendered with a layout. diff --git a/lib/jekyll/renderer.rb b/lib/jekyll/renderer.rb index 26637139..1d68a874 100644 --- a/lib/jekyll/renderer.rb +++ b/lib/jekyll/renderer.rb @@ -77,7 +77,7 @@ module Jekyll end Jekyll.logger.debug "Rendering Markup:", document.relative_path - output = convert(output) + output = convert(output.to_s) document.content = output if document.place_in_layout? diff --git a/lib/jekyll/utils.rb b/lib/jekyll/utils.rb index cb649391..742a862c 100644 --- a/lib/jekyll/utils.rb +++ b/lib/jekyll/utils.rb @@ -147,6 +147,14 @@ module Jekyll rescue EOFError false end + + # Determine whether the given content string contains Liquid Tags or Vaiables + # + # Returns true is the string contains sequences of `{%` or `{{` + def has_liquid_construct?(content) + return false if content.nil? || content.empty? + content.include?("{%") || content.include?("{{") + end # rubocop: enable PredicateName # Slugify a filename or title.