Optimize parsing of parameters in include tag

[ashmaroli]

Summary

• Replace while..do loop generating MatchData with faster String#scan and a block to use meaningful local variable names.
• Since String#gsub duplicates the input string irrespective of a match, test if substitution is necessary before calling String#gsub. (The overhead for prior testing is negligible.)

[DirtyF]

How can we measure the gain here?

[ashmaroli]

How can we measure the gain here?

@DirtyF I've added a benchmark script to this branch as well.. You can run it by checking the PR branch out locally:

git fetch upstream pull/8192/head:optimize-includes-params-parsing
git checkout optimize-includes-params-parsing
ruby benchmark/parse-include-tag-params.rb

[DirtyF]

I am missing some gems here, can you copy-paste the output of the benchmark please?

[ashmaroli]

@DirtyF ref: https://travis-ci.org/github/ashmaroli/jekyll/builds/689225401

$ benchmark/parse-include-tag-params.rb
old w/ escaping ---------------------------------

MARKUP: foo=bar lorem="ipsum "dolor"" alpha='beta 'gamma''
RESULT: {"foo"=>"The quick brown fox", "lorem"=>"ipsum "dolor"", "alpha"=>"beta 'gamma'"}

Total allocated: 2.85 kB (31 objects)
Total retained:  440.00 B (3 objects)

new w/ escaping ---------------------------------

MARKUP: foo=bar lorem="ipsum "dolor"" alpha='beta 'gamma''
RESULT: {"foo"=>"The quick brown fox", "lorem"=>"ipsum "dolor"", "alpha"=>"beta 'gamma'"}

Total allocated: 1.85 kB (21 objects)
Total retained:  0 B (0 objects)

old no escaping ---------------------------------

MARKUP: foo=bar lorem="ipsum 'dolor'" alpha='beta "gamma"'
RESULT: {"foo"=>"The quick brown fox", "lorem"=>"ipsum 'dolor'", "alpha"=>"beta "gamma""}

Total allocated: 2.15 kB (26 objects)
Total retained:  480.00 B (4 objects)

new no escaping ---------------------------------

MARKUP: foo=bar lorem="ipsum 'dolor'" alpha='beta "gamma"'
RESULT: {"foo"=>"The quick brown fox", "lorem"=>"ipsum 'dolor'", "alpha"=>"beta "gamma""}

Total allocated: 1.35 kB (15 objects)
Total retained:  0 B (0 objects)

Warming up --------------------------------------
  old + esc    11.450k i/100ms
  new + esc    13.115k i/100ms

Calculating -------------------------------------
  old + esc    111.891k (± 2.2%) i/s -    561.050k in   5.016667s
  new + esc    134.800k (± 3.4%) i/s -    681.980k in   5.065103s

Comparison:
  new + esc:   134800.4 i/s
  old + esc:   111891.2 i/s - 1.20x  (± 0.00) slower

Warming up --------------------------------------
  old - esc    13.165k i/100ms
  new - esc    19.465k i/100ms

Calculating -------------------------------------
  old - esc    129.218k (± 6.1%) i/s -    645.085k in   5.010215s
  new - esc    201.098k (± 5.0%) i/s -      1.012M in   5.046792s

Comparison:
  new - esc:   201098.3 i/s
  old - esc:   129218.0 i/s - 1.56x  (± 0.00) slower

[ashmaroli]

@jekyllbot: merge +fix