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

Cache eleventy_img generated images #99

Merged
merged 7 commits into from
Aug 31, 2021

Conversation

zeroby0
Copy link
Contributor

@zeroby0 zeroby0 commented Jul 27, 2021

Adds filesystem caching of images generated by eleventy_img to speed up build time. Cache is persisted locally and on Netlify.

Case Before After
Build time 48 seconds 12 seconds
Serve time 42 seconds 7 seconds

Serve time is the time from running yarn run serve and content being available on localhost:4001.

Read more at:

If this is merged, I'll notify here when 116 is merged. All the images look okay in local testing, but please test when merging.

Thank you for opensourcing your website, it was very helpful in verifying that the patch is error-less. Here is a PR as a token of gratitude :)

@AleksandrHovhannisyan AleksandrHovhannisyan self-requested a review July 27, 2021 17:06
@AleksandrHovhannisyan
Copy link
Owner

This is awesome, thanks again! I'll test this thoroughly, but I may hold off on merging until your PR to the Eleventy image plugin gets reviewed, just to be on the safe side.

@zeroby0
Copy link
Contributor Author

zeroby0 commented Jul 28, 2021

Glad you like it! :D

I hope it gets reviewed soon. It's a pretty simple change though, it's puzzling how no one has reviewed it yet. I should probably make a tool to make the review easier.

Copy link
Owner

@AleksandrHovhannisyan AleksandrHovhannisyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question! Gonna also test this locally when I get a chance; the Netlify deploy preview looks good 😄

netlify.toml Show resolved Hide resolved
Copy link
Owner

@AleksandrHovhannisyan AleksandrHovhannisyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good locally!

I didn't notice a change in local build times; it's definitely faster on Netlify, though.

@zeroby0
Copy link
Contributor Author

zeroby0 commented Jul 31, 2021

Even on rebuilds?
The first local build would take just as much time as normal, but subsequent rebuilds should be much faster.
Either that or your computer is really really fast and I'm jealous 😄

@AleksandrHovhannisyan
Copy link
Owner

Yeah, rebuild took around 90 seconds total (which is typical for this site locally). My laptop is likely slower than yours, lol.

@zeroby0
Copy link
Contributor Author

zeroby0 commented Jul 31, 2021

That's interesting.. As long as the _site/assets/img folder is preserved, rebuilds should be faster.
I did change the clean script to rimraf '_site/!(assets)' '_site/assets/!(images)' to preserve it, could that be not working somehow? Which OS are you on?

Here is what I ran on linux:

git clone https://github.com/zeroby0/aleksandrhovhannisyan.com.git
git checkout cache
yarn
yarn run build # 42.6 seconds
yarn run build # 10.3 seconds

@AleksandrHovhannisyan
Copy link
Owner

AleksandrHovhannisyan commented Aug 6, 2021

Sorry for the delayed response—I'm on Windows (WSL), so that's likely the issue.

The good news is that the local builds aren't slower, and if I ever get a better dev setup going, they'll be faster 😄

@zeroby0
Copy link
Contributor Author

zeroby0 commented Aug 6, 2021

Maybe, I'll don't have a windows machine around, but I'll try to see it in a VM in the weekend.

Also, the PR is under review and I'm so excited!

@AleksandrHovhannisyan
Copy link
Owner

I noticed! They even tweeted about it 😄

@zeroby0
Copy link
Contributor Author

zeroby0 commented Aug 11, 2021

@AleksandrHovhannisyan
You're running WSL 2, right?

I tested it directly without wsl, and that worked as expected. Will test wsl now.

@AleksandrHovhannisyan
Copy link
Owner

@zeroby0 I'm actually running WSL1 like a scrub. (My project is in a Windows dir, so WSL1 is technically faster than 2 for me)

@zeroby0
Copy link
Contributor Author

zeroby0 commented Aug 11, 2021

Smart choice!

So I tried wsl1, wsl2, and bare windows on a VM.
On wsl1, with the folder in a Windows dir,

time npm run build # No cache
real 2m37.072s
user 1m37.547s
sys 0m35.422s

time npm run build  # Second run, so images are in cache
real 0m56.767s
user 0m21.453s
sys 0m18.406s

So I still have no idea why it isn't working, unfortunately 😂

Can you run this and see if the cache doesn't work there too: https://github.com/zeroby0/demo-eleventy-img-cache

@AleksandrHovhannisyan
Copy link
Owner

Sure! I'll give it a shot when I get a chance.

@AleksandrHovhannisyan
Copy link
Owner

@zeroby0 Wow I'm dumb 😅 I was testing without doing a yarn install. Should probably add a postcheckout git hook to install dependencies.

18 seconds now! Amazing stuff—will follow the 11ty PR closely and then we can point this PR to the plugin.

@zeroby0
Copy link
Contributor Author

zeroby0 commented Aug 30, 2021

Haha, happens 😄 I do that sometimes too 😂

TIL about the postcheckout git hook! :D

Can't believe it's been a month since we started this PR, seems like yesterday! 😄

@AleksandrHovhannisyan
Copy link
Owner

AleksandrHovhannisyan commented Aug 30, 2021

@zeroby0 Right? I'm excited to merge this!

When you get a chance, could you update the branch to point to the 11ty image v1.0.0-beta.0 release? (Might also need to catch up w/ master). And then I'll test one more time locally + on the deploy preview, and then we should be good.

@zeroby0
Copy link
Contributor Author

zeroby0 commented Aug 31, 2021

Done! :D

But you should probably wait for the second beta before merging if you're using remote URLs: 11ty/eleventy-img#123

@AleksandrHovhannisyan
Copy link
Owner

Nope, no remote URLs yet (I may support them in the future though)

@AleksandrHovhannisyan
Copy link
Owner

Works great, thanks again!

@AleksandrHovhannisyan AleksandrHovhannisyan merged commit c28eb81 into AleksandrHovhannisyan:master Aug 31, 2021
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