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 ~/.m2/ to speed up GH actions #1109

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

fbricon
Copy link
Contributor

@fbricon fbricon commented Aug 31, 2021

No description provided.

@fbricon fbricon force-pushed the gh-cache-m2-repo branch 2 times, most recently from 33943b4 to ab1ca90 Compare September 3, 2021 13:46
@fbricon fbricon requested a review from rgrunber September 3, 2021 14:25
@fbricon
Copy link
Contributor Author

fbricon commented Sep 3, 2021

Before this change (in master)
Screenshot 2021-09-03 at 16 23 30

After this change (building this PR)
Screenshot 2021-09-03 at 16 23 53

So build time improvement ranges from 3 to 4 times faster, provided the cache can be reused (which will work most of the time, except if pom.xml or native-image.yaml changes)

!~/.m2/repository/org/eclipse/lemminx
key: ${{ runner.os }}-${{ hashFiles('**/pom.xml') }}
restore-keys: |
${{ runner.os }}-node
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious about this value. Looking at https://docs.github.com/en/actions/guides/caching-dependencies-to-speed-up-workflows#matching-a-cache-key, this would match any cache key starting with ${{ runner.os }}-node and the most recent if there are multiple , but there aren't any that contain the "node" in the key declaration. Is it some pre-defined key somewhere else ?

Copy link
Contributor

@rgrunber rgrunber Sep 10, 2021

Choose a reason for hiding this comment

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

@fbricon , I think this should just be ${{ runner.os }}- according to the docs. Let me know if that makes sense. I wouldn't mind trying this out on on JDT-LS as well.

@fbricon
Copy link
Contributor Author

fbricon commented Sep 10, 2021

as per https://github.com/actions/cache/blob/main/examples.md#java---maven, it should actually be ${{ runner.os }}-maven

@rgrunber
Copy link
Contributor

as per https://github.com/actions/cache/blob/main/examples.md#java---maven, it should actually be ${{ runner.os }}-maven

That's fine. So the key should be ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml') }} to match.

@fbricon
Copy link
Contributor Author

fbricon commented Sep 10, 2021

Actually, we should split the cache in 2, for Maven and Graalvm, but I don't know what to use for graalvm restore key

@fbricon fbricon force-pushed the gh-cache-m2-repo branch 2 times, most recently from b94956b to 9bf07ba Compare September 10, 2021 15:05
Signed-off-by: Fred Bricon <fbricon@gmail.com>
Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

Looks fine to me. We can hopefully take a similar approach in JDT-LS.

@fbricon fbricon merged commit 950b68b into eclipse-lemminx:master Sep 10, 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