Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved performance of flush_cache and prime_cache when parameters are passed to them. #2

Merged

Conversation

jrafanie
Copy link
Contributor

The more methods defined on the class and the more methods that are asked to flush, the slower the code becomes because Array Math is performed in a loop. The attached pull requests minimizes this.

From the original rails ticket from @Fryguy: https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/5517-activesupportmemoizable-flush_cache-is-very-slow4#ticket-5517-3:

"After more playing around with this, I realized the issue is not with calling methods + private_methods + protected_methods directly, but instead they are being called IN A LOOP of the parameters passed in. So, if you pass, for example, 20 method names to flush cache, it will call methods + private_methods + protected_methods 20 times. This is why I was seeing such horrible performance. flush_cache by itself wasn't the problem, it was flush_cache with parameters...the more the worse."

Before patch:

>> Benchmark.realtime { 100.times { v.unmemoize_all }}
=> 1.62846398353577
>> Benchmark.realtime { 100.times { v.flush_cache *MEMOIZED_METHODS }}
=> 40.7326259613037

After patch:

>> Benchmark.realtime { 100.times { v.unmemoize_all }}
=> 0.400160074234009
>> Benchmark.realtime { 100.times { v.flush_cache *MEMOIZED_METHODS }}
=> 0.0130050182342529

See this branch for a slow test example without this optimization: https://github.com/ManageIQ/memoist/tree/slow_test_without_optimization

ruby-1.9.3-p194-falcon@328 (slow_test_without_optimization) ~/Code/github/ManageIQ/memoist $ rake test                                             
/Users/joerafaniello/.rvm/rubies/ruby-1.9.3-p194-falcon/bin/ruby -I"lib:test" -I"/Users/joerafaniello/.rvm/gems/ruby-1.9.3-p194-falcon@global/gems/rake-0.9.2.2/lib" "/Users/joerafaniello/.rvm/gems/ruby-1.9.3-p194-falcon@global/gems/rake-0.9.2.2/lib/rake/rake_test_loader.rb" "test/memoist_test.rb" 
Run options: 

# Running tests:

....................

Finished tests in 0.623739s, 32.0647 tests/s, 131.4652 assertions/s.

20 tests, 82 assertions, 0 failures, 0 errors, 0 skips

And here's a branch with the same test and the optimization:
https://github.com/ManageIQ/memoist/tree/flush_and_prime_cache_optimization_with_slow_test_example

ruby-1.9.3-p194-falcon@328 (flush_and_prime_cache_optimization_with_slow_test_example) ~/Code/github/ManageIQ/memoist $ rake test                                                             
/Users/joerafaniello/.rvm/rubies/ruby-1.9.3-p194-falcon/bin/ruby -I"lib:test" -I"/Users/joerafaniello/.rvm/gems/ruby-1.9.3-p194-falcon@global/gems/rake-0.9.2.2/lib" "/Users/joerafaniello/.rvm/gems/ruby-1.9.3-p194-falcon@global/gems/rake-0.9.2.2/lib/rake/rake_test_loader.rb" "test/memoist_test.rb" 
Run options: 

# Running tests:

....................

Finished tests in 0.006318s, 3165.5587 tests/s, 12978.7908 assertions/s.

20 tests, 82 assertions, 0 failures, 0 errors, 0 skips

@matthewrudy
Copy link
Owner

Cool thanks Joe.
The tests all pass and the diff seems to make sense,
so I'll give it a merge!

matthewrudy added a commit that referenced this pull request Aug 15, 2012
Improved performance of flush_cache and prime_cache when parameters are passed to them.
@matthewrudy matthewrudy merged commit 6b2bb95 into matthewrudy:master Aug 15, 2012
jrafanie added a commit to jrafanie/rails that referenced this pull request Nov 5, 2014
…meters are passed to them."

This reverts commit 76ba734.

We'll use the memoist gem instead since it contains what we need:
matthewrudy/memoist#2
jrafanie added a commit to jrafanie/manageiq that referenced this pull request Nov 5, 2014
We upstreamed our patch to memoist here:
matthewrudy/memoist#2

It has been available in memoist since:
v0.11.0 v0.10.0 v0.9.3 v0.9.2 0.9.0 0.2.0

We can now drop our rails fork patch:
ManageIQ/rails@76ba734

Fixes ManageIQ#538
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants