Skip to content

Commit 9f3426d

Browse files
committed
Review early cutoff fingerprints
Some of these were unnecessary, while others were very inefficient
1 parent 0f3eeac commit 9f3426d

File tree

6 files changed

+82
-54
lines changed

6 files changed

+82
-54
lines changed

ghcide/src/Development/IDE/Core/Compile.hs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@ import Control.Concurrent.Extra
114114
import Control.Concurrent.STM hiding (orElse)
115115
import Data.Aeson (toJSON)
116116
import Data.Binary
117-
import Data.Binary.Put
118-
import qualified Data.ByteString.Lazy as LBS
119117
import Data.Coerce
120118
import Data.Functor
121119
import qualified Data.HashMap.Strict as HashMap
@@ -242,7 +240,7 @@ mkHiFileResultNoCompile session tcm = do
242240
(iface, _) <- mkIfaceTc hsc_env_tmp Nothing sf details tcGblEnv
243241
#endif
244242
let mod_info = HomeModInfo iface details Nothing
245-
pure $! HiFileResult ms mod_info
243+
pure $! mkHiFileResult ms mod_info
246244

247245
mkHiFileResultCompile
248246
:: HscEnv
@@ -277,7 +275,7 @@ mkHiFileResultCompile session' tcm simplified_guts ltype = catchErrs $ do
277275
(final_iface,_) <- mkIface session Nothing details simplified_guts
278276
#endif
279277
let mod_info = HomeModInfo final_iface details linkable
280-
pure (diags, Just $! HiFileResult ms mod_info)
278+
pure (diags, Just $! mkHiFileResult ms mod_info)
281279

282280
where
283281
dflags = hsc_dflags session'
@@ -750,13 +748,12 @@ getModSummaryFromImports env fp modTime contents = do
750748
-- Compute a fingerprint from the contents of `ModSummary`,
751749
-- eliding the timestamps, the preprocessed source and other non relevant fields
752750
computeFingerprint opts ModSummary{..} = do
753-
let moduleUniques = runPut $ do
751+
fingerPrintImports <- fingerprintFromPut $ do
754752
put $ uniq $ moduleNameFS $ moduleName ms_mod
755753
forM_ (ms_srcimps ++ ms_textual_imps) $ \(mb_p, m) -> do
756754
put $ uniq $ moduleNameFS $ unLoc m
757755
whenJust mb_p $ put . uniq
758-
fingerPrintImports <- fingerprintFromByteString $ LBS.toStrict moduleUniques
759-
return $ fingerprintFingerprints $
756+
return $! fingerprintFingerprints $
760757
[ fingerprintString fp
761758
, fingerPrintImports
762759
] ++ map fingerprintString opts
@@ -927,7 +924,7 @@ loadInterface session ms sourceMod linkableNeeded regen = do
927924
if objUpToDate
928925
then do
929926
hmi <- liftIO $ mkDetailsFromIface sessionWithMsDynFlags iface linkable
930-
return ([], Just $ HiFileResult ms hmi)
927+
return ([], Just $ mkHiFileResult ms hmi)
931928
else regen linkableNeeded
932929
(_reason, _) -> regen linkableNeeded
933930

ghcide/src/Development/IDE/Core/FileStore.hs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import Control.Concurrent.STM (atomically)
2727
import Control.Concurrent.STM.TQueue (writeTQueue)
2828
import Control.Exception
2929
import Control.Monad.Extra
30-
import qualified Data.ByteString.Char8 as BS
30+
import qualified Data.ByteString as BS
3131
import Data.Either.Extra
3232
import qualified Data.HashMap.Strict as HM
3333
import Data.Int (Int64)
@@ -46,7 +46,6 @@ import Development.IDE.Types.Diagnostics
4646
import Development.IDE.Types.Location
4747
import Development.IDE.Types.Options
4848
import Development.Shake
49-
import Development.Shake.Classes
5049
import HieDb.Create (deleteMissingRealFiles)
5150
import Ide.Plugin.Config (CheckParents (..))
5251
import System.IO.Error
@@ -66,12 +65,13 @@ import qualified System.Posix.Error as Posix
6665

