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 TemplateHaskell as a Global Default Extension #2291

Merged
merged 2 commits into from
Apr 15, 2022

Conversation

mdimjasevic
Copy link
Contributor

@mdimjasevic mdimjasevic commented Apr 15, 2022

TemplateHaskell as a language extension can lead to unnecessary recompilations. To aid GHC in figuring out what needs recompilation, this PR removes TemplateHaskell as a global default extension and moves it to modules that do need it. Hopefully this reduces the compilation time by a noticeable amount of time.

The one-line change that triggered the rest is the removal of TemplateHaskell from package-defaults.yaml.

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

- The driving change comes deleting the language extension from
package-defaults.yaml. This leads to changes to all the Cabal files,
which then leads to updating specific modules that actually depend on
the extension.
@mdimjasevic mdimjasevic temporarily deployed to cachix April 15, 2022 08:19 Inactive
Copy link
Contributor

@fisx fisx left a comment

Choose a reason for hiding this comment

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

Did you take the time before and after? Either way, as long as it doesn't slow down compilation I'm in favor. :)

@mdimjasevic
Copy link
Contributor Author

Did you take the time before and after? Either way, as long as it doesn't slow down compilation I'm in favor. :)

I didn't. At first and without too much thinking, I thought I'd see some time savings in the CI runs for this PR. However, I think it's the same time as before because everything is compiled from scratch. Now I am expecting time savings while developing as that's when we edit already compiled modules and recompile them.

@isovector brought this up in one of our pairing sessions. I don't have a good reference for this, though I tried looking it up. To some extent these are relevant:

@mdimjasevic
Copy link
Contributor Author

And this cannot slow down the compilation as here we're making the reach of TemplateHaskell strictly smaller, suggesting GHC knows more about what needs to be recompiled.

@mdimjasevic
Copy link
Contributor Author

I'll proceed with merging. In the unlikely case of us having a proof this slows down the compilation, it can easily be reverted.

@mdimjasevic mdimjasevic merged commit 0dde5a7 into develop Apr 15, 2022
@mdimjasevic mdimjasevic deleted the template-haskell-per-module branch April 15, 2022 09:43
@fisx
Copy link
Contributor

fisx commented Apr 15, 2022

git co template-haskell-per-module
make clean
time make
git revert --no-commit a776512b0035eabf0f941ea31aa0e4d00e40fa45
make clean
time make

=>

real	8m33.984s
real	8m39.574s

i don't think that's significant, so all good!

@isovector
Copy link
Contributor

The performance improvements here are going to be in incremental builds. GHC maintains interface files which hash their exports; subsequent files only need to get rebuilt if one of their imports gets changed. TemplateHaskell messes with this machinery, where it doesn't check which imports are used by splices; thus, if any dependency of a module with TH changes, the TH module will rebuild, and invalidate the build artifacts of everything downstream.

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.

None yet

3 participants