Skip to content

Memory leak in Sprockets v3.0 #35

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

Closed
toncid opened this issue Apr 23, 2015 · 19 comments
Closed

Memory leak in Sprockets v3.0 #35

toncid opened this issue Apr 23, 2015 · 19 comments

Comments

@toncid
Copy link

toncid commented Apr 23, 2015

Hello,

I'm not sure how to be more precise, but there seems to be a memory leak in the asset fetching code in v3.0.

For some reason, we need to provide an HTML page with all assets embedded into the page. That being said, page's <head> looks like this:

    <head>
        <meta charset="utf-8" />
        <meta name="viewport" content="..." />

        <%= content_tag :style, type: :'text/css', media: :all do %>
            <%= Rails.application.assets['path/to/template.css'].to_s.html_safe %>
        <% end %>

        <%= javascript_tag do %>
            <%= Rails.application.assets['path/to/template.js'].to_s.html_safe %>
        <% end %>
    </head>

In some cases it takes 5 seconds for this part to render (according to New Relic). Also, there is a memory usage spike that eventually leads to "Memory limit reached" errors (on Heroku).

The problem vanished when I reverted to v2.12.

Please mind that this is happening in production and that assets have already been minified during code deploy.

Thanks!

Edit: Fixed some typos.

@rafaelfranca
Copy link
Member

I don't know if this is a memory leak. What I think it is happening here is that you code is compiling the assets when rendering and this is growing the memory usage (what is expected if you are compiling the asset). I think in sprocket 2 we cache it somehow. @josh can confirm that

@josh
Copy link
Contributor

josh commented Apr 23, 2015

Woah, definitely curious what changed in the memory growth between 2.x and 3.x. Any addition ways to reproduce this would be helpful.

Its interesting that your trying to inline the result of the asset. Though, I'd probably advise against this practice since you're using compile = true production which should really be avoided. Have you thought about just doing a File.read from the compiled asset on disk in production?

@josh
Copy link
Contributor

josh commented Apr 23, 2015

One thing I'd be curious if you could try is to configure the size of the local in memory cache. Right now it defaults to 4096.

@fetch_cache = Cache::MemoryStore.new(4096)

Try dropping that to like 1024 and see what happens.

@toncid
Copy link
Author

toncid commented Apr 24, 2015

@josh The reason we need the assets inline with HTML is because the page is downloaded to a mobile device where it must be loadable into a WebView at any time, even while the device is offline. That's why we need everything packed into HTML.

The assets are precompiled and minified during code deployment and served by the web server, rather than by Rails. However, this page has some user-specific parts (a view partial is rendered within the <body>) and thus cannot be precompiled in advance.

The CSS file is rather large, mainly due to embedded web fonts via data URLs. The JS file is much smaller, but I suppose file size can only affect the speed at which memory is consumed.

Hope this provides some context.

What would the cache size tweak do?

@toncid
Copy link
Author

toncid commented Apr 26, 2015

@josh @rafaelfranca Any additional thoughts on this issue? What is the best and preferably future-proof way of circumventing the problem?

Thanks!

@josh
Copy link
Contributor

josh commented Apr 27, 2015

However, this page has some user-specific parts (a view partial is rendered within the ) and thus cannot be precompiled in advance.

You should definitely try serving those from regular ActionView methods. Its going to be much faster. You really don't want to be exec'ing CoffeeScript or Sass to render those inline components.

What would the cache size tweak do?

Could you give changing the cache size a try please. That would hopefully narrow down the issues to Sprockets cache references. Reducing those references may limit how many cached objects are kept around in memory possibly contributing to the memory warning.

@toncid
Copy link
Author

toncid commented Apr 27, 2015

Hm, I may be wrong, but aren't you describing how to circumvent a buggy behavior: if an asset is already precompiled, shouldn't Rails.application.assets['asset_name'].to_s fall back to File.read automatically and never run CoffeeScript/SASS for those?

You should definitely try serving those from regular ActionView methods.

To which "regular" ActionView methods you're referring to? I'm not familiar of any other way of loading assets other than via the above-mentioned Sprockets interface (Rails.application.assets).

@josh
Copy link
Contributor

josh commented Apr 27, 2015

if an asset is already precompiled, shouldn't Rails.application.assets['asset_name'].to_s fall back to File.read automatically and never run CoffeeScript/SASS for those?

No, its never worked that way.

@toncid
Copy link
Author

toncid commented Apr 27, 2015

Gotcha. All right, thanks for the info!

@josh
Copy link
Contributor

josh commented Apr 28, 2015

if an asset is already precompiled, shouldn't Rails.application.assets['asset_name'].to_s fall back to File.read automatically and never run CoffeeScript/SASS for those?

If you have compile = true set, it means always compile.

Is there anyway you can test the cache size limit idea I had? I don't a reproducible example to try myself.

@toncid
Copy link
Author

toncid commented Apr 28, 2015

No, the compile flag is set to false:

  config.serve_static_files = false

  config.assets.js_compressor = :uglifier
  config.assets.compile = false
  config.assets.digest = true
  config.assets.version = '1.0'

Does this mean that asset.to_s will fall back to File.read in this case then?

Is there anyway you can test the cache size limit idea I had? I don't a reproducible example to try myself.

No, unfortunately, I cannot promise that I'll be able to try out the cache tweak, since we're talking about the production environment.

@josh
Copy link
Contributor

josh commented Apr 28, 2015

No, the compile flag is set to false:

Rails.application.assets should be nil in that case.

@toncid
Copy link
Author

toncid commented Apr 28, 2015

Strange, it is not nil, not even with v3. Accessing Rails.application.assets probably causes the compilation.

How do I get the file path on the file system then? The precompiled asset contains its digest in the file name and I just cannot use the file name.

@josh
Copy link
Contributor

josh commented Apr 28, 2015

Rails.application.asset_manifest["something.js"] should give you "something-123abc.js".

@toncid
Copy link
Author

toncid commented Apr 28, 2015

So it shouldn't be nil? :-)

@josh
Copy link
Contributor

josh commented Apr 28, 2015

@toncid
Copy link
Author

toncid commented Apr 28, 2015

Please ignore, I missed the _manifest part.

@josh
Copy link
Contributor

josh commented Apr 28, 2015

@toncid
Copy link
Author

toncid commented Apr 28, 2015

OK, will check. Thanks!

@josh josh closed this as completed Apr 28, 2015
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

No branches or pull requests

3 participants