Plugin dependency information is stored separately
ClosedPublic

Authored by christiaanb on Jul 5 2018, 9:54 AM.

Details

Summary

We need to store the used plugins so that we recompile
a module when a plugin that it uses is recompiled.

However, storing the ModuleNames of the plugins used by a
module in the dep_mods field made the rest of GHC think
that they belong in the HPT, causing at least the issues
reported in Trac #15234

We therefor store the ModuleNames of the plugins in a
new field, dep_plgins, which is only used the the
recompilation logic.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
christiaanb created this revision.Jul 5 2018, 9:54 AM

Did you test this causes recompilation when the plugin module changes?

@mpickering No I didn't test it. But now on the offical ghc8.6rc1-alpha I'm having great difficulties triggering a recompilation of A.hs that uses the B.hs plugin by changing the source of B.hs when B is declared a pure plugin.

Did you test this causes recompilation when the plugin module changes?

Sounds like we could write a test for something like this. @christiaanb Let me know if you need any help with that once you've got everything else figured out.

Alright... I now understand how actually work checkDependencies: It checks whether all the current imported modules are equal to the imported modules from the last compilation run. If there's any difference, a recompile is triggered.
So what used to happen, before @mpickering's patch, is that plugin modules were never added to the field that stores the previously imported modules, and so plugins always triggered recompilation.
So after @mpickering patch, the plugins got added to the field that stores the previously imported modules, and so recompilation is no longer triggered automatically.
All my patch is currently doing is storing those plugins in a different field, which is only used by checkDependencies, and doesn't mess up the rest of GHC.

So how do we trigger recompilation once a plugin changes? I tried using the fingerprints in the interface files, but that's basically a non-option.
Why? because interface fingerprints are relatively stable unless you compile your plugin with optimisations turned on and give everything an INLINE pragma.
The next option I want to explore is instead depend on the hashes of the binary object files of the plugin.

Did you test this causes recompilation when the plugin module changes?

Sounds like we could write a test for something like this. @christiaanb Let me know if you need any help with that once you've got everything else figured out.

We have a test case now which I assume @christiaanb will add or I will afterwards.

https://github.com/mpickering/plugin-recompilation-test

mpickering requested changes to this revision.Jul 6 2018, 11:07 AM
This revision now requires changes to proceed.Jul 6 2018, 11:07 AM
  • Plugin dependency information is stored separately
  • Track object files of plugin in usages
  • Track object files of plugin in usages
  • Track object files of plugin in usages

@mpickering @bgamari This patch is ready for review now

bgamari requested changes to this revision.Jul 12 2018, 4:44 PM

Looks pretty sensible to me but more comments are needed and I'm rather suspicious of the lack of error handling.

compiler/deSugar/DsUsage.hs
129

Can you add a Note explaining how we account for plugin dependencies and the rationale for this design. The comment you gave in the code review discussion above would be a great start.

160

Shouldn't we panic in this case? I can't see why we would expect this case to be hit

compiler/main/HscTypes.hs
2376

I'll clarify this when I merged: "used while compiling this module."

This revision now requires changes to proceed.Jul 12 2018, 4:44 PM
  • Plugin dependency information is stored separately
  • Track object files of plugin in usages
mpickering accepted this revision.Jul 17 2018, 10:42 AM
  • Plugin dependency information is stored separately
  • Track object files of plugin in usages
  • Track object files of plugin in usages
bgamari accepted this revision.Aug 1 2018, 1:20 PM

Alright, looks good to me.

This revision is now accepted and ready to land.Aug 1 2018, 1:20 PM
This revision was automatically updated to reflect the committed changes.