Skip to content

haskellPackages: build with RTS -A64M options#317251

Merged
sternenseemann merged 1 commit intoNixOS:haskell-updatesfrom
wolfgangwalther:rtsopts
Jun 10, 2024
Merged

haskellPackages: build with RTS -A64M options#317251
sternenseemann merged 1 commit intoNixOS:haskell-updatesfrom
wolfgangwalther:rtsopts

Conversation

@wolfgangwalther
Copy link
Contributor

While working on #317181 I realized that the +RTS -A64M -RTS options in parallelBuildingFlags were never passed to GHC, because the arguments are not passed quoted. This means those arguments are actually passed to Setup.hs. This can be verified by removing -rtsopts from setupCompileFlags - after this change the configure command will throw an error, because it doesn't understand the +RTS ... argument anymore. Once you pass it properly, the error is gone again and GHC receives the arguments.

Passing the RTS flags to Setup.hs (i.e. cabal) - makes little to zero sense. The same is true for the entire parallelBuildingFlags being passed to setupCompileFlags: If there was any custom Setup.hs script in any package anywhere that benefits from heavy parallelization like this... there would be something wrong with that script! ;)

Those flags were introduced in #86948. The related twitch live stream uses the build of git-annex as a measurement. I get the following numbers when building git-annex with doCheck = false;:

  • current master: 1:40 wall clock / 340s user
  • without -A64M: 1:40 wall clock / 340s user
  • with this fix: 1:13 wall clock / 280s user

This 27% improvement is essentially the result from that live stream mentioned above. Let's use that, shall we?

Things done

  • Built on platform(s)
    • x86_64-linux: haskell.lib.overrideCabal haskellPackages.git-annex { doCheck = false; }
      [...]
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label Jun 4, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jun 4, 2024
@sternenseemann sternenseemann self-assigned this Jun 8, 2024
Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Otherwise this makes sense to me. -A64M is beneficial in multi-threaded situations. As far as I'm aware, Cabal is single-threaded for our usecase while ghc (even with our current escaping mess) receives a -j flag.

@sternenseemann
Copy link
Member

My testing with lens seems to confirm what you are saying (though I got very noisy data).

@wolfgangwalther wolfgangwalther force-pushed the rtsopts branch 2 times, most recently from c91ca68 to 0b1a452 Compare June 10, 2024 06:47
Those flags were not actually passed to GHC before, but to Setup.hs.

They were introduced in NixOS#86948. The related twitch live stream uses the
build of git-annex as a measurement. I get the following numbers when
building git-annex with doCheck = false:

 - for current master: 1:40 wall clock / 340s user
 - without any -A64M argument: 1:40 wall clock / 340s user
 - with this fix: 1:13 wall clock / 280s user

The idea was good, but the settings were never active.

More testing revealed that this seems to work on darwin just as well, so
we're removing the isLinux condition, too.
@sternenseemann sternenseemann merged commit e160c2a into NixOS:haskell-updates Jun 10, 2024
@sternenseemann
Copy link
Member

We should consider backporting this and #317181 after testing for regressions (#318363)!

@sternenseemann sternenseemann added the 9.needs: port to stable A PR needs a backport to the stable release. label Jun 10, 2024
@wolfgangwalther wolfgangwalther deleted the rtsopts branch June 10, 2024 15:16
@sternenseemann sternenseemann added backport staging-24.05 and removed 9.needs: port to stable A PR needs a backport to the stable release. labels Jun 20, 2024
@github-actions
Copy link
Contributor

Successfully created backport PR for staging-24.05:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants