Skip to content

Commit e84c6ed

Browse files
committed
Improve recompilation avoidance in the presence of TH
The old recompilation avoidance scheme performs quite poorly when code generation is needed. We end up needed to recompile modules basically any time anything in their transitive dependency closure changes. Most versions of GHC we currently support don't have a working implementation of code unloading for object code, and no version of GHC supports this on certain platforms like Windows. This makes it completely infeasible for interactive use, as symbols from previous compiles will shadow over all future compiles. This means that we need to use bytecode when generating code for Template Haskell. Unfortunately, we can't serialize bytecode, so we will always need to recompile when the IDE starts. However, we can put in place a much tighter recompilation avoidance scheme for subsequent compiles: 1. If the source file changes, then we always need to recompile a. For files of interest, we will get explicit `textDocument/change` events that will let us invalidate our build products b. For files we read from disk, we can detect source file changes by comparing the mtime of the source file with the build product (.hi/.o) file on disk. 2. If GHC's recompilation avoidance scheme based on interface file hashes says that we need to recompile, the we need to recompile. 3. If the file in question requires code generation then, we need to recompile if we don't have the appropriate kind of build products. a. If we already have the build products in memory, and the conditions 1 and 2 hold, then we don't need to recompile b. If we are generating object code, then we can also search for it on disk and ensure it is up to date. Notably, we did _not_ previously re-use old bytecode from memory when hls-graph/shake decided to rebuild the 'HiFileResult' for some reason 4. If the file in question used Template Haskell on the previous compile, then we need to recompile if any `Linkable` in its transitive closure changed. This sounds bad, but it is possible to make some improvements. In particular, we only need to recompile if any of the `Linkable`s actually used during the previous compile change. How can we tell if a `Linkable` was actually used while running some TH? GHC provides a `hscCompileCoreExprHook` which lets us intercept bytecode as it is being compiled and linked. We can inspect the bytecode to see which `Linkable` dependencies it requires, and record this for use in recompilation checking. We record all the home package modules of the free names that occur in the bytecode. The `Linkable`s required are then the transitive closure of these modules in the home-package environment. This is the same scheme as used by GHC to find the correct things to link in before running bytecode. This works fine if we already have previous build products in memory, but what if we are reading an interface from disk? Well, we can smuggle in the necessary information (linkable `Module`s required as well as the time they were generated) using `Annotation`s, which provide a somewhat general purpose way to serialise arbitrary information along with interface files. Then when deciding whether to recompile, we need to check that the versions of the linkables used during a previous compile match whatever is currently in the HPT. The changes that were made to `ghcide` in order to implement this scheme include: 1. Add `RuleWithOldValue` to define Rules which have access to the previous value. This is the magic bit that lets us re-use bytecode from previous compiles 2. `IsHiFileStable` rule was removed as we don't need it with this scheme in place. 3. Everything in the store is properly versioned with a `FileVersion`, not just FOIs. 4. The VFSHandle type was removed. Instead we now take a VFS snapshot on every restart, and use this snapshot for all the `Rules` in that build. This ensures that Rules see a consistent version of the VFS and also makes The `setVirtualFileContents` function was removed since it was not being used anywhere. If needed in the future, we can easily just modify the VFS using functions from `lsp`. 5. Fix a bug with the `DependencyInformation` calculation, were modules at the top of the hierarchy (no incoming edges) weren't being recorded properly A possible future improvement is to use object-code on the first load (so we have a warm cache) and use bytecode for subsequent compiles.
1 parent 5f57614 commit e84c6ed

File tree

8 files changed

+472
-114
lines changed

8 files changed

+472
-114
lines changed

Diff for: ghcide/src/Development/IDE.hs

