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

Performance degradation after upgrading to 5.1.0 #253

Closed
6 tasks done
MartijnKooij opened this issue Dec 29, 2020 · 5 comments
Closed
6 tasks done

Performance degradation after upgrading to 5.1.0 #253

MartijnKooij opened this issue Dec 29, 2020 · 5 comments
Labels

Comments

@MartijnKooij
Copy link

MartijnKooij commented Dec 29, 2020

Describe the bug
We upgraded from 4.2.0 to 5.1.0 and are noticing a pretty significant performance degradation, see attached image from the image handler lambda. Is that to be expected from 5.0/5.1 or is something wrong?

These increased execution times were causing problems for us because our reserved concurrency limit of 120 was no longer enough to handle the requests in time which started causing HTTP 500 errors for our end users due to lambda throttling.

To Reproduce
Install 4.2 and produce some load no the system to get enough metrics, then upgrade to 5.1.0 and measure again.

Expected behavior
The lambda execution times remain similar to what they were.

Please complete the following information about the solution:

  • Version 5.1.0
  • Region: eu-west-1
  • Required modifications for SSL certificate/domain config and setting the ReservedConcurrentExecutions of the handler to 120 (currently 250)
  • Not available on GitHub but just those changes.
  • Have you checked your service quotas for the sevices this solution uses?
  • No errors in the lambda at all.

Screenshots
image

Additional context
We skipped 5.0.0 because of #234 so we're not sure if the performance hit was already noticeable there.
If the upgrade simply delivers more value / is more complex for other reasons and an increase is expected then that's fine. But it might be worth mentioning in the release notes then.

@MartijnKooij
Copy link
Author

Did not realize this before but the added/new value of this upgrade for us is that we can now resize SVG > PNG. So that's the big change which would indeed heavily affect this instance of the image handler because it happens to contain mostly SVGs.
So previously on 4.2 they just were not resized, and now they are. So yes, that's a performance hit. But is this also the performance increase that you could expect? There's quite a few max duration of 5+ seconds.

@beomseoklee
Copy link
Member

@MartijnKooij Thanks for your feedback. From v4.2 to v5.0, we've fixed the WebP issue. On v4.2, every image had changed to WebP even after choosing "No" for WebP Automation. That caused a performance issue: #205

From v5.0 to v5.1, we've fixed the SVG issue like you mentioned. So when you update from v4.2 to v5.1, it would fix the WebP issue and the SVG issue together. v5.1 would change the format to PNG when you resize the SVG images. However, on v4.2, it would change the format to WebP.

So, I'm not exactly sure what would cause a big performance difference between v4.2 and v5.0 for SVG images. Do you have any SVG file that I can test the performance difference between v4.2 and v5.1?

@MartijnKooij
Copy link
Author

I might be mistaken but I thought 4.2 just did not yet support SVG (#31), did it not just return the unmodified SVG? If that's the case then that would explain the performance hit right?

In case I'm mistaken here are a couple of SVG images that we use. A common edit looks like

{"bucket":"ourbucket.aws.org","key":"icons/7569.SVG","edits":{"resize":{"width":90,"height":90,"fit":"fill"}}}

(3 icons zipped because I could not directly add the svg files)
icons.zip

@beomseoklee
Copy link
Member

beomseoklee commented Jan 5, 2021

@MartijnKooij v4.2 supported SVG with encoded URLs. It didn't support SVG with Thumbor style URLs, and v5.x supports the Thumbor style URLs for SVG as well.

I've tested v4.2 and v5.1 around 400 times, but I haven't seen significant performance difference regarding the SVG resize, but I agree sometimes processing images would take longer than your expectation.

We would see how we can improve the underlying performance.

@fisenkodv
Copy link
Contributor

@MartijnKooij we have updated our solution. If you still see the issue with the latest version (v6.0.0), please feel free to reopen the issue.

You can refer to the recent changes here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants