jamesmartin / inline_svg

Embed SVG documents in your Rails views and style them with CSS
MIT License
716 stars 73 forks source link

Improve CachedAssetFile #118

Closed stevendaniels closed 4 years ago

stevendaniels commented 4 years ago

Closes #108

Some speed improvements for the CachedAssetFile class. Doing the key length comparisons once and removing the generated regexes drastically sped up the implementation.

Here are the benchmarks

Benchmark.ips do |x|
  x.report("original_cached")    { cached.named(svgs.sample) }
  x.report("noncached") { noncached.named(svgs.sample) }
  x.report("new_cached") { supercached.named(svgs.sample) }

  x.compare!
end
Warming up --------------------------------------
     original_cached    35.000  i/100ms
           noncached    82.000  i/100ms
          new_cached     3.024k i/100ms
Calculating -------------------------------------
     original_cached    383.179  (±15.9%) i/s -      1.890k in   5.088049s
           noncached      1.041k (±18.2%) i/s -      5.002k in   5.022027s
          new_cached     30.145k (±15.6%) i/s -    145.152k in   5.043947s

Comparison:
          new_cached:    30145.5 i/s
           noncached:     1040.6 i/s - 28.97x  slower
     original_cached:      383.2 i/s - 78.67x  slower

This isn't a perfect benchmark (it's using .sample), but the cache is clearly faster in the new implementation.

stevendaniels commented 4 years ago

Using include? instead of end_with? fixes the broken test, but it slightly slower. It's still faster than un-cached.

Warming up --------------------------------------
          old_cached    35.000  i/100ms
           noncached   104.000  i/100ms
          new_cached     1.213k i/100ms
Calculating -------------------------------------
              cached    394.127  (±14.0%) i/s -      1.925k in   5.012903s
           noncached      1.166k (±17.6%) i/s -      5.616k in   5.005705s
          new_cached     15.572k (±13.3%) i/s -     75.206k in   5.062016s

Comparison:
         new_cached:    15571.8 i/s
          noncached:     1165.9 i/s - 13.36x  slower
             cached:      394.1 i/s - 39.51x  slower
jamesmartin commented 4 years ago

Thanks for opening this pull request, @stevendaniels. This looks like a worthwhile improvement! ✨