codewars / runner

Issue tracker for Code Runner
34 stars 8 forks source link

Add Haskell GHC 8.8.x (LTS 16.x) with `-O1` #9

Closed Bubbler-4 closed 3 years ago

Bubbler-4 commented 5 years ago

Name: Haskell Version: Haskell 8.2.2 (LTS 10.4) -> Haskell 8.6.3 (LTS 13.4) Release Notes

IMO, the biggest change is in the base package in version 4.11, which includes the enhanced Semigroup and Monoid along with their exposure to Prelude and improved instances of Maybe. Plus, albeit not documented, liftA2 is now in Prelude.

While I don't think updating the GHC and base will break anything, switching the Stackage LTS version might involve a few big changes in other bundled packages.

Bubbler-4 commented 5 years ago

Another big change is the time package, which deals with date and time. (no breaking change, just lots of enhancements)

kazk commented 5 years ago

I can create 8.6 and see the effects of update. If it's negligible, I'll swap it. If it's not, I'll see what we can do.

Voileexperiments commented 4 years ago

What's the runner difference between ghc 8.2 to ghc 8.6? If 8.2 and 8.6 are not too different maybe we can just skip 8.2 altogether.

kazk commented 4 years ago

I'm not planning to change anything so the difference will be from the compiler and package versions. The latest LTS Haskell is LTS 15.11 (ghc-8.8.3).

namedots commented 3 years ago

I'd like to see runhaskell changed to ghc -O1 Main.hs && ./Main Some code is disproportionally affected by not being compiled, making for surprisingly abysmal performance (compilation is expected and relied on, rewrite rules need to fire for some things to even have correct time complexity)

Regarding lts: if the snapshot version isn't going to be updated for a long time then it doesn't matter if you go with lts or nightly. long term support has no effect when not coming back for that support. ghc 8.10 has been out for a while now, the packages that codewars requires are most likely there.

I'd also like to see the package extra in there, and perhaps some alternative preludes.

Yes, I do see the previous comment about not planning to change anything other than versions.

kazk commented 3 years ago

@namedots I honestly had forgotten about this, so thanks for reminding me.

We don't use runhaskell for GHC 8.2:

stack ghc -- -j2 +RTS -A128m -n2m -RTS --make -v0 -outputdir /tmp test/Main.hs -o test && ./test

The authors (or solvers) can add other flags with OPTIONS_GHC if needed.


I can add the latest LTS after cleaning up the remaining GHC 7.x kata and replacing it. We don't need to worry about breaking changes. Then remove GHC 8.2 if it's compatible with existing kata.

namedots commented 3 years ago

Oh. Hmm. I was seeing some not-at-all-funny performance. I tested that code locally just now with and without -O1 -O0 : 6.7seconds -O1: 1.1seconds

please add -O1 in there! <3 I'd even prefer 8.2 with -O1 over any other changes, kata authors/solvers won't be adding it and it's needed in both those files The argument against is compilation time, right? But this is small amounts of code, doing heavy work. Seems like a Good Idea™ to me.

No chance of 8.10.2 then? Otherwise we update to something that's already getting old. I imagine all you would be doing is writing nightly-2020-12-07 in place of lts-16.25 and if you could close your eyes and add extra, protolude. ... it's not important but I will ask :D

Anyway. Regardless of what I/we get. Thank you :3

kazk commented 3 years ago

it's needed in both those files

Why can't the author update them? Note that the compilation time is included in the time limit and optimization can increase it significantly in some cases. I think it's better to give the authors control when it's possible.

Does OPTIONS_GHC have precedence over CLI flags? If so, we won't lose the flexibility by adding -O1 by default.


For nightly or not, I just prefer stability to bleeding edge as a maintainer. But I can consider adding one after replacing the current versions with the latest LTS (so newest LTS + nightly instead of 7.10 + 8.2). It might be also helpful to prepare for any future changes.

Why doesn't 8.10 have LTS if it's been out that long and it's usable?

namedots commented 3 years ago

Why doesn't 8.10 have LTS if it's been out that long and it's usable?

To my understanding they're waiting for some linking issue on windows

Why can't the author update them?

individual ones can, of course, but getting authors and translators to do it, and having the extra option in the the solution too since it is per file.. not great.

It does seem that {-# OPTIONS_GHC -O0 #-} in a file has precedence over the commandline -O1 (I just tested) That still allows authors to override it.

I think the small amount of code involved makes -O1 a perfectly reasonable default, and it often has a large effect in haskell, it's not just a little bit.

kazk commented 3 years ago

That still allows authors to override it.

Cool, then we can add -O1 by default for the newer versions and see how that goes.

namedots commented 3 years ago

Speaking of compilation time, I suspect invoking the stack executable is adding some (because it does a bunch of housekeeping, but there's probably no housekeeping to do here)

It's not something I care to warrior for (it's probably out of scope here too) but I thought I'd point it out in the context of -O1 possibly adding time. One way to get rid of it is to toss out stack, install things using cabal instead.

Losing a flat half second isn't something I care about... The problems I've been encountering is that things run at 15% of expected speed. :^)

