Is there any reason not to make -haddock
the default and introduce --no-haddock
to disable it?
Pepe Iborra (32491265) at 13 Nov 09:54
drop instance Semigroup InstalledModuleEnv
... and 56 more commits
We are using plusModuleEnv
so the Semigroup
instance for ModuleEnv
is not needed. But there is no equivalent for InstalledModuleEnv
, so I have updated the MR to replace the Semigroup
instance with a plusInstalledModuleEnv
function
InstalledModuleEnv is abstract and lacks a Monoid
instance.
This causes problems downstream:
This PR adds a function plusInstalledModuleEnv
.
(spin-off from !6922)
Pepe Iborra (bb7fc117) at 08 Nov 19:14
Monoid instance for InstalledModuleEnv
... and 3736 more commits
@mpickering no rush, we have workarounds in HLS for the missing instances.
Should we embed the NodeKey in the ModuleGraphNode datatype to avoid duplicate calls to mkNodeKey
?
Also, the import of GHC.Driver.Make
introduces an import cycle. What's the best way to break it, boot module?
I included two links in the description, did you miss them?
I noticed that these two data structures are abstract and don't have efficient union operations. The use case is composing HPTs and ModuleGraphs for efficiently incremental load of module interfaces in HLS:
Pepe Iborra (2d134f54) at 06 Nov 07:59
Monoid instance for InstalledModuleEnv
... and 3735 more commits
Attaching both the minimal and non-minimal module graphs (with module names elided) modulegraphs.tar.xz
╰─>$ cat dump | sort | uniq -c | wc -l
53914074
$ cat dump
...
check
Nodeapi.Edges.AccessibilityOnGlobalRootPage Duckling.CreditCardNumber.Types 4 3
check
Nodeapi.Edges.AccessibilityOnGlobalRootPage Duckling.Distance.Types 4 4
check
Nodeapi.Edges.AccessibilityOnGlobalRootPage Duckling.Duration.Types 4 2
check
Nodeapi.Edges.AccessibilityOnGlobalRootPage Duckling.Email.Types 4 2
check
Nodeapi.Edges.AccessibilityOnGlobalRootPage Duckling.Locale 4 2
...
The workload is a module with ~12k transitive dependencies, all them autogenerated since it is an Interop layer. I'm not really familiar with the structure, ping @watashi for more details, but there is a large number of class and type instances, and multiple reexports. The body of the example module itself is irrelevant.
If I replace the module imports by the minimal set, the number of transitive dependencies goes down by 40% (from 12k to 7k) and the typecheck time by 99.9% (from 7m to 0.5s).
Typechecking the example module, the pprTrace
above fires around 54 million times. The dump is 8GB big and hard to analyse, but I'll keep it around for a while in case you want to run some commands on it
This is CPU time, +RTS -pj
, obtained following the same procedure as the one in #19703. That is, using a profiled build of sigma-ide by loading the example and performing 10 whitespace edits. Attached: prof.tar.xz
I have tested the 8.8 backport on the extreme example in the Sigma codebase, and unfortunately the results aren't good. Here is a manually annotated profile, happy to run more tests if you give me the SCCs:
And these are the SCCs I added:
diff --git a/8.8.3/src/ghc-8.8.3/compiler/typecheck/FamInst.hs b/8.8.3/src/ghc-8.8.3/compiler/typecheck/FamInst.hs
index aec8e4153..d10e2b162 100644
--- a/8.8.3/src/ghc-8.8.3/compiler/typecheck/FamInst.hs
+++ b/8.8.3/src/ghc-8.8.3/compiler/typecheck/FamInst.hs
@@ -329,7 +329,7 @@ checkFamInstConsistency directlyImpMods
}
- ; checkMany hpt_fam_insts modConsistent directlyImpMods
+ ; {-# SCC "checkMany" #-} checkMany hpt_fam_insts modConsistent directlyImpMods
}
where
-- See Note [Checking family instance optimization]
@@ -699,7 +699,7 @@ environments (one for the EPS and one for the HPT).
checkForConflicts :: FamInstEnvs -> FamInst -> TcM Bool
checkForConflicts inst_envs fam_inst
- = do { let conflicts = lookupFamInstEnvConflicts inst_envs fam_inst
+ = do { let conflicts = {-# SCC "lookupFamInstEnvConflicts" #-} lookupFamInstEnvConflicts inst_envs fam_inst
; traceTc "checkForConflicts" $
vcat [ ppr (map fim_instance conflicts)
, ppr fam_inst
@@ -718,7 +718,7 @@ checkForInjectivityConflicts instEnvs famInst
-- type family is injective in at least one argument
, Injective inj <- tyConInjectivityInfo tycon = do
{ let axiom = coAxiomSingleBranch fi_ax
- conflicts = lookupFamInstEnvInjectivityConflicts inj instEnvs famInst
+ conflicts = {-# SCC "lookupFamInstEnvInjectivityConflicts" #-} lookupFamInstEnvInjectivityConflicts inj instEnvs famInst
-- see Note [Verifying injectivity annotation] in FamInstEnv
errs = makeInjectivityErrors fi_ax axiom inj conflicts
; mapM_ (\(err, span) -> setSrcSpan span $ addErr err) errs
diff --git a/8.8.3/src/ghc-8.8.3/compiler/typecheck/TcRnDriver.hs b/8.8.3/src/ghc-8.8.3/compiler/typecheck/TcRnDriver.hs
index d673fc884..22ba54e8f 100644
--- a/8.8.3/src/ghc-8.8.3/compiler/typecheck/TcRnDriver.hs
+++ b/8.8.3/src/ghc-8.8.3/compiler/typecheck/TcRnDriver.hs
@@ -317,7 +317,7 @@ implicitPreludeWarn
tcRnImports :: HscEnv -> [LImportDecl GhcPs] -> TcM TcGblEnv
tcRnImports hsc_env import_decls
- = do { (rn_imports, rdr_env, imports, hpc_info) <- rnImports import_decls ;
+ = do { (rn_imports, rdr_env, imports, hpc_info) <- {-# SCC "rnImports" #-} rnImports import_decls ;
; this_mod <- getModule
; let { dep_mods :: ModuleNameEnv (ModuleName, IsBootInterface)
@@ -379,7 +379,7 @@ tcRnImports hsc_env import_decls
; let { dir_imp_mods = moduleEnvKeys
. imp_mods
$ imports }
- ; checkFamInstConsistency dir_imp_mods
+ ; {-# SCC "checkFamInstConsistency" #-} checkFamInstConsistency dir_imp_mods
; traceRn "rn1: } checking family instance consistency" empty
; getGblEnv } }
diff --git a/8.8.3/src/ghc-8.8.3/compiler/types/FamInstEnv.hs b/8.8.3/src/ghc-8.8.3/compiler/types/FamInstEnv.hs
index 2eefe03a1..4a87c3ec4 100644
--- a/8.8.3/src/ghc-8.8.3/compiler/types/FamInstEnv.hs
+++ b/8.8.3/src/ghc-8.8.3/compiler/types/FamInstEnv.hs
@@ -3,6 +3,7 @@
-- FamInstEnv: Type checked family instance declarations
{-# LANGUAGE CPP, GADTs, ScopedTypeVariables, BangPatterns, TupleSections #-}
+{-# OPTIONS -fprof-auto-top #-}
module FamInstEnv (
-- * Family instances
diff --git a/8.8.3/src/ghc-8.8.3/compiler/types/InstEnv.hs b/8.8.3/src/ghc-8.8.3/compiler/types/InstEnv.hs
index 04d77cc56..45357a543 100644
--- a/8.8.3/src/ghc-8.8.3/compiler/types/InstEnv.hs
+++ b/8.8.3/src/ghc-8.8.3/compiler/types/InstEnv.hs
@@ -8,6 +8,7 @@ The bits common to TcInstDcls and TcDeriv.
-}
{-# LANGUAGE CPP, DeriveDataTypeable #-}
+{-# OPTIONS -fprof-auto-top #-}
module InstEnv (
DFunId, InstMatch, ClsInstLookupResult,
It is used by hls-tactics-plugin (and could be used by any other GHC API consumers
@trac-gershomb thank you!!! Cabal 3.4 worked just fine
I tried setting up a Windows VM to build HLS, and used ghcup to install ghc. Cannot get past from the network package:
Using the Win10 image for VirtualBox from https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/
We seem to have lost the instance Monoid FamInstEnv
, at least it's missing from https://gitlab.haskell.org/bgamari/ghc/-/tree/wip/T19703-ghc-8.8