Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

paperclip 4.2.1 memory/object use quite high #1882

Closed
bitcoder opened this issue Jun 2, 2015 · 5 comments
Closed

paperclip 4.2.1 memory/object use quite high #1882

bitcoder opened this issue Jun 2, 2015 · 5 comments

Comments

@bitcoder
Copy link

bitcoder commented Jun 2, 2015

I'm using Rails 4.2.0 with Ruby 2.1.5 in an application that usees Paperclip 4.2.1.
We have been diagnosing memory allocation and consumption using derailed_benchmarks (v1.0.0).
One thing that we noticed is that the memory/object allocation is quite high for paperclip.
Bellow is the output of derailed benchmarks for object use.

➜  $ USE_AUTH=true TEST_COUNT=5 PATH_TO_HIT=/checkinout_tasks  RUBY_GC_HEAP_OLDOBJECT_LIMIT_FACTOR=2   USE_SERVER=puma bundle exec derailed  exec perf:objects
Booting: production
Endpoint: "/checkinout_tasks"
Auth: true
/Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/derailed_benchmarks-1.0.0/lib/derailed_benchmarks/tasks.rb:70: warning: already initialized constant DERAILED_APP
/Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/derailed_benchmarks-1.0.0/lib/derailed_benchmarks/tasks.rb:24: warning: previous definition of DERAILED_APP was here
Port: 3486
Server: "puma"
Puma 2.11.1 starting...
* Min threads: 0, max threads: 16
* Environment: none
* Listening on tcp://0.0.0.0:3486
Running 5 times
Total allocated 939215
Total retained 316

allocated memory by gem
-----------------------------------
  46320275  activerecord-4.2.0
  43261760  paperclip-4.2.1
...
allocated memory by file
-----------------------------------
  41315680  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/paperclip-4.2.1/lib/paperclip/interpolations.rb
  20244395  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/connection_adapters/postgresql/database_statements.rb
   9717400  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/result.rb
   5566040  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activesupport-4.2.0/lib/active_support/core_ext/string/output_safety.rb
allocated memory by location
-----------------------------------
  33945520  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/paperclip-4.2.1/lib/paperclip/interpolations.rb:33
  20097300  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/connection_adapters/postgresql/database_statements.rb:168
   9532960  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/result.rb:116
   7053440  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/paperclip-4.2.1/lib/paperclip/interpolations.rb:23
   3966760  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/attribute_methods.rb:359

We trace the source of the "problem" to paperclip's interpolation.rb, namely the "self.instance_methods(false).sort" of self.all method, and the gsub line inside the all.reverse.inject iteration in self.interpolate method.
We changed this method a bit (it works but it is not a real and clean patch) so the memory use is quite lower now. Even so, I guess that it could be even lower but for that the internal iteration would have to be rethinked.

    # Returns a sorted list of all interpolations.
    def self.all
      @all_tags ||= self.instance_methods(false).sort
    end

    # Perform the actual interpolation. Takes the pattern to interpolate
    # and the arguments to pass, which are the attachment and style name.
    # You can pass a method name on your record as a symbol, which should turn
    # an interpolation pattern for Paperclip to use.
    def self.interpolate pattern, *args
      pattern = args.first.instance.send(pattern) if pattern.kind_of? Symbol
      @h={} if @h.nil?
      all.reverse.inject(pattern) do |result, tag|
        @h[tag] ||=/:#{tag}/
        result.gsub(@h[tag]) do |match|
          send( tag, *args )
        end

      end
    end

After the change, we obtained these results, that are quite lower (aprox 1/10th) but, as I said, far from perfect.

➜  $ USE_AUTH=true TEST_COUNT=5 PATH_TO_HIT=/tasjs  RUBY_GC_HEAP_OLDOBJECT_LIMIT_FACTOR=2   USE_SERVER=puma bundle exec derailed  exec perf:objects

