fix: Sharp memory fragmentation #16978
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -6 B (0%) Total Size: 1.52 MB ℹ️ View Unchanged
|
|
This pull request has been mentioned on Strapi Community Forum. There might be relevant details there: |
|
Hello, I'm not sure if this is the right place, but I wanted to add my two cents to "things to consider". Adding the option to store a "Breakpoint" as a string in the database:
Dynamic Breakpoints:
File Conversion on the Server:
Thanks for working on this issue 🙏. It's a massive pain point for us as we are running Sharp locally and then uploading the thumbnail and cover image for each product (over 1500 products so far). |
|
Hi, I'm the sharp maintainer and have been made aware of some of the discussions around Strapi and memory fragmentation. The number of "memory leak" complaints from Strapi users seems to be rather disproportionate, so I've had a quick look at the Strapi source code to see if there are possible improvements in the way sharp is being used. Given input images appear to already exist on the filesystem, I highly recommend Strapi passes the (absolute) filesystem path to sharp; do not use Streams. This will allow libvips to mmap input and use its operation cache, resulting in significantly fewer memory allocations and therefore reducing the effects of fragmentation. I notice from this PR that you're refactoring much of this code right now, but after this is complete I'd be happy to offer a review or attempt a PR to help address the underlying issues. |
|
wow! thank you for coming by and reviewing our work!! This PR pipes all the transformations into a single stream, and greatly reduces the issue. Your proposal makes total sense, to not use streams for images. I think that could be something we can do for images with relatively small weight, and would reduce the number of mem allocations. That is totally doable. And on a side topic, do you know the status of sharp being compiled to webassembly? That could help some installation issues for some users 🚀 Again, thank you for your time and we will keep you updated <3 |
|
Really can't wait for this to be merged!! |
|
@longd3 trying to get around it, but at the moment this introduces breaking changes, if we can not sort them out this will target the next major release v5. |
|
Any updates on this? @Marc-Roig |
|
@SvenWesterlaken sorry for the delayed response :) we are atm focused on the stabilizing the new V5 major, after that we do plan on retaking this work, it might take a bit more time than expected. |
markkaylor
reviewed
Mar 21, 2024
| "react-query": "3.39.3", | ||
| "react-redux": "8.0.5", | ||
| "react-select": "5.7.0", | ||
| "sharp": "0.32.0", |
There was a problem hiding this comment.
Do you think we could bump sharp to 0.33.2? I'm thinking of this PR #19311 and the attached issue.
There was a problem hiding this comment.
yes! lets do that
3 tasks
|
closing it in favor of #20080 :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does it do?
Proposes a more composable way to define file transformations, that allows:
This is still a draft and a work in progress.
I also used this opportunity to clean a bit the upload plugin, specially the main upload service https://github.com/strapi/strapi/pull/16978/files#diff-3c0f1c68e7bcad4702d7bd9600b9ca459715158a9451da2ae05454d163cada16 .
Why is it needed?
Sharp is a fast and reliable tool to transform images that we rely upon in Strapi. Unfortunately, in some Linux machines, there seems to be some sort of memory fragmentation that causes RAM spikes and memory fragmentation.
lovell/sharp#3052
lovell/sharp#1710
lovell/sharp#955
Because of the nature of this issue we decided to move this forward in an extensible way, where anyone can create it's own custom solution that works for each setup.
We are going to continue exploring this and see what tools we can provide to better handle issues like this one.
How to test it?
Provide information about the environment and the path to verify the behaviour.
Related issue(s)/PR(s)
Fixes #14417