From b01456ac68dabeda6dc99805f3d7a127bd7be4f1 Mon Sep 17 00:00:00 2001 From: schneems Date: Wed, 1 Oct 2014 13:36:20 -0500 Subject: [PATCH] Minimize object creation on middleware call. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch isn't nearly as invasive as #773. Also it was tested against itself, not in a full Rails app. I'm contributing the perf scripts I wrote in another PR. Notice that by memorizing methods, not only are we reducing objects created in omniauth, but also in hashie due to fewer method calls to various omniauth options. ## Bench Before patch ``` Calculating ------------------------------------- ips 470 i/100ms ------------------------------------------------- ips 4877.1 (±10.1%) i/s - 24440 in 5.061663s 2.1.2 ~/documents/projects/omniauth (master) $ be rake perf:ips Calculating ------------------------------------- ips 467 i/100ms ------------------------------------------------- ips 4840.9 (±14.5%) i/s - 23817 in 5.068649s 2.1.2 ~/documents/projects/omniauth (master) $ be rake perf:ips Calculating ------------------------------------- ips 465 i/100ms ------------------------------------------------- ips 4799.5 (±11.8%) i/s - 23715 in 5.019191s ``` After Patch ``` $ be rake perf:ips Calculating ------------------------------------- ips 631 i/100ms ------------------------------------------------- ips 6269.1 (±11.8%) i/s - 30919 in 5.014849s 2.1.2 ~/documents/projects/omniauth (master) $ be rake perf:ips Calculating ------------------------------------- ips 615 i/100ms ------------------------------------------------- ips 6207.8 (±10.3%) i/s - 31365 in 5.106172s 2.1.2 ~/documents/projects/omniauth (master) $ be rake perf:ips Calculating ------------------------------------- ips 522 i/100ms ------------------------------------------------- ips 6155.8 (±9.3%) i/s - 30798 in 5.048455s ``` ## Memory Bench 100 requests without patch ``` allocated objects by gem ----------------------------------- hashie-3.3.1 x 6901 rack-1.5.2 x 5300 omniauth/lib x 3637 <==================== ruby-2.1.2/lib x 301 other x 300 ``` 100 requests with patch ``` allocated objects by gem ----------------------------------- rack-1.5.2 x 4800 hashie-3.3.1 x 4701 omniauth/lib x 1037 <==================== ruby-2.1.2/lib x 301 other x 300 ``` Results in `(3637 - 1037 )/ 3637.0 * 100 # => 71 %` less objects. --- lib/omniauth/strategy.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/omniauth/strategy.rb b/lib/omniauth/strategy.rb index 899f753..fb228f9 100644 --- a/lib/omniauth/strategy.rb +++ b/lib/omniauth/strategy.rb @@ -377,23 +377,27 @@ module OmniAuth end def request_path - options[:request_path].is_a?(String) ? options[:request_path] : "#{path_prefix}/#{name}" + @request_path ||= options[:request_path].is_a?(String) ? options[:request_path] : "#{path_prefix}/#{name}" end def callback_path - path = options[:callback_path] if options[:callback_path].is_a?(String) - path ||= current_path if options[:callback_path].respond_to?(:call) && options[:callback_path].call(env) - path ||= custom_path(:request_path) - path ||= "#{path_prefix}/#{name}/callback" - path + @callback_path ||= begin + path = options[:callback_path] if options[:callback_path].is_a?(String) + path ||= current_path if options[:callback_path].respond_to?(:call) && options[:callback_path].call(env) + path ||= custom_path(:request_path) + path ||= "#{path_prefix}/#{name}/callback" + path + end end def setup_path options[:setup_path] || "#{path_prefix}/#{name}/setup" end + CURRENT_PATH_REGEX = /\/$/ + EMPTY_STRING = ''.freeze def current_path - request.path_info.downcase.sub(/\/$/, '') + @current_path ||= request.path_info.downcase.sub(CURRENT_PATH_REGEX, EMPTY_STRING) end def query_string