Optimise common cases of GHC.setProgramDynFlags
ClosedPublic

Authored by simonmar on Mar 29 2017, 8:37 AM.

Details

Summary
  • If the package flags haven't changed, don't do initPackages (which might take multiple seconds in extreme cases)
  • Provide a way to change the log_action without invalidating the summary cache.
Test Plan

validate

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.
simonmar created this revision.Mar 29 2017, 8:37 AM
ezyang requested changes to this revision.Mar 29 2017, 11:51 AM
ezyang added inline comments.
compiler/main/DynFlags.hs
1272

What about package databases?

This revision now requires changes to proceed.Mar 29 2017, 11:51 AM
mpickering added inline comments.
compiler/main/GHC.hs
582

This could have unexpected results if log_finaliser has also been set.

simonmar updated this revision to Diff 11930.Mar 30 2017, 6:59 AM
  • packageFlagsChanged: check package DB flags too
  • setLogAction: set log_finaliser too
simonmar marked 2 inline comments as done.Mar 30 2017, 10:55 AM

I don't think the extraPkgConfs test is enough. If someone applies removeGlobalPkgConf we won't notice it if we run the function on the empty list. I guess the solution here is to get an actual data type (I never liked the function anyway.)

I also combed through Packages looking for fields of DynFlags and other data that is used during initialization, and I noticed the following:

  1. interpretPackageEnv probes for environment files in the file system, which could have changed when a reload happens. I don't think the "dynflags changed" mechanism should necessary do a file system test, but I'd like it to be obvious that we failed to pick up a package environment file because of the caching mechanism. (They can also come through via environment variables but if you are modifying your process env vars you are a bad man ;)
  2. Need Opt_HideAllPackages and Opt_HideAllPluginPackages
  3. Need Opt_AutoLinkPackages

There might be more that I missed. It might be safest if we carved out the parameters used by Packages initialization into their own type. Though, I know you're just trying to get an easy optimization going, and not into the yak shaving ;)

simonmar updated this revision to Diff 11939.Mar 31 2017, 4:07 AM

Make a data type for package DB flags

Wow. I didn't do anything about point (1), I don't think this is the right place to take into account environment files, and I'm not sure what you mean by "obvious that we failed to pick up a package environment file because of the caching mechanism".

simonmar updated this revision to Diff 11940.Mar 31 2017, 8:25 AM

fix warning

Well, that's the problem, I don't really know how to do that; all I know is that I change my environment file and ask GHC to "reload its configuration", it would be surprising if it didn't get picked up...

Let's call that a separate problem, I don't think I've made it worse here. Arguably I've made it better because :set won't put you in an inconsistent state after you change your environment file.

bgamari accepted this revision.Apr 1 2017, 10:51 AM

Thanks @simonmar.

I've opened Trac #13507 to track the environmentfile/GHCi issue.

This revision was automatically updated to reflect the committed changes.