Compare commits

..

16 Commits

Author SHA1 Message Date
Justin Li
350addf364 Run liquidtruffle from the performance runner 2019-08-23 16:11:00 -04:00
Justin Li
49488e3757 Add rake dump_portable_ast 2019-08-21 13:24:21 -04:00
Florian Weingarten
b3b63a683f Merge pull request #1097 from ashmaroli/stackprof-no-jruby
Don't attempt to install stackprof gem on JRuby
2019-04-24 09:11:44 -04:00
Ashwin Maroli
1c577c5b62 Don't attempt to install stackprof gem on JRuby 2019-04-24 11:31:20 +05:30
David Cornu
755d2821f3 Merge pull request #1094 from Shopify/for-tag/invalid-limit-offset
Make sure the for tag's limit and offset are integers
2019-04-23 17:20:54 -04:00
David Cornu
495b3d312f Merge pull request #1095 from Shopify/travis/remove-rainbow-gem
Stop installing the rainbow gem on Travis
2019-04-23 17:20:38 -04:00
David Cornu
453f6348c2 Stop installing the rainbow gem on Travis 2019-04-23 16:55:37 -04:00
David Cornu
70ed1fc86d Make sure the limit and offset values are integers 2019-04-23 16:44:37 -04:00
Florian Weingarten
c2ef247be5 Merge pull request #1092 from Shopify/rake-memory-profiler-task
rake memory_profile:run
2019-04-22 16:33:32 -04:00
Florian Weingarten
1518d3f6f9 Merge pull request #1093 from Shopify/bytesize-not-length
use bytesize, not length
2019-04-18 18:39:21 +01:00
Florian Weingarten
c67b77709d rake memory_profile:run 2019-04-17 19:09:26 +01:00
Florian Weingarten
c89ce9c2ed use bytesize, not length 2019-04-17 18:55:13 +01:00
Richard Monette
b0629f17f7 Merge pull request #1073 from Shopify/defer-alloc-hash
defer hash allocation until needed for unparsed_args
2019-03-20 13:34:48 -04:00
Richard Monette
274f078806 defer hash allocation in parse_filter_expressions
add exploration of GC object allocation

remove performance test

can actually remove one more if branch

use named locals to improve readability
2019-03-20 13:20:31 -04:00
Richard Monette
d7171aa084 Merge pull request #1077 from Shopify/update-cops-for-trailing-comma
update Rubocop for trailing comma styles
2019-03-19 16:02:26 -04:00
Richard Monette
06c4789dc5 update Rubocop for trailing comma styles 2019-03-19 11:05:05 -04:00
17 changed files with 260 additions and 47 deletions

View File

@@ -1,4 +1,6 @@
inherit_from: ./.rubocop_todo.yml
inherit_from:
- .rubocop_todo.yml
- ./.rubocop_todo.yml
AllCops:
Exclude:
@@ -29,7 +31,7 @@ Lint/ParenthesesAsGroupedExpression:
Lint/UnusedBlockArgument:
Enabled: false
Lint/EndAlignment:
Layout/EndAlignment:
EnforcedStyleAlignWith: variable
Lint/UnusedMethodArgument:
@@ -59,7 +61,7 @@ Style/BracesAroundHashParameters:
Style/NumericLiterals:
Enabled: false
Layout/SpaceInsideBrackets:
Layout/SpaceInsideArrayLiteralBrackets:
Enabled: false
Layout/SpaceBeforeBlockBraces:
@@ -71,7 +73,10 @@ Style/Documentation:
Style/ClassAndModuleChildren:
Enabled: false
Style/TrailingCommaInLiteral:
Style/TrailingCommaInArrayLiteral:
Enabled: false
Style/TrailingCommaInHashLiteral:
Enabled: false
Layout/IndentHash:
@@ -104,7 +109,7 @@ Style/SymbolLiteral:
Performance/Count:
Enabled: false
Style/ConstantName:
Naming/ConstantName:
Enabled: false
Layout/CaseIndentation:
@@ -122,6 +127,6 @@ Style/TrivialAccessors:
Style/WordArray:
Enabled: false
Style/MethodName:
Naming/MethodName:
Exclude:
- 'example/server/liquid_servlet.rb'

View File

@@ -1,14 +1,22 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2019-02-03 21:12:39 +0530 using RuboCop version 0.49.1.
# on 2019-03-19 11:04:37 -0400 using RuboCop version 0.53.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: Include, TreatCommentsAsGroupSeparators.
# Include: **/*.gemspec
Gemspec/OrderedDependencies:
Exclude:
- 'liquid.gemspec'
# Offense count: 5
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: auto_detection, squiggly, active_support, powerpack, unindent
Layout/IndentHeredoc:
Exclude:
@@ -17,7 +25,7 @@ Layout/IndentHeredoc:
# Offense count: 6
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: symmetrical, new_line, same_line
Layout/MultilineMethodCallBraceLayout:
Exclude:
@@ -26,38 +34,65 @@ Layout/MultilineMethodCallBraceLayout:
# Offense count: 2
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: runtime_error, standard_error
Lint/InheritException:
Exclude:
- 'lib/liquid/interrupts.rb'
# Offense count: 51
# Offense count: 1
# Configuration parameters: CheckForMethodsWithNoSideEffects.
Lint/Void:
Exclude:
- 'lib/liquid/parse_context.rb'
# Offense count: 54
Metrics/AbcSize:
Max: 56
# Offense count: 11
# Offense count: 12
Metrics/CyclomaticComplexity:
Max: 12
# Offense count: 639
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
# URISchemes: http, https
Metrics/LineLength:
Max: 294
# Offense count: 108
# Offense count: 112
# Configuration parameters: CountComments.
Metrics/MethodLength:
Max: 37
# Offense count: 7
# Offense count: 8
Metrics/PerceivedComplexity:
Max: 11
# Offense count: 52
# Configuration parameters: Blacklist.
# Blacklist: END, (?-mix:EO[A-Z]{1})
Naming/HeredocDelimiterNaming:
Exclude:
- 'test/integration/assign_test.rb'
- 'test/integration/capture_test.rb'
- 'test/integration/trim_mode_test.rb'
# Offense count: 23
# Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames.
# AllowedNames: io, id
Naming/UncommunicativeMethodParamName:
Exclude:
- 'example/server/example_servlet.rb'
- 'lib/liquid/condition.rb'
- 'lib/liquid/context.rb'
- 'lib/liquid/standardfilters.rb'
- 'lib/liquid/tags/if.rb'
- 'lib/liquid/utils.rb'
- 'lib/liquid/variable.rb'
- 'test/integration/filter_test.rb'
- 'test/integration/standard_filter_test.rb'
- 'test/integration/tags/for_tag_test.rb'
- 'test/integration/template_test.rb'
- 'test/unit/condition_unit_test.rb'
# Offense count: 10
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: prefer_alias, prefer_alias_method
Style/Alias:
Exclude:
@@ -69,14 +104,23 @@ Style/Alias:
- 'lib/liquid/tags/include.rb'
- 'lib/liquid/variable.rb'
# Offense count: 22
Style/CommentedKeyword:
Enabled: false
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles, SingleLineConditionsOnly, IncludeTernaryExpressions.
# Configuration parameters: EnforcedStyle, SingleLineConditionsOnly, IncludeTernaryExpressions.
# SupportedStyles: assign_to_condition, assign_inside_condition
Style/ConditionalAssignment:
Exclude:
- 'lib/liquid/errors.rb'
# Offense count: 1
Style/DateTime:
Exclude:
- 'test/unit/context_unit_test.rb'
# Offense count: 2
# Cop supports --auto-correct.
Style/EmptyCaseCondition:
@@ -86,7 +130,7 @@ Style/EmptyCaseCondition:
# Offense count: 5
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: compact, expanded
Style/EmptyMethod:
Exclude:
@@ -96,11 +140,28 @@ Style/EmptyMethod:
- 'test/integration/tags/include_tag_test.rb'
- 'test/unit/context_unit_test.rb'
# Offense count: 3
# Cop supports --auto-correct.
Style/Encoding:
Exclude:
- 'lib/liquid/version.rb'
- 'liquid.gemspec'
- 'test/integration/standard_filter_test.rb'
# Offense count: 2
# Configuration parameters: SupportedStyles.
# SupportedStyles: annotated, template
# Cop supports --auto-correct.
Style/ExpandPathArguments:
Exclude:
- 'Rakefile'
- 'liquid.gemspec'
# Offense count: 7
# Configuration parameters: EnforcedStyle.
# SupportedStyles: annotated, template, unannotated
Style/FormatStringToken:
EnforcedStyle: template
Exclude:
- 'test/integration/filter_test.rb'
- 'test/integration/hash_ordering_test.rb'
# Offense count: 14
# Configuration parameters: MinBodyLength.
@@ -121,7 +182,7 @@ Style/GuardClause:
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, MinBodyLength, SupportedStyles.
# Configuration parameters: EnforcedStyle, MinBodyLength.
# SupportedStyles: skip_modifier_ifs, always
Style/Next:
Exclude:
@@ -129,7 +190,7 @@ Style/Next:
# Offense count: 4
# Cop supports --auto-correct.
# Configuration parameters: AutoCorrect, EnforcedStyle, SupportedStyles.
# Configuration parameters: AutoCorrect, EnforcedStyle.
# SupportedStyles: predicate, comparison
Style/NumericPredicate:
Exclude:
@@ -166,14 +227,14 @@ Style/Semicolon:
# Offense count: 7
# Cop supports --auto-correct.
# Configuration parameters: MinSize, SupportedStyles.
# Configuration parameters: MinSize.
# SupportedStyles: percent, brackets
Style/SymbolArray:
EnforcedStyle: brackets
# Offense count: 2
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles, AllowSafeAssignment.
# Configuration parameters: EnforcedStyle, AllowSafeAssignment.
# SupportedStyles: require_parentheses, require_no_parentheses, require_parentheses_when_complex
Style/TernaryParentheses:
Exclude:
@@ -188,7 +249,12 @@ Style/UnneededPercentQ:
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: MaxLineLength.
Style/WhileUntilModifier:
Exclude:
- 'lib/liquid/tags/case.rb'
# Offense count: 640
# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
# URISchemes: http, https
Metrics/LineLength:
Max: 294

View File

@@ -23,7 +23,6 @@ matrix:
- rvm: jruby-head
install:
- gem install rainbow -v 2.2.1
- bundle install
script: bundle exec rake

13
Gemfile
View File

@@ -5,16 +5,23 @@ end
gemspec
gem 'stackprof', platforms: :mri
group :benchmark, :test do
gem 'benchmark-ips'
gem 'memory_profiler'
# install_if -> { RUBY_PLATFORM !~ /mingw|mswin|java/ } do
# gem 'stackprof'
# end
end
group :test do
gem 'rubocop', '~> 0.49.0'
gem 'rubocop', '~> 0.53.0'
platform :mri do
gem 'liquid-c', github: 'Shopify/liquid-c', ref: '9168659de45d6d576fce30c735f857e597fa26f6'
end
end
# gem "byebug", "~> 11.0"
gem "pry", "~> 0.12.2"

View File

@@ -85,6 +85,13 @@ namespace :profile do
end
end
namespace :memory_profile do
desc "Run memory profiler"
task :run do
ruby "./performance/memory_profile.rb"
end
end
desc "Run example"
task :example do
ruby "-w -d -Ilib example/server/server.rb"
@@ -93,3 +100,9 @@ end
task :console do
exec 'irb -I lib -r liquid'
end
task :dump_portable_ast, [:filename] do |_task, args|
require 'liquid/ast_to_portable_json'
ast = Liquid::Template.parse(File.read(args[:filename]))
puts(Liquid::ASTToPortableJSON.dump(ast))
end

