Dupe and escape span values only if necessary #1244
Conversation
|
Would you mind benchmarking on a very long comment that contains escape characters? |
If you provide a sample string, I shall use that. |
|
You can find some fairly long comments in |
|
@jneen Okay, updated the above benchmark script with the following: require 'yaml'
# set up a long string text
PLAINTEXT = YAML.load(<<-TEXT)
comment: >
@param [Proc] callback a block that will be evaluated in the context of the lexer
if `re` matches. This block has access to a number of lexer methods, including
`RegexLexer#push`, `RegexLexer#pop!`, `RegexLexer#token` et `RegexLexer#delegate`.
The first argument can be used to access the match groups.
-endcomment-
TEXT
RAWTEXT = YAML.load(<<-TEXT)
comment: >
@param [Proc] callback a block that will be evaluated in the context of the lexer
if `re` matches. This block has access to a number of lexer methods, including
#<RegexLexer#push>, #<RegexLexer#pop!>, #<RegexLexer#token>, & #<RegexLexer#delegate>.
The first argument can be used to access the match groups.
<endcomment>
TEXT
[
['plain', PLAINTEXT['comment']],
['specl', RAWTEXT['comment']]
].each do |key, value|
Benchmark.ips do |x|
...The result are similar to previous ones: |
|
Okay! I was thinking about the occasional pages-long comment that you will see on gitlab, etc, but honestly I think that's rare enough that the tradeoff is worth it. If you could add a comment explaining that it's done that way for performance reasons, then it's +1 to merge from me. |
Rationale
String.gsubcreates a copy of every string passed to it irrespective of whether a substitution occurs.This can lead to unnecessary memory consumption.
Summary
Use a private helper to substitute values conditionally.
Invoke
String.gsubonly if the string matches our pattern beforehand. Otherwise, return the given string itself.Impact
Local tests show a reduction in values reported by the
profile_memorytask.Report diff of Travis logs
Basic benchmarks show that the proposed route is faster for strings that don't need escaping, but equally slower for strings that do need escaping.
Benchmark script
Benchmark results
Warming up -------------------------------------- esc plain 50.502k i/100ms cond_esc plain 52.509k i/100ms Calculating ------------------------------------- esc plain 944.667k (A± 1.3%) i/s - 4.747M in 5.026118s cond_esc plain 1.056M (A± 0.4%) i/s - 5.303M in 5.024585s Comparison: cond_esc plain: 1055512.1 i/s esc plain: 944666.6 i/s - 1.12x slower Warming up -------------------------------------- esc specl 13.415k i/100ms cond_esc specl 12.547k i/100ms Calculating ------------------------------------- esc specl 169.639k (A± 0.7%) i/s - 858.560k in 5.061389s cond_esc specl 154.341k (A± 1.8%) i/s - 777.914k in 5.042006s Comparison: esc specl: 169638.6 i/s cond_esc specl: 154341.2 i/s - 1.10x slower