Booting: production
Endpoint: "/checkinout_tasks"
Auth: true
/Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/derailed_benchmarks-1.0.0/lib/derailed_benchmarks/tasks.rb:70: warning: already initialized constant DERAILED_APP
/Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/derailed_benchmarks-1.0.0/lib/derailed_benchmarks/tasks.rb:24: warning: previous definition of DERAILED_APP was here
Port: 3460
Server: "puma"
Puma 2.11.1 starting...
* Min threads: 0, max threads: 16
* Environment: none
* Listening on tcp://0.0.0.0:3460
Running 5 times
Total allocated 587596
Total retained 338

allocated memory by gem
-----------------------------------
  46315851  activerecord-4.2.0
   9293065  activesupport-4.2.0
   4892860  paperclip-4.2.1
allocated memory by location
-----------------------------------
  20097300  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/connection_adapters/postgresql/database_statements.rb:168
   9532960  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/result.rb:116
   3966760  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/attribute_methods.rb:359
   3790370  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activesupport-4.2.0/lib/active_support/core_ext/string/output_safety.rb:168
   2630060  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/paperclip-4.2.1/lib/paperclip/interpolations.rb:66

We noticed that the methods "all" and "interpolate" are being called many many times, apparently whenever their associated activerecord models are loaded.
Guess that this part needs some kind of otimization maybe?

@tute
Copy link
Contributor

tute commented Jun 2, 2015

This fix is in master: #1873. Would that fix your use case? Thanks for the detailed report!

@bitcoder
Copy link
Author

bitcoder commented Jun 2, 2015

Hi @tute , from what I saw that commit is related only with mime/types and not the issue I referred above ;) Even so, I tried master and the problem persists.
The memory use is a bit lower but even so it's about 30MB (in my "patch" is around 5MB) .

$ USE_AUTH=true TEST_COUNT=5 PATH_TO_HIT=/checkinout_tasks  RUBY_GC_HEAP_OLDOBJECT_LIMIT_FACTOR=2   USE_SERVER=puma bundle exec derailed  exec perf:objects 

Booting: production
Endpoint: "/checkinout_tasks"
Auth: true
/Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/derailed_benchmarks-1.0.0/lib/derailed_benchmarks/tasks.rb:70: warning: already initialized constant DERAILED_APP
/Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/derailed_benchmarks-1.0.0/lib/derailed_benchmarks/tasks.rb:24: warning: previous definition of DERAILED_APP was here
Port: 3386
Server: "puma"
Puma 2.11.1 starting...
* Min threads: 0, max threads: 16
* Environment: none
* Listening on tcp://0.0.0.0:3386
Running 5 times
Total allocated 815468
Total retained 315

allocated memory by gem
-----------------------------------
  46214619  activerecord-4.2.0
  30642240  paperclip-14356d7eccb9
...
allocated memory by location
-----------------------------------
  23793840  /Users/sergio/work/capthost/vendor/ruby/2.1.0/bundler/gems/paperclip-14356d7eccb9/lib/paperclip/interpolations.rb:33
  20097300  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/connection_adapters/postgresql/database_statements.rb:168
   9532960  /Users/sergio/work/capthost/vendor/ruby/2.1.0/gems/activerecord-4.2.0/lib/active_record/result.rb:116
   4944000  /Users/sergio/work/capthost/vendor/ruby/2.1.0/bundler/gems/paperclip-14356d7eccb9/lib/paperclip/interpolations.rb:23
...

@bitcoder
Copy link
Author

@tute, please check #1888

@tute
Copy link
Contributor

tute commented Jun 24, 2015

Thank you all! This is super useful. I'm not doing code on paperclip for a few weeks, so delegating these performance improvements to @jyurek for now.

@tute
Copy link
Contributor

tute commented Sep 9, 2015

Thank you! It looks like #1888 is soon to be merged, closing this issue in favor of that PR. 👍

@tute tute closed this as completed Sep 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants