Compare commits

..

9 Commits

Author SHA1 Message Date
Aaron Patterson
94df9a8f52 Update lib/liquid/standardfilters.rb
Co-authored-by: Dylan Thacker-Smith <dylan.smith@shopify.com>
2020-11-16 10:23:20 -08:00
Aaron Patterson
8bf0e7dfae Reduce allocations in truncatewords
I don't know if this is a bottleneck IRL, but while doing some
performance testing I noticed that this method allocates a bunch of
objects and I decided to fix it.

Here is the benchmark I used:

```ruby
require_relative 'theme_runner'

Liquid::Template.error_mode = ARGV.first.to_sym if ARGV.first
profiler = ThemeRunner.new

def count_alloc
  before = GC.stat(:total_allocated_objects)
  yield
  GC.stat(:total_allocated_objects) - before
end

profiler.render # heat
p count_alloc { profiler.render }
p count_alloc { profiler.render }
p count_alloc { profiler.render }
```

Before this change:

```
[aaron@tc-lan-adapter ~/g/liquid (master)]$ ruby -I lib performance/run_once.rb
15753
15750
15752
```

After this change:

```
[aaron@tc-lan-adapter ~/g/liquid (master)]$ ruby -I lib performance/run_once.rb
14015
14010
14011
```

About an 11% reduction in allocations for this test.  I also added some
tests around the current behavior of the method so we can be more sure
the replacement behaves the same way
2020-10-09 16:01:12 -07:00
Dylan Thacker-Smith
077bf2a409 Test reporting of liquid error for filter call with wrong number of arguments (#1311) 2020-10-08 11:55:40 -04:00
Dylan Thacker-Smith
1a3e38c018 Merge pull request #1310 from Shopify/only-integration-test-liquid-c
Fix liquid-c integration testing
2020-10-08 11:52:50 -04:00
Dylan Thacker-Smith
e495f75cc2 Remove support for ruby 2.4, which is no longer supported upstream 2020-10-08 09:48:16 -04:00
Dylan Thacker-Smith
e781449c36 Remove root directory from library search path for tests
It isn't in the gemspec's require_path, so we shouldn't add any dependence
on it.
2020-10-08 01:53:11 -04:00
Dylan Thacker-Smith
7eb03ea198 Only test liquid-c integration using the integration tests 2020-10-08 01:52:40 -04:00
Peter Zhu
bd34cd5613 Merge pull request #1308 from Shopify/pz-gh-actions
Use GitHub Actions for CI
2020-10-07 14:38:15 -04:00
Peter Zhu
c28d455f7b Use GitHub Actions for CI 2020-10-07 13:29:39 -04:00
11 changed files with 78 additions and 79 deletions

40
.github/workflows/liquid.yml vendored Normal file
View File

@@ -0,0 +1,40 @@
name: Liquid
on: [push]
jobs:
test:
runs-on: ubuntu-latest
strategy:
matrix:
entry:
- { ruby: 2.5, allowed-failure: false }
- { ruby: 2.6, allowed-failure: false }
- { ruby: 2.7, allowed-failure: false }
- { ruby: ruby-head, allowed-failure: true }
name: test (${{ matrix.entry.ruby }})
steps:
- uses: actions/checkout@v2
- uses: ruby/setup-ruby@v1
with:
ruby-version: ${{ matrix.entry.ruby }}
- uses: actions/cache@v1
with:
path: vendor/bundle
key: ${{ runner.os }}-gems-${{ hashFiles('Gemfile') }}
restore-keys: ${{ runner.os }}-gems-
- run: bundle install --jobs=3 --retry=3 --path=vendor/bundle
- run: bundle exec rake
continue-on-error: ${{ matrix.entry.allowed-failure }}
memory_profile:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: ruby/setup-ruby@v1
with:
ruby-version: 2.7
- uses: actions/cache@v1
with:
path: vendor/bundle
key: ${{ runner.os }}-gems-${{ hashFiles('Gemfile') }}
restore-keys: ${{ runner.os }}-gems-
- run: bundle install --jobs=3 --retry=3 --path=vendor/bundle
- run: bundle exec rake memory_profile:run

View File

@@ -1,26 +0,0 @@
language: ruby
cache: bundler
rvm:
- 2.4
- 2.5
- 2.6
- &latest_ruby 2.7
- ruby-head
matrix:
include:
- rvm: *latest_ruby
script: bundle exec rake memory_profile:run
name: Profiling Memory Usage
allow_failures:
- rvm: ruby-head
branches:
only:
- master
- gh-pages
- /.*-stable/
notifications:
disable: true

View File

@@ -9,11 +9,17 @@ task(default: [:test, :rubocop])
desc('run test suite with default parser')
Rake::TestTask.new(:base_test) do |t|
t.libs << '.' << 'lib' << 'test'
t.libs << 'lib' << 'test'
t.test_files = FileList['test/{integration,unit}/**/*_test.rb']
t.verbose = false
end
Rake::TestTask.new(:integration_test) do |t|
t.libs << 'lib' << 'test'
t.test_files = FileList['test/integration/**/*_test.rb']
t.verbose = false
end
desc('run test suite with warn error mode')
task :warn_test do
ENV['LIQUID_PARSER_MODE'] = 'warn'
@@ -40,12 +46,12 @@ task :test do
ENV['LIQUID_C'] = '1'
ENV['LIQUID_PARSER_MODE'] = 'lax'
Rake::Task['base_test'].reenable
Rake::Task['base_test'].invoke
Rake::Task['integration_test'].reenable
Rake::Task['integration_test'].invoke
ENV['LIQUID_PARSER_MODE'] = 'strict'
Rake::Task['base_test'].reenable
Rake::Task['base_test'].invoke
Rake::Task['integration_test'].reenable
Rake::Task['integration_test'].invoke
end
end

View File

@@ -84,7 +84,6 @@ require 'liquid/usage'
require 'liquid/register'
require 'liquid/static_registers'
require 'liquid/template_factory'
require 'liquid/optimizer'
# Load all the tags of the standard library
#

View File

@@ -1,19 +0,0 @@
# frozen_string_literal: true
module Liquid
class Optimizer
class << self
attr_accessor :enabled
def optimize_variable(node)
return unless enabled
# Turn chained `| append: "..."| append: "..."`, into a single `append_all: [...]`
if node.filters.size > 1 && node.filters.all? { |f, _| f == "append" }
node.filters = [["append_all", node.filters.map { |f, (arg)| arg }]]
end
end
end
self.enabled = true
end
end

View File

@@ -89,13 +89,21 @@ module Liquid
def truncatewords(input, words = 15, truncate_string = "...")
return if input.nil?
wordlist = input.to_s.split
words = Utils.to_integer(words)
words = Utils.to_integer(words)
l = words - 1
l = 0 if l < 0
words = 1 if words <= 0
wordlist.length > l ? wordlist[0..l].join(" ").concat(truncate_string.to_s) : input
# Scan for non-space characters followed by one or more space characters
# `words` times. Also ignore leading whitespace
str = input[/\A[ ]*(?:[^ ]*[ ]+){#{words}}/]
if str
str.strip! # Remove trailing space
str.gsub!(/[ ]{2,}/, " ") # Shrink multiple spaces to one space
str.concat(truncate_string.to_s)
else
input
end
end
# Split input string into an array of substrings separated by given pattern.
@@ -281,10 +289,6 @@ module Liquid
input.to_s + string.to_s
end
def append_all(input, *items)
input.to_s + items.join
end
def concat(input, array)
unless array.respond_to?(:to_ary)
raise ArgumentError, "concat filter requires an array argument"

View File

@@ -31,8 +31,6 @@ module Liquid
@line_number = parse_context.line_number
parse_with_selected_parser(markup)
Optimizer.optimize_variable(self)
end
def raw

View File

@@ -17,7 +17,7 @@ Gem::Specification.new do |s|
s.license = "MIT"
# s.description = "A secure, non-evaling end user template engine with aesthetic markup."
s.required_ruby_version = ">= 2.4.0"
s.required_ruby_version = ">= 2.5.0"
s.required_rubygems_version = ">= 1.3.7"
s.test_files = Dir.glob("{test}/**/*")

View File

@@ -153,6 +153,15 @@ class FiltersTest < Minitest::Test
# tap still treated as a non-existent filter
assert_equal("1000", Template.parse("{{var | tap}}").render!('var' => 1000))
end
def test_liquid_argument_error
source = "{{ '' | size: 'too many args' }}"
exc = assert_raises(Liquid::ArgumentError) do
Template.parse(source).render!
end
assert_match(/\ALiquid error: wrong number of arguments /, exc.message)
assert_equal(exc.message, Template.parse(source).render)
end
end
class FiltersInTemplate < Minitest::Test

View File

@@ -168,6 +168,8 @@ class StandardFiltersTest < Minitest::Test
def test_truncatewords
assert_equal('one two three', @filters.truncatewords('one two three', 4))
assert_equal('one two...', @filters.truncatewords('one two three', 2))
assert_equal('one two...', @filters.truncatewords('one two three', 2))
assert_equal('one two...', @filters.truncatewords(' one two three', 2))
assert_equal('one two three', @filters.truncatewords('one two three'))
assert_equal(
'Two small (13&#8221; x 5.5&#8221; x 10&#8221; high) baskets fit inside one large basket (13&#8221;...',
@@ -175,6 +177,8 @@ class StandardFiltersTest < Minitest::Test
)
assert_equal("测试测试测试测试", @filters.truncatewords('测试测试测试测试', 5))
assert_equal('one two1', @filters.truncatewords("one two three", 2, 1))
assert_equal('one1', @filters.truncatewords("one two three", 0, 1))
assert_equal('one1', @filters.truncatewords("one two three", -1, 1))
end
def test_strip_html

View File

@@ -1,16 +0,0 @@
# frozen_string_literal: true
require 'test_helper'
class OptimizerUnitTest < Minitest::Test
include Liquid
def test_combines_append_filters
optimizer = Optimizer.new
var = Variable.new('hello | append: "a" | append: b', ParseContext.new)
var = optimizer.optimize(var)
assert_equal([
['append_all', ["a", VariableLookup.new("b")]]
], var.filters)
end
end