+1-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ import Development.IDE.Core.IdeConfiguration as X (IdeConfiguration (.
1717
isWorkspaceFile)
1818
import Development.IDE.Core.OfInterest as X (getFilesOfInterestUntracked)
1919
import Development.IDE.Core.RuleTypes as X
20-
import Development.IDE.Core.Rules as X (IsHiFileStable (..),
21-
getClientConfigAction,
20+
import Development.IDE.Core.Rules as X (getClientConfigAction,
2221
getParsedModule)
2322
import Development.IDE.Core.Service as X (runAction)
2423
import Development.IDE.Core.Shake as X (FastResult (..),

Diff for: ghcide/src/Development/IDE/Core/Compile.hs

+270-57
Large diffs are not rendered by default.

Diff for: ghcide/src/Development/IDE/Core/RuleTypes.hs

+7-2
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ data TcModuleResult = TcModuleResult
138138
-- ^ Typechecked splice information
139139
, tmrDeferedError :: !Bool
140140
-- ^ Did we defer any type errors for this module?
141+
, tmrRuntimeModules :: !(ModuleEnv UTCTime)
142+
-- ^ Which modules did we need at runtime while compiling this file?
143+
-- Used for recompilation checking in the presence of TH
141144
}
142145
instance Show TcModuleResult where
143146
show = show . pm_mod_summary . tmrParsed
@@ -158,13 +161,15 @@ data HiFileResult = HiFileResult
158161
-- ^ Fingerprint for the ModIface
159162
, hirLinkableFp :: ByteString
160163
-- ^ Fingerprint for the Linkable
164+
, hirRuntimeModules :: !(ModuleEnv UTCTime)
165+
-- ^ same as tmrRuntimeModules
161166
}
162167

163168
hiFileFingerPrint :: HiFileResult -> ByteString
164169
hiFileFingerPrint HiFileResult{..} = hirIfaceFp <> hirLinkableFp
165170

166-
mkHiFileResult :: ModSummary -> HomeModInfo -> HiFileResult
167-
mkHiFileResult hirModSummary hirHomeMod = HiFileResult{..}
171+
mkHiFileResult :: ModSummary -> HomeModInfo -> ModuleEnv UTCTime -> HiFileResult
172+
mkHiFileResult hirModSummary hirHomeMod hirRuntimeModules = HiFileResult{..}
168173
where
169174
hirIfaceFp = fingerprintToBS . getModuleHash . hm_iface $ hirHomeMod -- will always be two bytes
170175
hirLinkableFp = case hm_linkable hirHomeMod of

Diff for: ghcide/src/Development/IDE/Core/Rules.hs

+15-45
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ module Development.IDE.Core.Rules(
3030
usePropertyAction,
3131
-- * Rules
3232
CompiledLinkables(..),
33-
IsHiFileStable(..),
3433
getParsedModuleRule,
3534
getParsedModuleWithCommentsRule,
3635
getLocatedImportsRule,
@@ -42,7 +41,6 @@ module Development.IDE.Core.Rules(
4241
getModIfaceFromDiskRule,
4342
getModIfaceRule,
4443
getModSummaryRule,
45-
isHiFileStableRule,
4644
getModuleGraphRule,
4745
knownFilesRule,
4846
getClientSettingsRule,
@@ -659,13 +657,11 @@ typeCheckRuleDefinition hsc pm = do
659657

660658
-- | Get all the linkables stored in the graph, i.e. the ones we *do not* need to unload.
661659
-- Doesn't actually contain the code, since we don't need it to unload
662-
currentLinkables :: Action [Linkable]
660+
currentLinkables :: Action (ModuleEnv UTCTime)
663661
currentLinkables = do
664662
compiledLinkables <- getCompiledLinkables <$> getIdeGlobalAction
665663
hm <- liftIO $ readVar compiledLinkables
666-
pure $ map go $ moduleEnvToList hm
667-
where
668-
go (mod, time) = LM time mod []
664+
pure hm
669665

670666
loadGhcSession :: GhcSessionDepsConfig -> Rules ()
671667
loadGhcSession ghcSessionDepsConfig = do
@@ -743,15 +739,25 @@ ghcSessionDepsDefinition fullModSummary GhcSessionDepsConfig{..} env file = do
743739
-- | Load a iface from disk, or generate it if there isn't one or it is out of date
744740
-- This rule also ensures that the `.hie` and `.o` (if needed) files are written out.
745741
getModIfaceFromDiskRule :: Rules ()
746-
getModIfaceFromDiskRule = defineEarlyCutoff $ Rule $ \GetModIfaceFromDisk f -> do
742+
getModIfaceFromDiskRule = defineEarlyCutoff $ RuleWithOldValue $ \GetModIfaceFromDisk f old -> do
747743
ms <- msrModSummary <$> use_ GetModSummary f
748744
mb_session <- use GhcSessionDeps f
749745
case mb_session of
750746
Nothing -> return (Nothing, ([], Nothing))
751747
Just session -> do
752-
sourceModified <- use_ IsHiFileStable f
753748
linkableType <- getLinkableType f
754-
r <- loadInterface (hscEnv session) ms sourceModified linkableType (regenerateHiFile session f ms)
749+
ver <- use_ GetModificationTime f
750+
let m_old = case old of
751+
Shake.Succeeded (Just old_version) v -> Just (v, old_version)
752+
Shake.Stale _ (Just old_version) v -> Just (v, old_version)
753+
_ -> Nothing
754+
recompInfo = RecompilationInfo
755+
{ source_version = ver
756+
, old_value = m_old
757+
, get_file_version = use GetModificationTime_{missingFileDiagnostics = False}
758+
, regenerate = regenerateHiFile session f ms
759+
}
760+
r <- loadInterface (hscEnv session) ms linkableType recompInfo
755761
case r of
756762
(diags, Nothing) -> return (Nothing, (diags, Nothing))
757763
(diags, Just x) -> do
@@ -802,31 +808,6 @@ getModIfaceFromDiskAndIndexRule =
802808

803809
return (Just x)
804810

805-
isHiFileStableRule :: Rules ()
806-
isHiFileStableRule = defineEarlyCutoff $ RuleNoDiagnostics $ \IsHiFileStable f -> do
807-
ms <- msrModSummary <$> use_ GetModSummaryWithoutTimestamps f
808-
let hiFile = toNormalizedFilePath'
809-
$ Compat.ml_hi_file $ ms_location ms
810-
mbHiVersion <- use GetModificationTime_{missingFileDiagnostics=False} hiFile
811-
modVersion <- use_ GetModificationTime f
812-
sourceModified <- case mbHiVersion of
813-
Nothing -> pure SourceModified
814-
Just x ->
815-
if modificationTime x < modificationTime modVersion
816-
then pure SourceModified
817-
else do
818-
fileImports <- use_ GetLocatedImports f
819-
let imports = fmap artifactFilePath . snd <$> fileImports
820-
deps <- uses_ IsHiFileStable (catMaybes imports)
821-
pure $ if all (== SourceUnmodifiedAndStable) deps
822-
then SourceUnmodifiedAndStable
823-
else SourceUnmodified
824-
return (Just (summarize sourceModified), Just sourceModified)
825-
where
826-
summarize SourceModified = BS.singleton 1
827-
summarize SourceUnmodified = BS.singleton 2
828-
summarize SourceUnmodifiedAndStable = BS.singleton 3
829-
830811
displayTHWarning :: LspT c IO ()
831812
displayTHWarning
832813
| not isWindows && not hostIsDynamic = do
@@ -1122,7 +1103,6 @@ mainRule RulesConfig{..} = do
11221103
getModIfaceFromDiskAndIndexRule
11231104
getModIfaceRule
11241105
getModSummaryRule
1125-
isHiFileStableRule
11261106
getModuleGraphRule
11271107
knownFilesRule
11281108
getClientSettingsRule
@@ -1144,13 +1124,3 @@ mainRule RulesConfig{..} = do
11441124
persistentHieFileRule
11451125
persistentDocMapRule
11461126
persistentImportMapRule
1147-
1148-
-- | Given the path to a module src file, this rule returns True if the
1149-
-- corresponding `.hi` file is stable, that is, if it is newer
1150-
-- than the src file, and all its dependencies are stable too.
1151-
data IsHiFileStable = IsHiFileStable
1152-
deriving (Eq, Show, Typeable, Generic)
1153-
instance Hashable IsHiFileStable
1154-
instance NFData IsHiFileStable
1155-
1156-
type instance RuleResult IsHiFileStable = SourceModified

Diff for: ghcide/src/Development/IDE/Core/Shake.hs

+12-5
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,7 @@ data RuleBody k v
984984
{ newnessCheck :: BS.ByteString -> BS.ByteString -> Bool
985985
, build :: k -> NormalizedFilePath -> Action (Maybe BS.ByteString, Maybe v)
986986
}
987+
| RuleWithOldValue (k -> NormalizedFilePath -> Value v -> Action (Maybe BS.ByteString, IdeResult v))
987988

988989
-- | Define a new Rule with early cutoff
989990
defineEarlyCutoff
@@ -995,13 +996,13 @@ defineEarlyCutoff (Rule op) = addRule $ \(Q (key, file)) (old :: Maybe BS.ByteSt
995996
let diagnostics diags = do
996997
traceDiagnostics diags
997998
updateFileDiagnostics file (Key key) extras . map (\(_,y,z) -> (y,z)) $ diags
998-
defineEarlyCutoff' diagnostics (==) key file old mode $ op key file
999+
defineEarlyCutoff' diagnostics (==) key file old mode $ const $ op key file
9991000
defineEarlyCutoff (RuleNoDiagnostics op) = addRule $ \(Q (key, file)) (old :: Maybe BS.ByteString) mode -> otTracedAction key file mode traceA $ \traceDiagnostics -> do
10001001
ShakeExtras{logger} <- getShakeExtras
10011002
let diagnostics diags = do
10021003
traceDiagnostics diags
10031004
mapM_ (\d -> liftIO $ logWarning logger $ showDiagnosticsColored [d]) diags
1004-
defineEarlyCutoff' diagnostics (==) key file old mode $ second (mempty,) <$> op key file
1005+
defineEarlyCutoff' diagnostics (==) key file old mode $ const $ second (mempty,) <$> op key file
10051006
defineEarlyCutoff RuleWithCustomNewnessCheck{..} =
10061007
addRule $ \(Q (key, file)) (old :: Maybe BS.ByteString) mode ->
10071008
otTracedAction key file mode traceA $ \ traceDiagnostics -> do
@@ -1010,7 +1011,13 @@ defineEarlyCutoff RuleWithCustomNewnessCheck{..} =
10101011
mapM_ (\d -> liftIO $ logWarning logger $ showDiagnosticsColored [d]) diags
10111012
traceDiagnostics diags
10121013
defineEarlyCutoff' diagnostics newnessCheck key file old mode $
1013-
second (mempty,) <$> build key file
1014+
const $ second (mempty,) <$> build key file
1015+
defineEarlyCutoff (RuleWithOldValue op) = addRule $ \(Q (key, file)) (old :: Maybe BS.ByteString) mode -> otTracedAction key file mode traceA $ \traceDiagnostics -> do
1016+
extras <- getShakeExtras
1017+
let diagnostics diags = do
1018+
traceDiagnostics diags
1019+
updateFileDiagnostics file (Key key) extras . map (\(_,y,z) -> (y,z)) $ diags
1020+
defineEarlyCutoff' diagnostics (==) key file old mode $ op key file
10141021

10151022
defineNoFile :: IdeRule k v => (k -> Action v) -> Rules ()
10161023
defineNoFile f = defineNoDiagnostics $ \k file -> do
@@ -1031,7 +1038,7 @@ defineEarlyCutoff'
10311038
-> NormalizedFilePath
10321039
-> Maybe BS.ByteString
10331040
-> RunMode
1034-
-> Action (Maybe BS.ByteString, IdeResult v)
1041+
-> (Value v -> Action (Maybe BS.ByteString, IdeResult v))
10351042
-> Action (RunResult (A (RuleResult k)))
10361043
defineEarlyCutoff' doDiagnostics cmp key file old mode action = do
10371044
ShakeExtras{state, progress, dirtyKeys} <- getShakeExtras
@@ -1061,7 +1068,7 @@ defineEarlyCutoff' doDiagnostics cmp key file old mode action = do
10611068
Just (Failed b, _) -> Failed b
10621069

10631070
(bs, (diags, res)) <- actionCatch
1064-
(do v <- action; liftIO $ evaluate $ force v) $
1071+
(do v <- action staleV; liftIO $ evaluate $ force v) $
10651072
\(e :: SomeException) -> do
10661073
pure (Nothing, ([ideErrorText file $ T.pack $ show e | not $ isBadDependency e],Nothing))
10671074

0 commit comments

Comments
 (0)