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

Misc improvements #27

Merged
merged 5 commits into from
Nov 16, 2024
Merged

Misc improvements #27

merged 5 commits into from
Nov 16, 2024

Conversation

TerrorJack
Copy link
Member

  • Bump various stuff
  • Add cabal cache logic in CI
  • Apply -H64m to GHCRTS to reduce amortized gc overhead

Comment on lines 23 to 35
- name: wasm-cabal-cache
uses: actions/cache@v3
with:
key: nix-build-cache-${{ github.run_id }}
restore-keys: nix-build-cache-
path: |
~/.ghc-wasm/.cabal/store
dist-newstyle
Copy link
Member

Choose a reason for hiding this comment

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

This kind of cache is probably good enough for this repo (we also use it for Ormolu Live), but it has the notorious unbounded cache growth problem because nothing is ever deleted from the Cabal store. (Usually, this doesn't end up being a big problem because of a combination of sufficiently slow growth, cache expiry and manual deleting cache entries.)

I have been wanting to try out @andreabedini's https://github.com/andreabedini/cabal-cache-native that promises to do this properly, we could try it here. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice idea! And TIL cabal-cache-native, I'll give it a try instead of the home cooked cache here

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately the action doesn't seem to work for our case yet: https://github.com/tweag/ghc-wasm-miso-examples/actions/runs/11872593951

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, makes sense to revert to actions/cache in that case. Thanks for putting in the effort for trying it out!

Copy link
Member Author

Choose a reason for hiding this comment

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

No worries! The time is not wasted, given we have an extra step to generate plan.json now I'm hashing plan.json as cache key to avoid uploading the whole store each single time, let's see if that'll work

Copy link
Member Author

Choose a reason for hiding this comment

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

It works! I'm landing this patch. You might want to do something similar in ormolu. There's a slight downside that if plan.json doesn't change but home module source is changed, then there won't be a new cache with updated dist-newstyle and home modules may see more recompilation in ci. I think that's not a big deal though.

@TerrorJack TerrorJack added this pull request to the merge queue Nov 16, 2024
Merged via the queue into main with commit 794d769 Nov 16, 2024
4 checks passed
@TerrorJack TerrorJack deleted the develop branch November 16, 2024 20:45
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