twilio / twilio-ruby

A Ruby gem for communicating with the Twilio API and generating TwiML
MIT License
1.36k stars 464 forks source link

twilio-ruby is taking too much memory. #396

Open mhammiche opened 6 years ago

mhammiche commented 6 years ago

Version: 5.8

twilio-ruby is taking too much memory.

Steps to Reproduce

I audited my project dependencies with the help of derailed_benchmarks and I was surprised too find twilio-ruby at the top of mem usage even before the rails stack.

Here is an extract of the derailed gem

$  bundle exec derailed bundle:mem

TOP: 89.0391 MiB
  twilio-ruby: 28.1602 MiB
    ~/.rvm/gems/ruby-2.3.1/gems/twilio-ruby-5.8.0/lib/twilio-ruby/rest/ip_messaging/v1/service.rb: 0.3047 MiB
    ~/.rvm/gems/ruby-2.3.1/gems/twilio-ruby-5.8.0/lib/twilio-ruby/rest/api/v2010/account.rb: 0.3008 MiB
  rails/all: 20.332 MiB
    rails: 9.3125 MiB (Also required by: active_record/railtie, active_model/railtie, and 8 others)
...
philnash commented 6 years ago

This is interesting. Thanks for bringing it up. Can I ask how you are using twilio-ruby in your application? I found that just including it in the Gemfile lead to 17MiB in use (which is still a lot compared to other gems) but I wonder how to replicate that 28MiB that you're seeing.

Thanks

philnash commented 6 years ago

Could you test version 5.9 and see if it’s any better?

mhammiche commented 6 years ago

I just checked version 5.9. It does not make it better. derailed_benchmark still give me about 28 MB. I understand the rest client is huge. It is about a hundred of ruby file each taking 0.15 MB

But we are only using it to send small text message like a confirmation code. May be the best solution for our use case is to call the api directly without relying on twilio ruby

philnash commented 6 years ago

I understand, if you're only using it for one message sending, then the whole library and this memory footprint is probably a bit much.

In my testing I only found that derailed_benchmark showed it using ~16-17MiB, can you share how you are using the library within your application?

You've got me thinking about how this could be modularised better so that you could only include the bits that you need, but that is going to take some work. If all you are doing is sending messages and this memory is important, then for now I recommend you implement a call to the API directly. Let me know if I can help with that at all.

skplunkerin commented 6 years ago

Even ~16-17MiB is still really high... twilio-ruby is my 2nd highest Gem in my codebase, at 12.6797 MiB (with my 3rd highest being 5.1367MiB). I'm using this Gem very minimally (to send an SMS), so using this much MiB is really crazy. Is there a way to decrease this further? Or to only require the parts of this Gem that are needed? (i.e. for using @client.messages.create( message ))

I'm using twilio-ruby v5.10.3

philnash commented 6 years ago

Sorry to take an age to respond to this.

I've had another idea, which is to try to get the library to lazy require the resources it needs as it uses them, rather than loading them all up front. That would reduce initial memory usage, particularly in the case that you are only using it to send a message. I will try to get some time to experiment with this soon.

I would also suggest that if you are only sending messages, then perhaps using a plain HTTP client and building the request yourself might be easier. Using the full library is great if you are doing a lot of stuff with Twilio, but if @client.messages.create is all you are doing then you can cut the amount of code by just cutting the library itself.

kevinelliott commented 6 years ago

@philnash Have you had a chance to do your experiments? We'd really benefit from the reduction in memory at boot. This is also our second largest gem (behind rails itself), which feels incredibly abusive for an API client (and we have many).

philnash commented 6 years ago

I have not had the opportunity to do so yet, no.

kevinelliott commented 6 years ago

:(

philnash commented 6 years ago

I had a quick look last night after leaving that message and I do believe that the approach of lazy loading modules as they are used will help. It's going to be a big job to write that into the framework though. I'll try to put a proof of concept forward then look at porting it into the library generator that we use internally. I can't promise anything is going to happen soon though as this is a big change to the way the library is written.

kevinelliott commented 6 years ago

This would significantly help out the footprint of many Rails stacks! Thank you.

AnatoliiD commented 6 years ago

@philnash What about Kernel#autoload ? I've found that code structure is really awful. Do you plan to opensource yoyodyne or at least code templates for different languages?

philnash commented 6 years ago

@railsme autoload might work actually, good idea. I feel like I read something about autoload not working that well though. Will do some more research.

I don't think there are any plans to release yoyodyne publicly at the moment. What were you hoping for with it?

AnatoliiD commented 6 years ago

@philnash it will help to maintain gem source code. Nobody can change code/files structure in gem because it will be regenerated again. It is nice that Twilio provides libraries to use service but it's bad when everybody should rely on Twilio's code without any possibility to make it better. I'd really like to help but cannot do anything because of this reason :)

About autoload. Yes. There is a problem when multiple autoloaded classes/modules are declared in one file and accessed from different threads but if you'll put each class/module in its own file then you won't see any problem(at least I haven't seen them before in modern MRIs).

Maybe somebody knows more about autoload problems.

philnash commented 6 years ago

@railsme While yoyodyne does ultimately generate the code, we are happy to take improvements submitted to this repo as pull requests and either merge them in (not all the code is created by yoyodyne) or update yoyodyne internally to generate the result.

If you want to work closer on improving the library, let me know and I'll see what I can do to help.

Thanks for the autoload bug, I'll keep that in mind while I work on this.

rubendinho commented 5 years ago

We are seeing twilio-ruby take up 19.89MiB; that's almost as much memory at boot as rails/all itself (25.7MiB). We are also currently using this library on a limited basis, and the memory consumption is making us think twice about doubling down on it as we build new features.

I am grateful that this gem makes building with Twilio so easy, but it is a bit disappointing that a paid service would have a notable negative effect on our app's memory consumption.

LizPrescott commented 5 years ago

Same. twilio-ruby: 25.3008 MiB, in our case even more than rails/all: 20.0664 MiB. Edit: upgrading to the latest gem version (from 5.16 to 5.17) makes it slightly worse. twilio-ruby: 26.4063 MiB

philnash commented 5 years ago

I've spent a bit of time thinking about this and trying to come up with solutions.

Let me explain what I think the problem is and how I've started to work towards a solution.

Twilio these days has a huge API surface. It's no longer sending messages and making calls, there are resources for Chat, Video, Notifications, Fax, Wireless, Autopilot, TaskRouter... the list goes on. To date we have created libraries that cover the entire REST API as well as generators for TwiML and JWTs to give access to the client side portions of Twilio. On top of that, we have also written a library generator which takes API definitions and generates the libraries in multiple languages.

As Twilio has grown, the libraries have grown and I believe that this has lead to the memory bloat you are seeing in your apps today, especially since the library requires all of the files, regardless of how much or little you use them.

For this reason I believe that lazy loading the library with Kernel#autoload is the best move for now to cut down on initial memory usage for the library. It means that the library will continue to work as expected, but will load quicker and use less memory. For the future it might be more sensible to split the gem into parts such that you can choose explicitly to load.

I have begun work on this, autoloading everything I can from the non-generated parts of the application. This work is on the autoload-ftw branch of my fork.

I'd appreciate if those of you here who are experiencing high memory usage could install the gem from my branch and see whether there is a drop in memory usage. In my testing I have seen drops in memory usage of about 20%.

Note: all the API resource classes are still required, they will just take more time to tease apart. I wanted to put this forward to see whether it will help in real life applications.

If you are able to test this and let me know the memory usage compared to the published gem I would be most grateful.

AnatoliiD commented 5 years ago

Hi @philnash Really good job! You could think about splitting gem into multiple service-oriented gems. twilio-core is for api wrapper, other gems(twilio-chat, twilio-video, etc.) for each service with core reuse and the global gem for getting everything together if you use all the services :)

As I can see code generation limits you in moving files to different locations and you had to use File.dirname(__FILE__) everywhere.

raldred commented 5 years ago

@philnash trying our your branch, getting the following error:

.../twilio-ruby-353fd4692733/lib/twilio-ruby.rb:53:in `<module:Rack>': undefined method `dirname' for Rack::File:Class (NoMethodError)
from .../twilio-ruby-353fd4692733/lib/twilio-ruby.rb:52:in `<top (required)>'
olliebennett commented 5 years ago

I also found disproportionate memory usage for this gem; twilio-ruby (5.23.0) is taking up 18% of the memory of my application (with ~100 direct gem dependencies in our project).

# bundle exec derailed bundle:mem

TOP: 108.9727 MiB
  rails/all: 24.4063 MiB
  twilio-ruby: 19.8164 MiB
  newrelic_rpm: 10.6875 MiB
  ...

As with others, I am only using the SMS feature;

client.api.account.messages.create

I'm late to the party, but the autoload-ftw branch of philnash/twilio-ruby is at 353fd4692733340a982f3ea8ebbf338eedf38599 which seems to have been merged into the main repo anyway. Even on the current edge of master (8746725c4ff2407a8244301a168861ea664841d8) the memory usage doesn't change.

I'd love to see the approach (like fog takes) where I can only load gems containing the subset of API functionality that I want to use, or else perhaps free-standing gems for the most common functionality that could be used individually.

coffenbacher commented 5 years ago

Since like @olliebennett and others, I am only using the SMS sending feature, I was able to replace the whole thing in my project with the equivalent of the following:

require "rest-client"

content = "Hello world"
to = "+............"

params = {
  Body: content,
  From: ENV["TWILIO_PHONE_NUMBER"],
  To: to
}

RestClient::Request.execute(
  method: :post,
  url: "https://api.twilio.com/2010-04-01/Accounts/#{ENV["TWILIO_ACCOUNT_SID"]}/Messages.json?",
  payload: params.to_query,
  user: ENV["TWILIO_ACCOUNT_SID"],
  password: ENV["TWILIO_AUTH_TOKEN"]
)

Hardest part was figuring out that the parameters are case-sensitive and capitalized.

Memory usage is going πŸ“‰ πŸ˜ƒ

andrewr224 commented 5 years ago

Same here, twilio-ruby: 25.5234 MiB with version 5.13.0. Upgrading to 5.24 (the latest version as of this writing) increased memory up to 27.8438 MiB.

I'm using it for both calls and SMS though, so can't replace it.

I'm on ruby -v 2.5.0, rials -v 5.2.0

alexford commented 5 years ago

I put together a very small (no external dependencies) helper gem for sending SMS via the API: https://github.com/alexford/twilito

If, like me, you just need to send SMS and don't need the rest of the currently RAM-hungry Twilio library, it might help you. It has not been tested in the wild yet but it's pretty simple. PRs/issues welcome!

ErikAGriffin commented 5 years ago

We were using this gem solely for the functionality of authenticating incoming SMS webhooks to our gem, making its size quite excessive for such a small feature.

I extracted the authentication functionality out of this gem and packaged it separately here: https://github.com/pathrise-eng/twilio-ruby-authenticate-webhooks

Will try to keep it updated/maintained in the future if people @ me with noticeable bugs or changes.

childish-sambino commented 5 years ago

@ErikAGriffin Note that the validation logic was updated recently in this repo: https://github.com/twilio/twilio-ruby/pull/481

tonydehnke commented 5 years ago

Seeing similar issues on memory footprint with gem version 5.28, Rails 5.2.3 at around 20% of total memory: twilio-ruby: 19.25 MiB. My second largest after Rails-all from what I can see. Currently only using SMS send so may take a look at the options listed above.

stephancom commented 4 years ago

I had the exact problem, as seen in a few top-level lines from my derailed output:

TOP: 144.9023 MiB
  twilio-ruby: 25.957 MiB
  rails/all: 23.0 MiB
  aws-sdk-s3: 14.8398 MiB
  graphql/rails_logger: 12.0352 MiB

Bigger than rails, or almost as big as AWS and GraphQL combined, and like others, I was just using it to send a few SMS messages. I replaced it with Twilito (thank you @alexford !) and now:

TOP: 133.4883 MiB
  rails/all: 22.793 MiB
  graphql/rails_logger: 13.5859 MiB

Not as much difference as I'd hoped but a good 10% or so, surely worth the trouble. Oh, yeah, and Twilito seems to work just fine! Easier than the Twilio gem if you're just sending text messages!

coreydaley commented 3 years ago

Since this hasn't been updated in quite awhile...

This is still an ongoing issue with the twilio-ruby gem, using from 25-28 MiB of memory, which is ridiculous. If anyone is interested, here is a simple SMS example using HTTParty, which uses significantly less memory: https://gist.github.com/coreydaley/8cdcecf2cb21bbae416442c231be0d32

cromulus commented 3 years ago

It also adds a half a second to my boot time, using Bumbler

For reference, this code takes half a second to run and uses 29MB of memory on a 2019 MacBook pro:

require 'twilio-ruby'

Twilio.configure do |config|
  config.account_sid =MYSID
  config.auth_token = SUPERSECRETTOKEN
end

@client = Twilio::REST::Client.new

If I had to guess, it's the bulk require happening lines 19-33 in /lib/twilio-ruby.rb

The rest directory itself is 7 MB of text. It appears that the code in rest is autogenerated by Yoyodyne..

If this Yoyodyne could change the code it generates to utilize autoload could save substantial amounts of memory and boot time.