Skip to content

enhancement: optimize performance#902

Open
xiaofeige wants to merge 1 commit intoimgproxy:masterfrom
xiaofeige:master
Open

enhancement: optimize performance#902
xiaofeige wants to merge 1 commit intoimgproxy:masterfrom
xiaofeige:master

Conversation

@xiaofeige
Copy link

In my opinion, there is no need to wait for the IO complete. once the image caculation is done, the concurrency control strategy should let the request just go though, thus, making the CPU fully used.

@netlify
Copy link

netlify bot commented Jun 23, 2022

Deploy Preview for imgproxy-docs ready!

Name Link
🔨 Latest commit 28f5128
🔍 Latest deploy log https://app.netlify.com/sites/imgproxy-docs/deploys/62b47b5ed1027c0008c516ba
😎 Deploy Preview https://deploy-preview-902--imgproxy-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@DarthSim
Copy link
Member

Hi @xiaofeige!

I thought about something like this myself but decided not to do so and there are several reasons for this:

  1. Having the source image downloading process out of concurrency control may lead to memory bloat. If we have more requests than allowed by concurrency, imgproxy will download all the source images from that requests. Thus client can't control how many source images can be stored in memory at a time and can easily get OOM.

  2. Having the response writing process out of concurrency control doesn't make much sense. The response writing takes nanoseconds and isn't slowed down by slow clients.

@xiaofeige
Copy link
Author

Hi @xiaofeige!

I thought about something like this myself but decided not to do so and there are several reasons for this:

  1. Having the source image downloading process out of concurrency control may lead to memory bloat. If we have more requests than allowed by concurrency, imgproxy will download all the source images from that requests. Thus client can't control how many source images can be stored in memory at a time and can easily get OOM.
  2. Having the response writing process out of concurrency control doesn't make much sense. The response writing takes nanoseconds and isn't slowed down by slow clients.

yeah, you are right, but I think it's a trade-off. In my presure test, I noticed that there are much more memory left when CPU is 100% used. When I move the concurrency control to where I suggest, memory didn't grow that fast, basicly the same as before., but avg qps improved a lot.

@xiaofeige
Copy link
Author

by the way, format & resize operation performance are very pool, do you known how to improve it by adjust the configuration? thanks a lot. ^_^

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.

2 participants