Permalink
Browse files

Use 1.9 native XML escaping to speed up html_escape and shush regexp …

…warnings

        length      user     system      total        real
before  6      0.010000   0.000000   0.010000 (  0.012378)
after   6      0.010000   0.000000   0.010000 (  0.012866)
before  60     0.040000   0.000000   0.040000 (  0.046273)
after   60     0.040000   0.000000   0.040000 (  0.036421)
before  600    0.390000   0.000000   0.390000 (  0.390670)
after   600    0.210000   0.000000   0.210000 (  0.209094)
before  6000   3.750000   0.000000   3.750000 (  3.751008)
after   6000   1.860000   0.000000   1.860000 (  1.857901)
  • Loading branch information...
1 parent dc43e40 commit 63cd9432265a32d222353b535d60333c2a6a5125 @jeremy jeremy committed Dec 11, 2011
Showing with 36 additions and 15 deletions.
  1. +36 −15 activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -6,21 +6,42 @@ module Util
HTML_ESCAPE = { '&' => '&amp;', '>' => '&gt;', '<' => '&lt;', '"' => '&quot;' }
JSON_ESCAPE = { '&' => '\u0026', '>' => '\u003E', '<' => '\u003C' }
- # A utility method for escaping HTML tag characters.
- # This method is also aliased as <tt>h</tt>.
- #
- # In your ERB templates, use this method to escape any unsafe content. For example:
- # <%=h @person.name %>
- #
- # ==== Example:
- # puts html_escape("is a > 0 & a < 10?")
- # # => is a &gt; 0 &amp; a &lt; 10?
- def html_escape(s)
- s = s.to_s
- if s.html_safe?
- s
- else
- s.gsub(/[&"><]/n) { |special| HTML_ESCAPE[special] }.html_safe
+ # Detect whether 1.9 can transcode with XML escaping.
+ if '"&gt;&lt;&amp;&quot;"' == ('><&"'.encode('utf-8', :xml => :attr) rescue false)
+ # A utility method for escaping HTML tag characters.
+ # This method is also aliased as <tt>h</tt>.
+ #
+ # In your ERB templates, use this method to escape any unsafe content. For example:
+ # <%=h @person.name %>
+ #
+ # ==== Example:
+ # puts html_escape("is a > 0 & a < 10?")
+ # # => is a &gt; 0 &amp; a &lt; 10?
+ def html_escape(s)
+ s = s.to_s
+ if s.html_safe?
+ s
+ else
+ s.encode(s.encoding, :xml => :attr)[1...-1].html_safe
+ end
+ end
+ else
+ # A utility method for escaping HTML tag characters.
+ # This method is also aliased as <tt>h</tt>.
+ #
+ # In your ERB templates, use this method to escape any unsafe content. For example:
+ # <%=h @person.name %>
+ #
+ # ==== Example:
+ # puts html_escape("is a > 0 & a < 10?")
+ # # => is a &gt; 0 &amp; a &lt; 10?
+ def html_escape(s)
+ s = s.to_s
+ if s.html_safe?
+ s
+ else
+ s.gsub(/[&"><]/n) { |special| HTML_ESCAPE[special] }.html_safe
+ end
end
end

10 comments on commit 63cd943

@jeremy
Member
jeremy commented on 63cd943 Dec 12, 2011

Cut down on object allocations too:

before     4 utf8 chars:      3.4ms elapsed    13 objects per call
 after     4 utf8 chars:      3.9ms elapsed     3 objects per call
before    40 utf8 chars:     16.1ms elapsed    44 objects per call
 after    40 utf8 chars:     10.6ms elapsed     3 objects per call
before   400 utf8 chars:    145.7ms elapsed   404 objects per call
 after   400 utf8 chars:     67.5ms elapsed     3 objects per call
before  4000 utf8 chars:   1429.3ms elapsed  4004 objects per call
 after  4000 utf8 chars:    618.0ms elapsed     3 objects per call
before     6 crap chars:      6.1ms elapsed    15 objects per call
 after     6 crap chars:      5.8ms elapsed     3 objects per call
before    60 crap chars:     21.5ms elapsed    54 objects per call
 after    60 crap chars:     13.4ms elapsed     3 objects per call
before   600 crap chars:    189.5ms elapsed   504 objects per call
 after   600 crap chars:     90.6ms elapsed     3 objects per call
before  6000 crap chars:   1906.6ms elapsed  5004 objects per call
 after  6000 crap chars:    864.9ms elapsed     3 objects per call
@seanwalbran
Contributor

@jeremy Any chance you could share more detail about that benchmark? I seem to be seeing the opposite, and would like to sanity-check:
https://gist.github.com/50ec9ab8ae1e223f3b75

@jeremy
Member
jeremy commented on 63cd943 Jul 31, 2012

@seanwalbran Here's a bench script used to tune that method:

HTML_ESCAPE = { '&' => '&amp;',  '>' => '&gt;',   '<' => '&lt;', '"' => '&quot;' }

def original(s)
  s.gsub(/[&"><]/n) { |special| HTML_ESCAPE[special] }
end

def new_range(s)
  s.encode(s.encoding, :xml => :attr)[1...-1]
end

def new_tr(s)
  s.encode(s.encoding, :xml => :attr).tr!('"', '')
end

def new_slice(s)
  t = s.encode(s.encoding, :xml => :attr)
  t[1, t.size - 1]
end

def new_delete(s)
  s.encode(s.encoding, :xml => :attr).delete!('"')
end

def new_gsub(s)
  s.encode(s.encoding, :xml => :attr).gsub!(/^"|"$/o, '')
end

def bench(method, string, times = 1000)
  t0 = Time.now
  counts = ObjectSpace.count_objects
  o0 = counts[:TOTAL] - counts[:FREE]
  GC.start
  GC.disable
  times.times do
    send method, string
  end
  elapsed = Time.now - t0
  counts = ObjectSpace.count_objects
  allocated = counts[:TOTAL] - counts[:FREE]
  GC.enable
  [elapsed, allocated]
end

s = [192, 60].pack('CC') + '<>&"'

[s, s * 10, s * 100, s * 1000, s * 10000].each do |string|
  %w(original new_range new_tr new_slice new_delete new_gsub).each do |method|
    elapsed, allocated = bench method, string
    puts "%10s %5d chars:  %7.1fms elapsed %10d objects" % [method, string.bytesize, elapsed * 1000, allocated]
  end
end
  original     6 chars:      9.5ms elapsed      19806 objects
 new_range     6 chars:     13.2ms elapsed       7806 objects
    new_tr     6 chars:     10.8ms elapsed       8685 objects
 new_slice     6 chars:     10.7ms elapsed       8688 objects
new_delete     6 chars:     11.7ms elapsed       7685 objects
  new_gsub     6 chars:     16.4ms elapsed      12686 objects
  original    60 chars:     48.2ms elapsed      58669 objects
 new_range    60 chars:     32.7ms elapsed       7671 objects
    new_tr    60 chars:     37.6ms elapsed       8670 objects
 new_slice    60 chars:     30.8ms elapsed       8672 objects
new_delete    60 chars:     30.7ms elapsed       7671 objects
  new_gsub    60 chars:     37.3ms elapsed      12670 objects
  original   600 chars:    437.3ms elapsed     508671 objects
 new_range   600 chars:    224.5ms elapsed       7265 objects
    new_tr   600 chars:    213.1ms elapsed       8264 objects
 new_slice   600 chars:    202.9ms elapsed       8266 objects
new_delete   600 chars:    207.6ms elapsed       7261 objects
  new_gsub   600 chars:    217.8ms elapsed      12260 objects
  original  6000 chars:   4689.4ms elapsed    5008261 objects
 new_range  6000 chars:   2371.1ms elapsed       7245 objects
    new_tr  6000 chars:   2322.2ms elapsed       8245 objects
 new_slice  6000 chars:   2183.1ms elapsed       8247 objects
new_delete  6000 chars:   2132.4ms elapsed       7246 objects
  new_gsub  6000 chars:   2022.4ms elapsed      12245 objects
  original 60000 chars:  83762.4ms elapsed   50008246 objects
 new_range 60000 chars:  31186.8ms elapsed       7246 objects
    new_tr 60000 chars:  21772.3ms elapsed       8246 objects
 new_slice 60000 chars:  21153.8ms elapsed       8248 objects
new_delete 60000 chars:  20834.6ms elapsed       7247 objects
  new_gsub 60000 chars:  20792.1ms elapsed      12246 objects
@rafaelfranca
Member

I got the same result that @jeremy.

@jeremy
Member
jeremy commented on 63cd943 Jul 31, 2012

Testing with a less pathological case, the #encode implementation is slower, but avoids tons of object allocations:

#s = [192, 60].pack('CC') + '<>&"'
s = '<script>alert("foo & bar & baz");</script> Laborum next level organic nihil cardigan.'
  original    85 chars:     12.3ms elapsed      16786 objects
 new_range    85 chars:     32.0ms elapsed       7827 objects
    new_tr    85 chars:     37.2ms elapsed       8680 objects
 new_slice    85 chars:     29.7ms elapsed       8682 objects
new_delete    85 chars:     30.5ms elapsed       7681 objects
  new_gsub    85 chars:     37.3ms elapsed      12680 objects
  original   850 chars:     85.2ms elapsed      88681 objects
 new_range   850 chars:    207.9ms elapsed       7682 objects
    new_tr   850 chars:    208.6ms elapsed       8681 objects
 new_slice   850 chars:    206.8ms elapsed       8683 objects
new_delete   850 chars:    205.5ms elapsed       7682 objects
  new_gsub   850 chars:    221.2ms elapsed      12681 objects
  original  8500 chars:    779.1ms elapsed     808682 objects
 new_range  8500 chars:   1997.9ms elapsed       7666 objects
    new_tr  8500 chars:   1989.3ms elapsed       8665 objects
 new_slice  8500 chars:   1944.1ms elapsed       8650 objects
new_delete  8500 chars:   1971.8ms elapsed       7649 objects
  new_gsub  8500 chars:   2042.0ms elapsed      12648 objects
  original 85000 chars:   8290.6ms elapsed    8008649 objects
 new_range 85000 chars:  19767.9ms elapsed       7649 objects
    new_tr 85000 chars:  20028.2ms elapsed       8241 objects
 new_slice 85000 chars:  19592.8ms elapsed       8243 objects
new_delete 85000 chars:  19884.5ms elapsed       7242 objects
  new_gsub 85000 chars:  20066.4ms elapsed      12241 objects
@cbarton

@rafaelfranca @jeremy I understand that it does create some good amount of objects, however the tradeoff that I am noticing is amount objects created vs less memory and runtime, here are my results adding in memory with the normal string:

def original(s)
  s.gsub(/[&"><]/n) { |special| HTML_ESCAPE[special] }
end

def new_gsub(s)
  s.gsub(/[&"><]/n, HTML_ESCAPE)
end

def new_range(s)
  s.encode(s.encoding, :xml => :attr)[1...-1]
end

original       85 chars:          9.7ms elapsed         27898 objects at     0.2mb
new_range      85 chars:         16.6ms elapsed         18940 objects at     8.6mb
new_gsub       85 chars:          7.4ms elapsed         27870 objects at     0.2mb
original      850 chars:         60.4ms elapsed         99871 objects at     0.9mb
new_range     850 chars:        114.6ms elapsed         18872 objects at    10.3mb
new_gsub      850 chars:         46.5ms elapsed         99870 objects at     0.9mb
original     8500 chars:        551.9ms elapsed        819872 objects at     8.2mb
new_range    8500 chars:       1052.9ms elapsed         18873 objects at    27.5mb
new_gsub     8500 chars:        469.1ms elapsed        819871 objects at     8.2mb
original    85000 chars:       5809.0ms elapsed       8019873 objects at    81.2mb
new_range   85000 chars:      10469.2ms elapsed         18466 objects at   199.2mb
new_gsub    85000 chars:       4551.2ms elapsed       8019464 objects at    81.2mb
original   850000 chars:      84644.3ms elapsed       80019467 objects at  810.7mb
new_range  850000 chars:     105092.8ms elapsed          18876 objects at 1915.8mb
new_gsub   850000 chars:      48107.9ms elapsed       80019873 objects at  810.7mb
@seanwalbran
Contributor

I'm also seeing the more-objects-but-less-runtime-and-memory behavior (assuming the gcdata patch values for memory are legitimate) for the less-pathological case:

$ echo "load '../jeremy-less-pathological-gcdata.rb'" | tools/console
Switch to inspect mode.
load '../jeremy-less-pathological-gcdata.rb'
  original    85 chars:      8.5ms elapsed      72613 objects 0.216 MB     0 collections
 new_range    85 chars:     17.8ms elapsed      63610 objects 8.630 MB     0 collections
    new_tr    85 chars:     17.4ms elapsed      64609 objects 8.519 MB     0 collections
 new_slice    85 chars:     17.4ms elapsed      64612 objects 8.519 MB     0 collections
new_delete    85 chars:     17.2ms elapsed      63611 objects 8.519 MB     0 collections
  new_gsub    85 chars:     21.3ms elapsed      68611 objects 8.752 MB     0 collections
  original   850 chars:     62.3ms elapsed     144613 objects 0.933 MB     0 collections
 new_range   850 chars:    119.1ms elapsed      63612 objects 10.347 MB     0 collections
    new_tr   850 chars:    117.6ms elapsed      64610 objects 9.249 MB     0 collections
 new_slice   850 chars:    114.4ms elapsed      64613 objects 9.249 MB     0 collections
new_delete   850 chars:    116.8ms elapsed      63612 objects 9.249 MB     0 collections
  new_gsub   850 chars:    119.9ms elapsed      68612 objects 10.469 MB     0 collections
  original  8500 chars:    575.1ms elapsed     864614 objects 8.228 MB     0 collections
 new_range  8500 chars:   1115.3ms elapsed      63613 objects 27.513 MB     0 collections
    new_tr  8500 chars:   1131.1ms elapsed      64611 objects 16.544 MB     0 collections
 new_slice  8500 chars:   1113.7ms elapsed      64614 objects 16.544 MB     0 collections
new_delete  8500 chars:   1098.2ms elapsed      63613 objects 16.544 MB     0 collections
  new_gsub  8500 chars:   1125.7ms elapsed      68613 objects 27.635 MB     0 collections
  original 85000 chars:   5709.7ms elapsed    8064615 objects 81.185 MB     0 collections
 new_range 85000 chars:  10839.8ms elapsed      63614 objects 199.174 MB     0 collections
    new_tr 85000 chars:  11027.0ms elapsed      64612 objects 89.501 MB     0 collections
 new_slice 85000 chars:  10814.6ms elapsed      64615 objects 89.501 MB     0 collections
new_delete 85000 chars:  10967.4ms elapsed      63614 objects 89.501 MB     0 collections
  new_gsub 85000 chars:  11010.7ms elapsed      68614 objects 199.296 MB     0 collections
true

test source: https://gist.github.com/8c690d9ae08357f2b3cd

@seanwalbran
Contributor

With an extremely simpleminded string construction, looks like the runtime crossover between the two approaches happens roughly where 25% of the string content needs escaping:

#pct-escapable  original/ms        new_range/ms         string/100
100             522.8              192.2                &&&&&&&&&&
90              489.0              199.5                a&&&&&&&&&
80              488.4              190.3                aa&&&&&&&&
70              372.8              176.5                aaa&&&&&&&
60              325.4              171.3                aaaa&&&&&&
50              270.3              166.2                aaaaa&&&&&
40              246.2              159.5                aaaaaa&&&&
30              174.0              152.1                aaaaaaa&&&
20              121.0              140.7                aaaaaaaa&&
10               64.3              133.2                aaaaaaaaa&
 0                2.4              122.3                aaaaaaaaaa 
@spastorino
Member

For another reason I have reverted that code back so problem fixed :trollface: b6ab441

@jeremy
Member
jeremy commented on 63cd943 Aug 1, 2012

Voilà ❤️

Please sign in to comment.