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

Update to 8.10.5, add webp, add heif #13

Merged
merged 14 commits into from
Mar 12, 2021
Merged

Update to 8.10.5, add webp, add heif #13

merged 14 commits into from
Mar 12, 2021

Conversation

brandoncc
Copy link
Contributor

No description provided.

@rjherrera
Copy link

Hi @brandoncc! This looks promising! Any luck getting it to work? Can I help you in any way?

@brandoncc
Copy link
Contributor Author

Hi @brandoncc! This looks promising! Any luck getting it to work? Can I help you in any way?

I totally missed your comment @rjherrera, sorry!

I believe this is finally working, and I am merging the PR now.

@brandoncc brandoncc merged commit 4920829 into master Mar 12, 2021
@brandoncc brandoncc deleted the add-heif-webp branch March 12, 2021 20:15
@rjherrera
Copy link

@brandoncc no worries, thank you! I will try this out soon, and I'll let you know! 🙌🏻

@rjherrera
Copy link

hi there @brandoncc ! I've been using this buildpack for a couple of weeks now. It works perfectly and I can process heic and webp files. Its a lot faster when one can use vips rather than minimagick. Thank you for your work!

One thing I've noticed is that memory tends to go up a considerable amount after a couple hours. I don't know if this is related to vips itself, to a dependency of the libvips library, the buildpack or even something unrelated, but maybe you know. This is the only considerable change I've noticed since making the move to this buildpack and using vips as a processor for image_processing instead of minimagick. I included an image for reference:

image

This may have nothing to do with the buildpack itself, I don't expect the magic answer haha, so thank you either way!

@brandoncc
Copy link
Contributor Author

Hi @rjherrera,

I'm glad you experienced measurable gains! The application I created the buildpack for was able to convert a 30 minute PDF conversion into about 90 seconds by switching to VIPS. I love VIPS (if there is such a thing as a VIPS-fanboy, I'm the president of the club), and @jcupitt deserves a ton of credit for all of his amazing work. He also deserves a lot of credit for helping me on this buildpack when I get stuck.

I don't know offhand the best way to measure your memory usage issues, because it has been a while since I had to look into this issue myself. I would recommend looking into profiling memory usage, and try to find Heroku-specific measurement methodologies. Once you identify the processes which are experiencing memory leaks (maybe newrelic or scout can be of use?), I'm happy to try to assist if it is something related to this PR.

Thanks again for the kind words.

@jcupitt
Copy link
Contributor

jcupitt commented Apr 23, 2021

You could try jemalloc:

https://github.com/jemalloc/jemalloc

It fixes most of the memory growth problems in node, so it could well help here too.

@rjherrera
Copy link

Thank you guys! I'm aware of some techniques to approach the memory issue that seems to be a frequent problem in rails apps hahaha. I haven't tried the jemalloc approch though @jcupitt, so I will, thanks!

The interesting thing @brandoncc is that this is the only considerable change I made for this deploy, and when using minimagick it used less memory. Maybe vips is more prone to leak memory or to generate fragmentation. I am using scout at the moment so I will gather some more info to see what can be done!

Here we can see in the picture when I did the change to the buildpack. But to be fair, the change also considered using the Aptfile with the dependencies, so it might be that
image

Spikes are common in my app both with minimagick and vips cause we have some intensive shrine processing done in background, but the general rise in the "base level" is what worries me.

I will try some stuff and come back, thanks for the ideas 💯 , and yeah, the time gain is so noticeable that I'm keeping vips either way!

@brandoncc
Copy link
Contributor Author

Thanks for chiming in @jcupitt 😄

@rjherrera I know when I was researching a similar issue, I found out that Ruby doesn't free memory, even if it is no longer used. Garbage collection is not the same thing as garbage compaction, which I didn't realize until then. Ruby 3.0 brought garbage compaction, but I was informed that it is unlikely to fix these types of issues. Basically, as we create junk in memory, that memory is never "freed" again. I hope this helps!

@jcupitt
Copy link
Contributor

jcupitt commented Apr 24, 2021

jemalloc really does return memory to the OS, it's worth trying. All you do is set LD_LIBRARY_PATH to point to the jemalloc allocator before you start your rails app, eg.:

LD_LIBRARY_PATH=/my/jemalloc/install rails server

@brandoncc
Copy link
Contributor Author

Oh, I didn't realize that is what jemalloc did. I'll be trying that as well. Thanks!

@brandoncc
Copy link
Contributor Author

@rjherrera a quick search found https://github.com/gaffneyc/heroku-buildpack-jemalloc. It looks pretty nice, so I will probably try that implementation first. Not sure if you had found that or another option yet.

@rjherrera
Copy link

@jcupitt @brandoncc I've just configured jemalloc with the buildpack you provided and I am waiting to get a bit more data to confirm if it has made a noticeable change! Thanks to both for the recommendations!

@brandoncc
Copy link
Contributor Author

That's great @rjherrera. If you are so inclined, I'd love to hear about your experience once you have data.

@rjherrera
Copy link

@brandoncc I tested in my staging environment and at first it seemed normal but then when the spikes came (which is normal in my app at certain hours) instead of maintaining the memory at a high base level untile a reboot, it started to gradually drop, which is a huge improvement! I will roll jemalloc to production and I hope it works as good as it worked there!

image

@jcupitt
Copy link
Contributor

jcupitt commented Apr 27, 2021

Yes, jemalloc is good stuff.

It splits your memory into "arenas" and puts similar types of things together. It's able to return memory to the OS when it's not longer needed, and because similar things are grouped, you get a lot less fragmentation. The downside is slightly higher initial memory usage.

@rjherrera
Copy link

@jcupitt yeah I noticed the slightly higher initial memory usage, but it looks like a price worth paying. Avoiding the threatening upwards trend in memory is exactly what I needed. Thank you guys! 🙌🏻

@jcupitt
Copy link
Contributor

jcupitt commented Apr 27, 2021

The upward trend would probably stabilize over time, since there are no leaks, just fragmentation, but I agree it looks alarming.

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.

3 participants