ssl_requirement icon indicating copy to clipboard operation
ssl_requirement copied to clipboard

Don't use 'rescue' unecessarily (making url_for faster)

Open jdelStrother opened this issue 11 years ago • 0 comments

Hi - I was profiling a slow page with a lot of url_fors, and found the non_ssl_host method showing up in my profile. Throwing exceptions & catching them is relatively slow - how about the attached commit?

If you want the benchmarks -

require 'bundler/setup'
require 'action_dispatch'
require 'active_support/core_ext'
require 'benchmark/ips'
require_relative 'lib/ssl_requirement'

routes = ActionDispatch::Routing::RouteSet.new
routes.default_url_options[:host] = 'test.host'
routes.draw do
  match ':controller(/:action(/:id(.:format)))'
end

routes.url_for(controller: 'c', action: 'a')

Benchmark.ips do |x|
  x.report("with exception+rescue") { routes.url_for(controller: 'c', action: 'a') }
end

# patch up SslRequirement to avoid the throw/rescue 
module SslRequirement
  self.ssl_host = nil
  self.non_ssl_host = nil
  def self.ssl_host
    determine_host(@@ssl_host)
  end
  def self.non_ssl_host
    determine_host(@@non_ssl_host)
  end
end
Benchmark.ips do |x|
  x.report("without exception+rescue") { routes.url_for(controller: 'c', action: 'a') }
end
Calculating -------------------------------------
with exception+rescue
                          1038 i/100ms
-------------------------------------------------
with exception+rescue
                        11491.7 (±4.3%) i/s -      58128 in   5.068276s
Calculating -------------------------------------
without exception+rescue
                          1208 i/100ms
-------------------------------------------------
without exception+rescue
                        13492.2 (±4.6%) i/s -      67648 in   5.025101s

jdelStrother avatar Jul 25 '13 14:07 jdelStrother