6766
import qualified Development.IDE.Types.Logger as L
6867

68+
import qualified Data.Binary as B
69+
import qualified Data.ByteString.Lazy as LBS
6970
import Language.LSP.Server hiding
7071
(getVirtualFile)
7172
import qualified Language.LSP.Server as LSP
7273
import Language.LSP.Types (FileChangeType (FcChanged),
7374
FileEvent (FileEvent),
74-
NormalizedFilePath (NormalizedFilePath),
7575
toNormalizedFilePath,
7676
uriToFilePath)
7777
import Language.LSP.VFS
@@ -102,8 +102,16 @@ makeLSPVFSHandle lspEnv = VFSHandle
102102
isFileOfInterestRule :: Rules ()
103103
isFileOfInterestRule = defineEarlyCutoff $ RuleNoDiagnostics $ \IsFileOfInterest f -> do
104104
filesOfInterest <- getFilesOfInterest
105-
let res = maybe NotFOI IsFOI $ f `HM.lookup` filesOfInterest
106-
return (Just $ BS.pack $ show $ hash res, Just res)
105+
let foi = maybe NotFOI IsFOI $ f `HM.lookup` filesOfInterest
106+
fp = summarize foi
107+
res = (Just fp, Just foi)
108+
return res
109+
where
110+
summarize NotFOI = BS.singleton 0
111+
summarize (IsFOI OnDisk) = BS.singleton 1
112+
summarize (IsFOI (Modified False)) = BS.singleton 2
113+
summarize (IsFOI (Modified True)) = BS.singleton 3
114+
107115

108116
getModificationTimeRule :: VFSHandle -> (NormalizedFilePath -> Action Bool) -> Rules ()
109117
getModificationTimeRule vfs isWatched = defineEarlyCutoff $ Rule $ \(GetModificationTime_ missingFileDiags) file ->
@@ -117,15 +125,15 @@ getModificationTimeImpl :: VFSHandle
117125
(Maybe BS.ByteString, ([FileDiagnostic], Maybe FileVersion))
118126
getModificationTimeImpl vfs isWatched missingFileDiags file = do
119127
let file' = fromNormalizedFilePath file
120-
let wrap time@(l,s) = (Just $ BS.pack $ show time, ([], Just $ ModificationTime l s))
128+
let wrap time@(l,s) = (Just $ LBS.toStrict $ B.encode time, ([], Just $ ModificationTime l s))
121129
mbVirtual <- liftIO $ getVirtualFile vfs $ filePathToUri' file
122130
-- we use 'getVirtualFile' to discriminate FOIs so make that
123131
-- dependency explicit by using the IsFileOfInterest rule
124132
_ <- use_ IsFileOfInterest file
125133
case mbVirtual of
126134
Just (virtualFileVersion -> ver) -> do
127135
alwaysRerun
128-
pure (Just $ BS.pack $ show ver, ([], Just $ VFSVersion ver))
136+
pure (Just $ LBS.toStrict $ B.encode ver, ([], Just $ VFSVersion ver))
129137
Nothing -> do
130138
isWF <- isWatched file
131139
unless (isWF || isInterface file) alwaysRerun

ghcide/src/Development/IDE/Core/OfInterest.hs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import Control.DeepSeq
1818
import Control.Exception
1919
import Control.Monad
2020
import Data.Binary
21-
import qualified Data.ByteString.UTF8 as BS
2221
import Data.HashMap.Strict (HashMap)
2322
import qualified Data.HashMap.Strict as HashMap
2423
import Data.Hashable
@@ -30,6 +29,7 @@ import GHC.Generics
3029

3130
import Control.Monad.Trans.Class
3231
import Control.Monad.Trans.Maybe
32+
import qualified Data.ByteString.Lazy as LBS
3333
import Data.List.Extra (nubOrd)
3434
import Data.Maybe (catMaybes)
3535
import Development.IDE.Core.RuleTypes
@@ -59,15 +59,13 @@ ofInterestRules = do
5959
defineEarlyCutoff $ RuleNoDiagnostics $ \GetFilesOfInterest _file -> assert (null $ fromNormalizedFilePath _file) $ do
6060
alwaysRerun
6161
filesOfInterest <- getFilesOfInterestUntracked
62-
pure (Just $ BS.fromString $ show filesOfInterest, Just filesOfInterest)
63-
62+
let !cutoff = LBS.toStrict $ encode $ HashMap.toList filesOfInterest
63+
pure (Just cutoff, Just filesOfInterest)
6464

6565
-- | Get the files that are open in the IDE.
6666
getFilesOfInterest :: Action (HashMap NormalizedFilePath FileOfInterestStatus)
6767
getFilesOfInterest = useNoFile_ GetFilesOfInterest
6868

69-
70-
7169
------------------------------------------------------------
7270
-- Exposed API
7371

ghcide/src/Development/IDE/Core/RuleTypes.hs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,21 @@ data HiFileResult = HiFileResult
156156
-- a reference to a typechecked module
157157
, hirHomeMod :: !HomeModInfo
158158
-- ^ Includes the Linkable iff we need object files
159+
, hirIfaceFp :: ByteString
160+
-- ^ Fingerprint for the ModIface
161+
, hirLinkableFp :: ByteString
162+
-- ^ Fingerprint for the Linkable
159163
}
160164

161165
hiFileFingerPrint :: HiFileResult -> ByteString
162-
hiFileFingerPrint hfr = ifaceBS <> linkableBS
166+
hiFileFingerPrint HiFileResult{..} = hirIfaceFp <> hirLinkableFp
167+
168+
mkHiFileResult :: ModSummary -> HomeModInfo -> HiFileResult
169+
mkHiFileResult hirModSummary hirHomeMod = HiFileResult{..}
163170
where
164-
ifaceBS = fingerprintToBS . getModuleHash . hirModIface $ hfr -- will always be two bytes
165-
linkableBS = case hm_linkable $ hirHomeMod hfr of
171+
hirIfaceFp =
172+
fingerprintToBS . getModuleHash . hm_iface $ hirHomeMod -- will always be two bytes
173+
hirLinkableFp = case hm_linkable hirHomeMod of
166174
Nothing -> ""
167175
Just l -> BS.pack $ show $ linkableTime l
168176