$ time stack ghc --resolver=nightly-2020-10-23 -- --version
The Glorious Glasgow Haskell Compilation System, version 8.10.2

real    0m0.489s
user    0m0.395s
sys     0m0.070s
$ time ghc --version
The Glorious Glasgow Haskell Compilation System, version 8.10.2

real    0m0.064s
user    0m0.029s
sys     0m0.025s
JohanWiltink commented 3 years ago

If the setup time per solution goes from two to five seconds, some kata are going to need 15 instead of 12 seconds total, because sometimes longer compilation time is not offset by shorter run time.

Would those be the kata that need OPTIONS_GHC -O0 ? I'd be hesitant to invalidate existing solutions that way.

Case in point: The Dollar Game

namedots commented 3 years ago

If this (a change to 8.2 in the last 24h) was an -O1 experiment for 8.2:

Yes, this is worse, and should be reverted :(. It is worse even for code that already specifies OPTIONS_GHC -O1 (Maybe there's a Main.hs taking time to compile, but doing no significant amount of work when running)

kazk commented 3 years ago

🤦‍♂️ All I did was add persistent package and rebuild the image so I can update the last kata that required 7.x. Nothing to do with the optimization flags. Probably some update to the base image is affecting performance.

JohanWiltink commented 3 years ago

7.x is not slower though

kazk commented 3 years ago

Yeah, I haven't touched 7.x image. I just added persitent package to 8.2 image and rebuilt it.

My plan was to:

  1. Update all kata to use 8.2
  2. Remove 7.x
  3. Add 8.8 with -O1. Try to upgrade all compatible kata to understand its effect.
  4. Remove 8.2 once every kata uses 8.8
  5. Consider adding nightly

I can revert the change and ignore the one kata for now.

JohanWiltink commented 3 years ago

I would LOVE to have 8.8!

You're not willing to have three versions?

namedots commented 3 years ago

My thought with 8.10 was that 8.8 could be skipped, same as 8.4 and 8.6 If doing 8.8 then (I at least) don't care about 8.10. I just want to move on to whatever is deemed the most recent among reasonable choices and if that's 8.8, sure. 8.10 additionally may mean next upgrade can be further away, while if 8.8 exists, 8.10 might not be enough of a leap to matter much. Anyway, whatever makes sense to you. Whatever gets us past 8.2.

Changing lts may come with breaking library changes. Idk what those would be, just pointing out that the current 8.2 kata aren't necessarily compatible :/

kazk commented 3 years ago

You're not willing to have three versions?

No, unless it's absolutely necessary for some reason.

I try to keep the number of versions per language at most 2 (some languages like JavaScript are exceptions because of the technical dept). It's just much easier to maintain. Haskell images are not small, so it also adds up to affect the time to build runner VMs (pulling all the language images takes about 40 minutes at the moment) and deploy them.

Having two versions makes updating language versions easier. The community can prepare for the new version by updating all kata to the newer version. Then I'll replace the older one with the newest.

All Haskell kata are compatible with 8.2 now (ignoring the SQL one that needs persistent) thanks to the help from some users. It was a lot of work because 7.10 had a hacky implementation that required changes to how tests work. We can finally remove 7.10, add 8.8 with -O1 and see how compatible they are.

To be clear, the runner can easily add new language versions. But having too many language versions have significant cost. It's also not great for the users because this usually means the supported version varies significantly by each kata (e.g., JS on Codewars often confuses users because of different Node versions).

If you'd like to have your favorite language to receive more frequent updates, please help to update existing kata :)

Changing lts may come with breaking library changes. Idk what those would be, just pointing out that the current 8.2 kata aren't necessarily compatible :/

Exactly. That's why I'm not replacing it.

kazk commented 3 years ago

I reverted the change to the 8.2 image.

kazk commented 3 years ago

@namedots

Oh. Hmm. I was seeing some not-at-all-funny performance.

If you meant the compilation time, maybe it's related to caching?

Although stack began its life with the best-of-class caching, cabal has now caught up and overtaken. stack struggles to support changing compiler flags (e.g. swapping between --fast builds and regular builds) whereas cabal can swap between -O0 / -O1 / -O2 without missing a beat. stack is unable to share the caches of extra-deps or git sources, whereas cabal treats everything equally and can share builds between projects.

https://medium.com/@fommil/why-not-both-8adadb71a5ed

The runner executes each submission in an ephemeral container, so nothing is persisted. This can make a significant difference when you compare the performance against your local setup. Maybe I can compile some sample code during image build with different flags to populate cache.

I'll try avoiding stack for 8.8.

JohanWiltink commented 3 years ago

So will GHC 7.x be removed and GHC 8.8 added, soonish? Or does something need to be figured out for "the SQL kata that needs persistent" yet?

I'm quite willing to help, but there seem to be no GHC 7.x kata left ..

kazk commented 3 years ago

Yes, I'll remove 7.x and add 8.8 soonish. Ignoring SQL kata for now. I'm pretty sure it can be updated later to 8.8 quickly because I had fixed it last night to work with 8.2 with persistent package. We can also have 3 versions temporarily if necessary.

There's nothing for the community to help at the moment in terms of existing content. Thanks for helping with the update to 8.2 :) I'll update the list after deploying 8.8 and updating compatible kata.

I might ask for help if I run into any issues building a new image for 8.8.

JohanWiltink commented 3 years ago

I was hoping for that ( three versions simultaneously, temporarily, if unavoidable ), but I didn't dare say it out loud. :yum:

namedots commented 3 years ago

If you meant the compilation time, maybe it's related to caching?

Once the program gets to running I expect somewhat comparable characteristics to what I do locally. Getting Main.hs precompiled does sound like a good idea. I don't think stack vs cabal is very significant. If you want a LTS snapshot then you want stack. This is totally fine, this already works. Having a global ghc, skipping stack/cabal executable when running, that shaves off a little bit. *shrug*

Me, I care about upgrading to something reasonable + hopefully being able to have -O1 because then the world makes sense.

kazk commented 3 years ago

I found out we might not need Stack to use LTS snapshot. We can freeze the package versions according to the snapshot with https://www.stackage.org/lts-16.25/cabal.config. But the file has a warning pointing to this issue: https://github.com/fpco/stackage-server/issues/232.

Anyway, I got a project for Haskell kata set up like the following:

-- challenge.cabal
cabal-version: >=1.10

name: challenge
version: 0.1.0
synopsis: Haskell challenge
build-type: Simple

library
  hs-source-dirs:
      src
  -- package versions are frozen to the LTS snapshot
  build-depends:
      base ==4.*
    , attoparsec
    , haskell-src-exts
    , hspec
    , hspec-attoparsec
    , hspec-codewars
    , hspec-contrib
    , hspec-formatters-codewars
    , hspec-megaparsec
    , HUnit-approx
    , lens
    , megaparsec
    , mtl
    , parsec
    , persistent
    , persistent-sqlite
    , persistent-template
    , random
    , regex-pcre
    , regex-posix
    , regex-tdfa
    , split
    , text
    , transformers
    , vector
  default-language: Haskell2010
-- cabal.project
packages: .

-- own packages
source-repository-package
  type: git
  location: https://github.com/codewars/hspec-formatters-codewars
  tag: v0.1.0

source-repository-package
  type: git
  location: https://github.com/codewars/hspec-codewars
  tag: v0.1.0

Having a global ghc, skipping stack/cabal executable when running, that shaves off a little bit.

stack ghc and cabal exec -- ghc does some magic to set up the correct environment for GHC to find all the packages. I'd like to use GHC directly as well, but having trouble finding what exactly it does.

kazk commented 3 years ago

I decided to use Stack, but without installing GHC through it. I'll also continue to use stack ghc --.

The following are the numbers on my laptop, so it's different from what you'll see, but it's faster:

GHC 8.8 has -O.

kazk commented 3 years ago

@namedots @JohanWiltink Feel free to open new issues for package requests. I can add them if it's widely used or if you have a specific use case.

Current package.yaml:

name: challenge
version: 0.1.0
license: BSD3
synopsis: Haskell challenge

dependencies:
- base >= 4.7 && < 5
- attoparsec
- haskell-src-exts
- hspec
- hspec-attoparsec
- hspec-codewars
- hspec-contrib
- hspec-formatters-codewars
- hspec-megaparsec
- HUnit-approx
- lens
- megaparsec
- mtl
- parsec
- persistent
- persistent-sqlite
- persistent-template
- random
- regex-pcre
- regex-posix
- regex-tdfa
- split
- text
- transformers
- vector

library:
  source-dirs: src
kazk commented 3 years ago

Deployed GHC 8.8.4. We have 23 kata not compatible with it. Please help to update them.

Also, let me know if you notice anything weird or slow. We can try to improve it.

JohanWiltink commented 3 years ago

Yet another Collatz kata required -O0 in the tests. Incidentally, there is a 20 000 element [ Int ] literal in those tests. I don't know if that's related.

FArekkusu commented 2 years ago

Not sure which repo is appropriate, so I'll say it here: the Haskell tests template was not updated after removing the older language versions, and it still contains obsolete code from those times.

-- the following line is optional for 8.2
main = hspec spec
kazk commented 2 years ago

Those are still optional and won't do any harm, but I'll update it.