perf(astro/assets): avoid downloading original image when using cache #11904
Conversation
🦋 Changeset detectedLatest commit: de53cab The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: astro
| @@ -207,6 +200,7 @@ export async function generateImagesForPath( | |||
| ? (options.src as ImageMetadata).src | |||
| : (options.src as string); | |||
|
|
|||
| const originalImage = await loadImage(originalFilePath, env); | |||
There was a problem hiding this comment.
There's a tradeoff here with this that I made somewhat on purpose, this massively improve fully cached runs, especially with remote images, but it makes it way worse for initial builds with a lot of variants of images. Ideally we'd be able to come to a perfect solution, but I couldn't figure it out when I first did it though.
There was a problem hiding this comment.
Thanks for your input @Princesseuh! I’ll mark this PR as draft and try to investigate this scenario. Do you have any idea why it made it worse?
There was a problem hiding this comment.
Because you end up loading the image for every transform of an image. For instance, if you have an image of which you generate 16 variations (which is quite easy to do with Picture), you'll end up loading the image 17 times.
Probably that a refactor here would be possible where we check if every variants is cached first, if so skip loading the image
There was a problem hiding this comment.
@Princesseuh, I've implemented your suggestions and updated the screenshots to include a non-cached run.
Changes
Moves the
loadImagemethod call to after the image cache check, improving performance massively and reducing memory consumption for remote images.Testing
Manual testing was performed in my company's website, with 950 optimized images, and in erika.florist website, with 626 images.
Docs
N/A.