helaili / jekyll-action

A GitHub Action to publish Jekyll based content as a GitHub Pages site
MIT License
250 stars 120 forks source link

Decrease build times #17

Closed limjh16 closed 4 years ago

limjh16 commented 4 years ago

Makes use of https://github.com/actions/cache and removes some dependencies from the Dockerfile to decrease GitHub Actions build times. From my own testing, the runtime for the workflow has decreased to ~40sec from ~2min20sec previously.

limjh16 commented 4 years ago

Hey, I just recently found this jekyll docker image (https://hub.docker.com/r/jekyll/jekyll/), can I ask if there is any reason why you didn't use this instead of the ruby one? I personally have not tested it yet but it looks like it can get the job done EDIT: nevermind, that image seems outdated, and other jekyll images are not really suitable they mess up the cache and don't really decrease the dockerfile build time anyways.

helaili commented 4 years ago

Thanks @limjh16

The main reason I'm adding more packages (i.e. build-essential) is so I can run the jekyll-asciidoc gem out-of-the-box, and most likely some other jekyll plugins.

helaili commented 4 years ago

... oh, and I do like the cache optimization!

limjh16 commented 4 years ago

if I'm not wrong, the Ruby docker base image already contains gcc and make and a few other tools. that should be enough to run the plugin? I will try it tomorrow to confirm. also, sorry 😅 I accidentally pressed the close PR button on my phone...

limjh16 commented 4 years ago

Okay I fixed the error when building extensions, when I was testing it previously it worked but it was using the cache and not really building the extension that's why it was working

However to fix it I used the ruby base image instead of the slim one which increased the Docker build time by ~10sec, seems like this image is really huge (though it is still faster than the slim one +build-essentials)

I'm looking into using other base images or maybe creating a smaller one and uploading it to docker hub, though there are a lot of jekyll images images there already so I'm not sure if this is a good idea. I did a few experiments yesterday with the ruby alpine image, seems like the decreased image size may help here.

limjh16 commented 4 years ago

Seems like it worked, asciidoc and the previously failed eventmachine can both be built. Changing to alpine image also decreases the total build time to ~30sec, so that seems good. Successful workflow run can be seen here https://github.com/limjh16/jekyll-action/runs/603871282

EDIT: To skip past the docker image initialisation part and directly use the virtual environment inside actions, I made another action https://github.com/limjh16/jekyll-action/pull/3 but I'm not sure how to turn it into a action.yml file that others can use rather than just a workflow file, any ideas would be appreciated! Testing without the docker image yields total build times of ~15sec! That's faster than any CI I tried to build jekyll with.

EDIT 2: Seems like using the virtual environment that GitHub provides directly also allows for installing a specific Bundler version (https://github.com/ruby/setup-ruby/#bundler), which may help to fix issue #16

limjh16 commented 4 years ago

Just a suggestion: Maybe you can include a link to jekyll's docs in the README (it is currently in PR stage, not merged to their main website yet https://github.com/jekyll/jekyll/pull/8119) It explains how to use this action, and may be useful for beginners as they explain it step by step. And maybe they should include the caching step of this action?

helaili commented 4 years ago

To skip past the docker image initialisation part and directly use the virtual environment inside actions, I made another action limjh16#3 but I'm not sure how to turn it into a action.yml file that others can use rather than just a workflow file,

I've thought already about getting rid of the Docker part but like you said the packaging is problematic. It's a trade-of between performance and simplicity I guess.

limjh16 commented 4 years ago

Hey! sorry for all the spam, https://github.com/helaili/jekyll-action/pull/17/commits/af3293c2d6f25c063e2a3448433fe47d4d65b11c should have fixed https://github.com/helaili/jekyll-action/pull/17#discussion_r413017026 so the review issues seems resolved. However, with regards to the getting rid of docker, I made a separate branch testing a javascript action rather than a Docker action and it looks good, with runs taking ~15sec since we can skip the docker initialisation, and all as a action rather than a workflow file. Not sure whether you'd like me to PR that, since I haven't tested it with any repo other than my own so I cannot confirm whether it works for more use cases. EDIT: I have tested that javascript action with an older bundler version to fix #16 , https://github.com/limjh16/jekyll-action/commit/f7704f66eedb95e235786bd37b85038d19229170 it seems to work well.

helaili commented 4 years ago

Thanks a lot for all this ✨

Regarding the JS version, why don't you publish it under your account and I'll link to it in the readme as a better alternative? You deserve all the credit for it so it's only fair.

limjh16 commented 4 years ago

Regarding the JS version, why don't you publish it under your account and I'll link to it in the readme as a better alternative? You deserve all the credit for it so it's only fair.

Hey, thanks for the offer! I recently finished it and tested it on my own repo, it'll be great if you could link it, it's here: limjh16/jekyll-action-ts It's more complicated to set up but it also allows for custom ruby and bundler versions since I am using the ruby/setup-ruby action which allows for more flexibility

helaili commented 4 years ago

I've updated the README and mentioned your version @limjh16. Thanks again!