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

Server-Side Rendering srcSet generation performance bottleneck #328

Closed
tstirrat15 opened this issue Mar 30, 2019 · 3 comments
Closed

Server-Side Rendering srcSet generation performance bottleneck #328

tstirrat15 opened this issue Mar 30, 2019 · 3 comments

Comments

@tstirrat15
Copy link
Contributor

tstirrat15 commented Mar 30, 2019

When profiling our isomorphic React app, one thing that we noticed was that srcSet generation was taking up a whole lot of CPU time - somewhere between a third and a half of the total cpu render time on a page that had 17 images.

Here's a node-clinic flamegraph showing the problem.

We were able to work around the problem by disabling srcset generation on the backend, especially because the images that were being sent from the backend were LQIPs, and it didn't make sense to have a full srcset for those, but the amount of CPU time consumed by generating srcSets was surprising to me.

Has this already been optimized, and it's just an inherently expensive operation? Or are there gains to be made in optimizing the srcSet generation code?

@sherwinski
Copy link
Contributor

Hi @tstirrat15 thanks for bringing this up. As far as I know, srcset generation has not been exhaustively optimized so perhaps there are gains to be made there. I won't be able to dedicate time to look into this just yet, but would happily review a PR or discuss anything you come across if you have the time and want to look into it.

@sherwinski
Copy link
Contributor

hey @tstirrat15,

There have been numerous performance changes made to URL building (all credit goes to @baldurh) since this issue was first opened. I would invite you, or anyone else who's curious, to test these out in the latest release. I'm going to go ahead and close this issue now, but feel free to comment back with any questions you may have.

Also relates to #406, #414, #418

@tstirrat15
Copy link
Contributor Author

Thank you!

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

2 participants