View File

@@ -0,0 +1,27 @@
require 'yaml'
require 'json'
require 'liquid'
module Liquid
class ASTToPortableJSON
def self.dump(ast)
yaml = YAML.dump(ast)
yaml.gsub!(/---.*/, '')
yaml.gsub!(/!ruby\/object:(.+)\n(\s+)/, "\n\\2class_name: \\1\n\\2")
bare_hash_ast = YAML.load(yaml)
delete_parse_context(bare_hash_ast)
JSON.pretty_generate(bare_hash_ast)
end
def self.delete_parse_context(bare_hash_ast)
if bare_hash_ast.is_a?(Array)
bare_hash_ast.each { |v| delete_parse_context(v) }
elsif bare_hash_ast.is_a?(Hash)
bare_hash_ast.delete('parse_context')
bare_hash_ast.each { |k, v| delete_parse_context(v) }
end
end
end
end

View File

@@ -115,7 +115,7 @@ module Liquid
end
def check_resources(context, node_output)
context.resource_limits.render_length += node_output.length
context.resource_limits.render_length += node_output.bytesize
return unless context.resource_limits.reached?
raise MemoryError.new("Memory limits exceeded".freeze)
end

View File

@@ -37,7 +37,7 @@ module Liquid
def assign_score_of(val)
if val.instance_of?(String)
val.length
val.bytesize
elsif val.instance_of?(Array) || val.instance_of?(Hash)
sum = 1
# Uses #each to avoid extra allocations.

View File

@@ -25,7 +25,7 @@ module Liquid
def render(context)
output = super
context.scopes.last[@to] = output
context.resource_limits.assign_score += output.length
context.resource_limits.assign_score += output.bytesize
''.freeze
end

View File

@@ -124,14 +124,23 @@ module Liquid
from = if @from == :continue
offsets[@name].to_i
else
context.evaluate(@from).to_i
from_value = context.evaluate(@from)
if from_value.nil?
0
else
Utils.to_integer(from_value)
end
end
collection = context.evaluate(@collection_name)
collection = collection.to_a if collection.is_a?(Range)
limit = context.evaluate(@limit)
to = limit ? limit.to_i + from : nil
limit_value = context.evaluate(@limit)
to = if limit_value.nil?
nil
else
Utils.to_integer(limit_value) + from
end
segment = Utils.slice_collection(collection, from, to)
segment.reverse! if @reversed

View File

@@ -1,3 +1,4 @@
require 'liquid/ast_to_portable_json'
module Liquid
# Templates are central to liquid.
# Interpretating templates is a two step process. First you compile the
@@ -130,6 +131,8 @@ module Liquid
@line_numbers = options[:line_numbers] || @profiling
parse_context = options.is_a?(ParseContext) ? options : ParseContext.new(options)
@root = Document.parse(tokenize(source), parse_context)
@json = ASTToPortableJSON.dump(self)
@warnings = parse_context.warnings
self
end
@@ -198,6 +201,15 @@ module Liquid
context.add_filters(args.pop)
end
unless @truffle_context
environment = context.environments.first
@truffle_context = Polyglot.eval('liquid', @json)
environment.each do |key, value|
@truffle_context[key] = value
end
end
return @truffle_context[:render].call
# Retrying a render resets resource usage
context.resource_limits.reset

View File

@@ -95,16 +95,17 @@ module Liquid
def parse_filter_expressions(filter_name, unparsed_args)
filter_args = []
keyword_args = {}
keyword_args = nil
unparsed_args.each do |a|
if matches = a.match(JustTagAttributes)
keyword_args ||= {}
keyword_args[matches[1]] = Expression.parse(matches[2])
else
filter_args << Expression.parse(a)
end
end
result = [filter_name, filter_args]
result << keyword_args unless keyword_args.empty?
result << keyword_args if keyword_args
result
end

View File

@@ -1,3 +1,4 @@
require 'bundler/setup'
require 'benchmark/ips'
require_relative 'theme_runner'
@@ -5,14 +6,15 @@ Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first
profiler = ThemeRunner.new
Benchmark.ips do |x|
x.time = 10
x.warmup = 5
x.time = 60
x.warmup = 1
puts
puts "Running benchmark for #{x.time} seconds (with #{x.warmup} seconds warmup)."
puts
x.report("parse:") { profiler.compile }
profiler.compile
# x.report("parse:") { profiler.compile }
x.report("render:") { profiler.render }
x.report("parse & render:") { profiler.run }
end

View File

@@ -0,0 +1,26 @@
# frozen_string_literal: true
require 'benchmark/ips'
require 'memory_profiler'
require_relative 'theme_runner'
def profile(phase, &block)
puts
puts "#{phase}:"
puts
report = MemoryProfiler.report(&block)
report.pretty_print(
color_output: true,
scale_bytes: true,
detailed_report: true
)
end
Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first
profiler = ThemeRunner.new
profile("Parsing") { profiler.compile }
profile("Rendering") { profiler.render }

View File

@@ -24,7 +24,7 @@ class ThemeRunner
# Initialize a new liquid ThemeRunner instance
# Will load all templates into memory, do this now so that we don't profile IO.
def initialize
@tests = Dir[__dir__ + '/tests/**/*.liquid'].collect do |test|
@tests = Dir[__dir__ + '/tests/ripen/product.liquid'].collect do |test|
next if File.basename(test) == 'theme.liquid'
theme_path = File.dirname(test) + '/theme.liquid'

View File

@@ -103,6 +103,34 @@ HERE
assert_template_result('3456', '{%for i in array limit: 4 offset: 2 %}{{ i }}{%endfor%}', assigns)
end
def test_limiting_with_invalid_limit
assigns = { 'array' => [1, 2, 3, 4, 5, 6, 7, 8, 9, 0] }
template = <<-MKUP
{% for i in array limit: true offset: 1 %}
{{ i }}
{% endfor %}
MKUP
exception = assert_raises(Liquid::ArgumentError) do
Template.parse(template).render!(assigns)
end
assert_equal("Liquid error: invalid integer", exception.message)
end
def test_limiting_with_invalid_offset
assigns = { 'array' => [1, 2, 3, 4, 5, 6, 7, 8, 9, 0] }
template = <<-MKUP
{% for i in array limit: 1 offset: true %}
{{ i }}
{% endfor %}
MKUP
exception = assert_raises(Liquid::ArgumentError) do
Template.parse(template).render!(assigns)
end
assert_equal("Liquid error: invalid integer", exception.message)
end
def test_dynamic_variable_limiting
assigns = { 'array' => [1, 2, 3, 4, 5, 6, 7, 8, 9, 0] }
assigns['limit'] = 2

View File

@@ -139,6 +139,16 @@ class TemplateTest < Minitest::Test
refute_nil t.resource_limits.assign_score
end
def test_resource_limits_assign_score_counts_bytes_not_characters
t = Template.parse("{% assign foo = 'すごい' %}")
t.render
assert_equal 9, t.resource_limits.assign_score
t = Template.parse("{% capture foo %}すごい{% endcapture %}")
t.render
assert_equal 9, t.resource_limits.assign_score
end
def test_resource_limits_assign_score_nested
t = Template.parse("{% assign foo = 'aaaa' | reverse %}")
@@ -187,6 +197,14 @@ class TemplateTest < Minitest::Test
assert_equal "ababab", t.render
end
def test_render_length_uses_number_of_bytes_not_characters
t = Template.parse("{% if true %}すごい{% endif %}")
t.resource_limits.render_length_limit = 10
assert_equal "Liquid error: Memory limits exceeded", t.render
t.resource_limits.render_length_limit = 18
assert_equal "すごい", t.render
end
def test_default_resource_limits_unaffected_by_render_with_context
context = Context.new
t = Template.parse("{% for a in (1..100) %} {% assign foo = 1 %} {% endfor %}")