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

Remove duplicate compilation #80

Closed

Conversation

basvandijk
Copy link
Contributor

We're noticing that on #79 GHC
requires massive amounts of memory to build certain
modules (IC.Test.Agent being the worst offender requiring more than
13GB while GitHub Runners only have 7GB causing OOM kills).

What's worse is that multiple components in the cabal file require the
same modules causing the same module to be build multiple times, often
in parallel exacerbating the memory problem.

This fixes the duplicate compilation issue by letting the components
in the cabal file only depend on the ic-hs library.

This might be deterimental to ghcid as mentioned in the cabal
file. But the associated ticket has been closed so the problem is
hopefully fixed in an upcoming GHC release.

marcin-dziadus and others added 15 commits May 20, 2022 08:23
… OOM

I've seen GHC require >16GB of RES while GitHub runners only have 7GB
of RAM.
We're noticing that on #79 GHC
requires massive amounts of memory to build certain
modules (`IC.Test.Agent` being the worst offender requiring more than
13GB while GitHub Runners only have 7GB causing OOM kills).

What's worse is that multiple components in the cabal file require the
same modules causing the same module to be build multiple times, often
in parallel exacerbating the memory problem.

This fixes the duplicate compilation issue by letting the components
in the cabal file only depend on the ic-hs library.

This might be deterimental to ghcid as mentioned in the cabal
file. But the associated ticket has been closed so the problem is
hopefully fixed in an upcoming GHC release.
@basvandijk basvandijk requested a review from nomeata May 23, 2022 15:55
@basvandijk
Copy link
Contributor Author

basvandijk commented May 23, 2022

@nomeata is the ghcid issue still a problem these days?

@nomeata
Copy link
Contributor

nomeata commented May 24, 2022

Yes, multi-packages support in GHC has not reached all the tools, I believe.

One could do a very elaborate .cabal file with flags where there are internal libraries during normal compilation, and common stanzas for iterative development.

Maybe the massive memory consumption can be avoided instead, which would be beneficial in any case, by splitting a module or a suitable compiler flag. And not building in parallel.

What I'd also love to do is to use a better CI system than the slow runners we get for free, if we aren't that cheap. Hooking this up to nixbuilds.net should be simple and if dfinity crates an account there, I can do the setup.

@basvandijk
Copy link
Contributor Author

One could do a very elaborate .cabal file with flags where there are internal libraries during normal compilation, and common stanzas for iterative development.

I hope we can get the cabal file as simple as possible. What about the alternative you mentioned in the cabal file:

-- We could consider the alternative of putting everything, including Main modules
-- into one library; then ghcid -T would work again.

Would that work for you? If so, I can push it to this PR.

Maybe the massive memory consumption can be avoided instead, which would be beneficial in any case, by splitting a module or a suitable compiler flag. And not building in parallel.

Yes, I was already planning to start splitting up modules. But need to find some time for it.

What I'd also love to do is to use a better CI system than the slow runners we get for free, if we aren't that cheap. Hooking this up to nixbuilds.net should be simple and if dfinity crates an account there, I can do the setup.

Thanks for the offer to set it up! I'm for it and will propose this in our team.

@nomeata
Copy link
Contributor

nomeata commented May 24, 2022

Would that work for you? If so, I can push it to this PR.

Yeah, I guess that is a valid work-around. What I don’t like about that is that now all binaries depend on all modules, but I guess we can’t have it all :-)

@basvandijk
Copy link
Contributor Author

@nomeata I setup a dfinity account at nixbuild.net and I'm in the process of adding the org's credit card. But we'll have 25 free hours so we can already get started.

What's your public SSH key? Then I'll add that to the account so you can start setting it up.

@nomeata
Copy link
Contributor

nomeata commented May 24, 2022

Thanks!

ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIJRd0CZZQXyKTEQSEtrIpcTg15XEoRjuYodwo0nr5hNj jojo@kirk

Sorry that I forgot that they may not support darwin. But maybe customer demand will fix that :-)

I’ll also use this to speed up Motoko CI; they share the same nix cache anyways.

It might be a few days or a week before I’ll get to it, though.

@basvandijk
Copy link
Contributor Author

When adding your key I'm getting the error:

Key can't be added, it exists in another account

We prioritised ic-hs in our team so I'll go ahead and start integrating nixbuild.

ic-hs.cabal Show resolved Hide resolved
@basvandijk
Copy link
Contributor Author

@nomeata I'm not a ghcid user so I would like to understand that use case a bit better. How are you actually using ghcid, or ghci for that matter, in ic-hs?

When I try to load ic-ref.hs ghci complains that it can't find IC.Crypto.BLS which makes sense because that's a .hsc file:

(BTW note that I created symlinks to all binaries in src/ such that you can load them in ghci).

[nix-shell:~/d/ic-hs/ic-hs/src]$ ghci
GHCi, version 8.10.7: https://www.haskell.org/ghc/  :? for help
Prelude> :load ic-ref.hs
[ 1 of 49] Compiling IC.CBOR.Patterns ( IC/CBOR/Patterns.hs, interpreted )
[ 2 of 49] Compiling IC.CBOR.Parser   ( IC/CBOR/Parser.hs, interpreted )
[ 3 of 49] Compiling IC.Canister.StableMemory ( IC/Canister/StableMemory.hs, interpreted )
[ 4 of 49] Compiling IC.Constants     ( IC/Constants.hs, interpreted )
[ 5 of 49] Compiling IC.Crypto.Bitcoin ( IC/Crypto/Bitcoin.hs, interpreted )
[ 6 of 49] Compiling IC.Crypto.DER.Decode ( IC/Crypto/DER/Decode.hs, interpreted )
[ 7 of 49] Compiling IC.Crypto.DER    ( IC/Crypto/DER.hs, interpreted )
[ 8 of 49] Compiling IC.Crypto.DER_BLS ( IC/Crypto/DER_BLS.hs, interpreted )

IC/Crypto/DER_BLS.hs:19:1: error:
    Could not find module ‘IC.Crypto.BLS’
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
   |
19 | import qualified IC.Crypto.BLS as BLS
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Failed, 7 modules loaded.
*IC.Crypto.DER>

How do you work around that?

@nomeata
Copy link
Contributor

nomeata commented May 29, 2022

Here is how I use ghcid:

~/dfinity/ic-hs $ ghcid -c 'cabal repl ic-ref-test' --test Main.main --setup ":set args -p \"metadata\" --rerun"

this loads ic-ref-test, when it compiles, run the main with the given flags.

Base automatically changed from marcin/ecdsa-sig to master May 30, 2022 06:43
@basvandijk basvandijk closed this Sep 1, 2022
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.

3 participants