Skip to content

Commit

Permalink
Minimize object creation on middleware call.
Browse files Browse the repository at this point in the history
This patch isn't nearly as invasive as omniauth#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.
  • Loading branch information
schneems committed Oct 1, 2014
1 parent 18f8fb8 commit 7f0541f
Showing 1 changed file with 11 additions and 7 deletions.
18 changes: 11 additions & 7 deletions lib/omniauth/strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,23 +377,27 @@ def custom_path(kind)
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
Expand Down

0 comments on commit 7f0541f

Please sign in to comment.