From 8bf0e7dfaee0561597fa968b8baf76b5974d4d3b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 9 Oct 2020 15:54:25 -0700 Subject: [PATCH] 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 --- lib/liquid/standardfilters.rb | 19 ++++++++++++++----- test/integration/standard_filter_test.rb | 4 ++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index 0f4c914..d39211e 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -89,13 +89,22 @@ 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 + l = words + l = 1 if l <= 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 + # `l` times. Also ignore leading whitespace + str = input[/\A[ ]*(?:[^ ]*[ ]+){#{l}}/] + + 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. diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb index 76ab462..69c57ab 100644 --- a/test/integration/standard_filter_test.rb +++ b/test/integration/standard_filter_test.rb @@ -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” x 5.5” x 10” high) baskets fit inside one large basket (13”...', @@ -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