ghcide/src/Development/IDE/Core/Rules.hs

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ import Control.Monad.Trans.Maybe
7272
import Data.Aeson (Result (Success),
7373
toJSON)
7474
import Data.Binary hiding (get, put)
75-
import qualified Data.ByteString.Char8 as BS
75+
import qualified Data.ByteString as BS
7676
import Data.Foldable
7777
import Data.IntMap.Strict (IntMap)
7878
import qualified Data.IntMap.Strict as IntMap
@@ -150,6 +150,8 @@ import TcRnMonad (tcg_dependent_fil
150150
import qualified Data.Aeson.Types as A
151151
import qualified HieDb
152152
import Ide.Plugin.Config
153+
import qualified Data.ByteString.Lazy as LBS
154+
import qualified Data.Binary as B
153155

154156
-- | This is useful for rules to convert rules that can only produce errors or
155157
-- a result into the more general IdeResult type that supports producing
@@ -308,7 +310,9 @@ priorityFilesOfInterest = Priority (-2)
308310
-- and https://github.com/mpickering/ghcide/pull/22#issuecomment-625070490
309311
-- GHC wiki about: https://gitlab.haskell.org/ghc/ghc/-/wikis/api-annotations
310312
getParsedModuleRule :: Rules ()
311-
getParsedModuleRule = defineEarlyCutoff $ Rule $ \GetParsedModule file -> do
313+
getParsedModuleRule =
314+
-- this rule does not have early cutoff since all its dependencies already have it
315+
define $ \GetParsedModule file -> do
312316
ModSummaryResult{msrModSummary = ms} <- use_ GetModSummary file
313317
sess <- use_ GhcSession file
314318
let hsc = hscEnv sess
@@ -318,7 +322,7 @@ getParsedModuleRule = defineEarlyCutoff $ Rule $ \GetParsedModule file -> do
318322
mainParse = getParsedModuleDefinition hsc opt file ms
319323

320324
-- Parse again (if necessary) to capture Haddock parse errors
321-
res@(_, (_,pmod)) <- if gopt Opt_Haddock dflags
325+
res@(_,pmod) <- if gopt Opt_Haddock dflags
322326
then
323327
liftIO mainParse
324328
else do
@@ -330,20 +334,20 @@ getParsedModuleRule = defineEarlyCutoff $ Rule $ \GetParsedModule file -> do
330334
-- If we can parse Haddocks, might as well use them
331335
--
332336
-- HLINT INTEGRATION: might need to save the other parsed module too
333-
((fp,(diags,res)),(fph,(diagsh,resh))) <- liftIO $ concurrently mainParse haddockParse
337+
((diags,res),(diagsh,resh)) <- liftIO $ concurrently mainParse haddockParse
334338

335339
-- Merge haddock and regular diagnostics so we can always report haddock
336340
-- parse errors
337341
let diagsM = mergeParseErrorsHaddock diags diagsh
338342
case resh of
339343
Just _
340344
| HaddockParse <- optHaddockParse opt
341-
-> pure (fph, (diagsM, resh))
345+
-> pure (diagsM, resh)
342346
-- If we fail to parse haddocks, report the haddock diagnostics as well and
343347
-- return the non-haddock parse.
344348
-- This seems to be the correct behaviour because the Haddock flag is added
345349
-- by us and not the user, so our IDE shouldn't stop working because of it.
346-
_ -> pure (fp, (diagsM, res))
350+
_ -> pure (diagsM, res)
347351
-- Add dependencies on included files
348352
_ <- uses GetModificationTime $ map toNormalizedFilePath' (maybe [] pm_extra_src_files pmod)
349353
pure res
@@ -372,26 +376,30 @@ mergeParseErrorsHaddock normal haddock = normal ++
372376
-- | This rule provides a ParsedModule preserving all annotations,
373377
-- including keywords, punctuation and comments.
374378
-- So it is suitable for use cases where you need a perfect edit.
375-
-- FIXME this rule should probably not produce diagnostics
376379
getParsedModuleWithCommentsRule :: Rules ()
377-
getParsedModuleWithCommentsRule = defineEarlyCutoff $ Rule $ \GetParsedModuleWithComments file -> do
380+
getParsedModuleWithCommentsRule =
381+
-- The parse diagnostics are owned by the GetParsedModule rule
382+
-- For this reason, this rule does not produce any diagnostics
383+
defineNoDiagnostics $ \GetParsedModuleWithComments file -> do
378384
ModSummaryResult{msrModSummary = ms} <- use_ GetModSummary file
379385
sess <- use_ GhcSession file
380386
opt <- getIdeOptions
381387

382388
let ms' = withoutOption Opt_Haddock $ withOption Opt_KeepRawTokenStream ms
383389

384-
liftIO $ getParsedModuleDefinition (hscEnv sess) opt file ms'
390+
liftIO $ snd <$> getParsedModuleDefinition (hscEnv sess) opt file ms'
385391

386-
getParsedModuleDefinition :: HscEnv -> IdeOptions -> NormalizedFilePath -> ModSummary -> IO (Maybe BS.ByteString, ([FileDiagnostic], Maybe ParsedModule))
392+
getParsedModuleDefinition
393+
:: HscEnv
394+
-> IdeOptions
395+
-> NormalizedFilePath
396+
-> ModSummary -> IO (([FileDiagnostic], Maybe ParsedModule))
387397
getParsedModuleDefinition packageState opt file ms = do
388398
let fp = fromNormalizedFilePath file
389399
(diag, res) <- parseModule opt packageState fp ms
390400
case res of
391-
Nothing -> pure (Nothing, (diag, Nothing))
392-
Just modu -> do
393-
mbFingerprint <- traverse (fmap fingerprintToBS . fingerprintFromStringBuffer) (ms_hspp_buf ms)
394-
pure (mbFingerprint, (diag, Just modu))
401+
Nothing -> pure (diag, Nothing)
402+
Just modu -> pure (diag, Just modu)
395403

396404
getLocatedImportsRule :: Rules ()
397405
getLocatedImportsRule =
@@ -686,7 +694,7 @@ knownFilesRule :: Rules ()
686694
knownFilesRule = defineEarlyCutOffNoFile $ \GetKnownTargets -> do
687695
alwaysRerun
688696
fs <- knownTargets
689-
pure (BS.pack (show $ hash fs), unhashed fs)
697+
pure (LBS.toStrict $ B.encode $ hash fs, unhashed fs)
690698

691699
getModuleGraphRule :: Rules ()
692700
getModuleGraphRule = defineNoFile $ \GetModuleGraph -> do
@@ -737,8 +745,8 @@ loadGhcSession = do
737745
opts <- getIdeOptions
738746
res <- optGhcSession opts
739747

740-
let fingerprint = hash (sessionVersion res)
741-
return (BS.pack (show fingerprint), res)
748+
let fingerprint = LBS.toStrict $ B.encode $ hash (sessionVersion res)
749+
return (fingerprint, res)
742750

743751
defineEarlyCutoff $ Rule $ \GhcSession file -> do
744752
IdeGhcSession{loadSessionFun} <- useNoFile_ GhcSessionIO
@@ -759,7 +767,7 @@ loadGhcSession = do
759767
Just {} -> ""
760768
-- Hash the HscEnvEq returned so cutoff if it didn't change
761769
-- from last time
762-
Nothing -> BS.pack (show (hash (snd val)))
770+
Nothing -> LBS.toStrict $ B.encode (hash (snd val))
763771
return (Just cutoffHash, val)
764772

765773
define $ \GhcSessionDeps file -> ghcSessionDepsDefinition file
@@ -815,7 +823,9 @@ getModIfaceFromDiskRule = defineEarlyCutoff $ Rule $ \GetModIfaceFromDisk f -> d
815823
-- disk since we are careful to write out the `.hie` file before writing the
816824
-- `.hi` file
817825
getModIfaceFromDiskAndIndexRule :: Rules ()
818-
getModIfaceFromDiskAndIndexRule = defineEarlyCutoff $ RuleNoDiagnostics $ \GetModIfaceFromDiskAndIndex f -> do
826+
getModIfaceFromDiskAndIndexRule =
827+
-- doesn't need early cutoff since all its dependencies already have it
828+
defineNoDiagnostics $ \GetModIfaceFromDiskAndIndex f -> do
819829
x <- use_ GetModIfaceFromDisk f
820830
se@ShakeExtras{hiedb} <- getShakeExtras
821831

@@ -844,8 +854,7 @@ getModIfaceFromDiskAndIndexRule = defineEarlyCutoff $ RuleNoDiagnostics $ \GetMo
844854
L.logDebug (logger se) $ "Re-indexing hie file for" <> T.pack (fromNormalizedFilePath f)
845855
indexHieFile se ms f hash hf
846856

847-
let fp = hiFileFingerPrint x
848-
return (Just fp, Just x)
857+
return (Just x)
849858

850859
isHiFileStableRule :: Rules ()
851860
isHiFileStableRule = defineEarlyCutoff $ RuleNoDiagnostics $ \IsHiFileStable f -> do
@@ -866,7 +875,11 @@ isHiFileStableRule = defineEarlyCutoff $ RuleNoDiagnostics $ \IsHiFileStable f -
866875
pure $ if all (== SourceUnmodifiedAndStable) deps
867876
then SourceUnmodifiedAndStable
868877
else SourceUnmodified
869-
return (Just (BS.pack $ show sourceModified), Just sourceModified)
878+
return (Just (summarize sourceModified), Just sourceModified)
879+
where
880+
summarize SourceModified = BS.singleton 1
881+
summarize SourceUnmodified = BS.singleton 2
882+
summarize SourceUnmodifiedAndStable = BS.singleton 3
870883

871884
getModSummaryRule :: Rules ()
872885
getModSummaryRule = do
@@ -942,7 +955,7 @@ getModIfaceWithoutLinkableRule = defineEarlyCutoff $ RuleNoDiagnostics $ \GetMod
942955
mhfr <- use GetModIface f
943956
let mhfr' = fmap (\x -> x{ hirHomeMod = (hirHomeMod x){ hm_linkable = Just (error msg) } }) mhfr
944957
msg = "tried to look at linkable for GetModIfaceWithoutLinkable for " ++ show f
945-
pure (fingerprintToBS . getModuleHash . hirModIface <$> mhfr', mhfr')
958+
pure (hirIfaceFp <$> mhfr', mhfr')
946959

947960
-- | Also generates and indexes the `.hie` file, along with the `.o` file if needed
948961
-- Invariant maintained is that if the `.hi` file was successfully written, then the
@@ -953,12 +966,12 @@ regenerateHiFile sess f ms compNeeded = do
953966
opt <- getIdeOptions
954967

955968
-- Embed haddocks in the interface file
956-
(_, (diags, mb_pm)) <- liftIO $ getParsedModuleDefinition hsc opt f (withOptHaddock ms)
969+
(diags, mb_pm) <- liftIO $ getParsedModuleDefinition hsc opt f (withOptHaddock ms)
957970
(diags, mb_pm) <- case mb_pm of
958971
Just _ -> return (diags, mb_pm)
959972
Nothing -> do
960973
-- if parsing fails, try parsing again with Haddock turned off
961-
(_, (diagsNoHaddock, mb_pm)) <- liftIO $ getParsedModuleDefinition hsc opt f ms
974+
(diagsNoHaddock, mb_pm) <- liftIO $ getParsedModuleDefinition hsc opt f ms
962975
return (mergeParseErrorsHaddock diagsNoHaddock diags, mb_pm)
963976
case mb_pm of
964977
Nothing -> return (diags, Nothing)
@@ -1021,7 +1034,7 @@ getClientSettingsRule :: Rules ()
10211034
getClientSettingsRule = defineEarlyCutOffNoFile $ \GetClientSettings -> do
10221035
alwaysRerun
10231036
settings <- clientSettings <$> getIdeConfiguration
1024-
return (BS.pack . show . hash $ settings, settings)
1037+
return (LBS.toStrict $ B.encode $ hash settings, settings)
10251038

10261039
-- | Returns the client configurarion stored in the IdeState.
10271040
-- You can use this function to access it from shake Rules
@@ -1062,7 +1075,7 @@ needsCompilationRule = defineEarlyCutoff $ RuleNoDiagnostics $ \NeedsCompilation
10621075
(uses NeedsCompilation revdeps)
10631076
pure $ computeLinkableType ms modsums (map join needsComps)
10641077

1065-
pure (Just $ BS.pack $ show $ hash res, Just res)
1078+
pure (Just $ LBS.toStrict $ B.encode $ hash res, Just res)
10661079
where
10671080
uses_th_qq (ms_hspp_opts -> dflags) =
10681081
xopt LangExt.TemplateHaskell dflags || xopt LangExt.QuasiQuotes dflags

0 commit comments

Comments
 (0)