Skip to content

Commit 682386d

Browse files
pepeiborrandmitchell
pepeiborra
and
ndmitchell
authored
Reimplement shake (continued) (#2060)
* Rewrite hls-graph to not use the Shake code * redundant import * Fix the bug * add comments and format imports * Fix another bug * fix locking in incDatabase * avoid calling withNumCapabilities on every build * faster cleanup * implement reverse deps * --conservative-change-tracking * Avoid grabbing the db lock when updating reverse deps * update reverse deps asynchronously * Profiling graph builds * extend up profiling to record visiting steps The main benefit of reverse dependency tracking is that we avoid unnecessary node lookups. However, these lookups were not shown in hls-graph profiles, hiding their real cost. Concretely, if the number of lookups per build is proportional to the number of transitive dependencies of a node, which in turn is proportional to the number of transitive imports of a module, then we have an O(edges) complexity instead of an O(nodes) complexity, which is really bad for large projects. This Diff extends the recorded data and the profiling UI to keep track of visited nodes and to show them in a new column "Visited" of the "Rules" tab. The cost of doing this is storing an additional Int per node at runtime. While I was editing the profiling UI, I took the chance to remove the command tabs, update the README and add some missing files * include dirty set in profiles * actionFork * avoid spawning threads for simple lookups * Fix a flaky test * record changes to GetKnownTargets * Readme for hls-graph * Bump version numbers * Drop dependency on Shake * explain why we restart a Shake session * clean up Internal.Database * add a new benchmark example cabal-1module * Fix masking and further reduce threading * Trace aborted rule evaluations * Fix code actions after cradle edit experiment * Avoid spawning threads for build rules with 1 or fewer deps * simplify a test * hlint * Add a test for off-editor changes * Fix flaky tests * fix incomplete pattern match in Tactics test suite * Fix flaky tests * attempt to fix tactics test suite in Windows Co-authored-by: Neil Mitchell <ndmitchell@gmail.com>
1 parent c419b37 commit 682386d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+8219
-301
lines changed

‎ghcide/bench/config.yaml

+22-1
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,35 @@ examples:
1919
- Distribution/Simple.hs
2020
- Distribution/Types/Module.hs
2121
extra-args: [] # extra ghcide command line args
22+
- name: cabal-1module
23+
package: Cabal
24+
version: 3.0.0.0
25+
modules:
26+
- Distribution/Simple.hs
27+
- name: cabal-conservative
28+
package: Cabal
29+
version: 3.0.0.0
30+
modules:
31+
- Distribution/Simple.hs
32+
- Distribution/Types/Module.hs
33+
extra-args: # extra ghcide command line args
34+
- --conservative-change-tracking
2235
# Small-sized project with TH
2336
- name: lsp-types
2437
package: lsp-types
2538
version: 1.0.0.1
2639
modules:
2740
- src/Language/LSP/VFS.hs
2841
- src/Language/LSP/Types/Lens.hs
29-
extra-args: [] # extra ghcide command line args
42+
- name: lsp-types-conservative
43+
package: lsp-types
44+
version: 1.0.0.1
45+
modules:
46+
- src/Language/LSP/VFS.hs
47+
- src/Language/LSP/Types/Lens.hs
48+
extra-args:
49+
- --conservative-change-tracking
50+
# Small-sized project with TH
3051
# Small but heavily multi-component example
3152
# Disabled as it is far to slow. hie-bios >0.7.2 should help
3253
# - name: HLS

‎ghcide/bench/lib/Experiments.hs

+22-17
Original file line numberDiff line numberDiff line change
@@ -152,21 +152,22 @@ experiments =
152152
benchWithSetup
153153
"code actions after cradle edit"
154154
( \docs -> do
155-
unless (any (isJust . identifierP) docs) $
156-
error "None of the example modules is suitable for this experiment"
157-
forM_ docs $ \DocumentPositions{..} ->
158-
forM_ identifierP $ \p -> changeDoc doc [charEdit p]
155+
forM_ docs $ \DocumentPositions{..} -> do
156+
forM identifierP $ \p -> do
157+
changeDoc doc [charEdit p]
158+
waitForProgressStart
159+
void waitForBuildQueue
159160
)
160161
( \docs -> do
161162
hieYamlUri <- getDocUri "hie.yaml"
162163
liftIO $ appendFile (fromJust $ uriToFilePath hieYamlUri) "##\n"
163164
sendNotification SWorkspaceDidChangeWatchedFiles $ DidChangeWatchedFilesParams $
164165
List [ FileEvent hieYamlUri FcChanged ]
165-
forM_ docs $ \DocumentPositions{..} -> do
166-
changeDoc doc [charEdit stringLiteralP]
167-
waitForProgressStart
166+
waitForProgressStart
167+
waitForProgressStart
168+
waitForProgressStart -- the Session logic restarts a second time
168169
waitForProgressDone
169-
not . null . catMaybes <$> forM docs (\DocumentPositions{..} -> do
170+
not . all null . catMaybes <$> forM docs (\DocumentPositions{..} -> do
170171
forM identifierP $ \p ->
171172
getCodeActions doc (Range p p))
172173
),
@@ -421,6 +422,17 @@ waitForProgressDone = loop
421422
done <- null <$> getIncompleteProgressSessions
422423
unless done loop
423424

425+
-- | Wait for the build queue to be empty
426+
waitForBuildQueue :: Session Seconds
427+
waitForBuildQueue = do
428+
let m = SCustomMethod "test"
429+
waitId <- sendRequest m (toJSON WaitForShakeQueue)
430+
(td, resp) <- duration $ skipManyTill anyMessage $ responseForId m waitId
431+
case resp of
432+
ResponseMessage{_result=Right Null} -> return td
433+
-- assume a ghcide binary lacking the WaitForShakeQueue method
434+
_ -> return 0
435+
424436
runBench ::
425437
(?config :: Config) =>
426438
(Session BenchRun -> IO BenchRun) ->
@@ -451,15 +463,8 @@ runBench runSess b = handleAny (\e -> print e >> return badRun)
451463
else do
452464
output (showDuration t)
453465
-- Wait for the delayed actions to finish
454-
let m = SCustomMethod "test"
455-
waitId <- sendRequest m (toJSON WaitForShakeQueue)
456-
(td, resp) <- duration $ skipManyTill anyMessage $ responseForId m waitId
457-
case resp of
458-
ResponseMessage{_result=Right Null} -> do
459-
loop (userWaits+t) (delayedWork+td) (n -1)
460-
_ ->
461-
-- Assume a ghcide build lacking the WaitForShakeQueue command
462-
loop (userWaits+t) delayedWork (n -1)
466+
td <- waitForBuildQueue
467+
loop (userWaits+t) (delayedWork+td) (n -1)
463468

464469
(runExperiment, result) <- duration $ loop 0 0 samples
465470
let success = isJust result

‎ghcide/exe/Arguments.hs

+11-9
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,16 @@ import Ide.Types (IdePlugins)
99
import Options.Applicative
1010

1111
data Arguments = Arguments
12-
{argsCwd :: Maybe FilePath
13-
,argsVersion :: Bool
14-
,argsShakeProfiling :: Maybe FilePath
15-
,argsOTMemoryProfiling :: Bool
16-
,argsTesting :: Bool
17-
,argsDisableKick :: Bool
18-
,argsThreads :: Int
19-
,argsVerbose :: Bool
20-
,argsCommand :: Command
12+
{argsCwd :: Maybe FilePath
13+
,argsVersion :: Bool
14+
,argsShakeProfiling :: Maybe FilePath
15+
,argsOTMemoryProfiling :: Bool
16+
,argsTesting :: Bool
17+
,argsDisableKick :: Bool
18+
,argsThreads :: Int
19+
,argsVerbose :: Bool
20+
,argsCommand :: Command
21+
,argsConservativeChangeTracking :: Bool
2122
}
2223

2324
getArguments :: IdePlugins IdeState -> IO Arguments
@@ -38,6 +39,7 @@ arguments plugins = Arguments
3839
<*> option auto (short 'j' <> help "Number of threads (0: automatic)" <> metavar "NUM" <> value 0 <> showDefault)
3940
<*> switch (short 'd' <> long "verbose" <> help "Include internal events in logging output")
4041
<*> (commandP plugins <|> lspCommand <|> checkCommand)
42+
<*> switch (long "conservative-change-tracking" <> help "disable reactive change tracking (for testing/debugging)")
4143
where
4244
checkCommand = Check <$> many (argument str (metavar "FILES/DIRS..."))
4345
lspCommand = LSP <$ flag' True (long "lsp" <> help "Start talking to an LSP client")

‎ghcide/exe/Main.hs

+3
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ main = do
7171
then Test.plugin
7272
else mempty
7373

74+
,Main.argsThreads = case argsThreads of 0 -> Nothing ; i -> Just (fromIntegral i)
75+
7476
,Main.argsIdeOptions = \config sessionLoader ->
7577
let defOptions = defaultIdeOptions sessionLoader
7678
in defOptions
@@ -80,5 +82,6 @@ main = do
8082
, optShakeOptions = (optShakeOptions defOptions){shakeThreads = argsThreads}
8183
, optCheckParents = pure $ checkParents config
8284
, optCheckProject = pure $ checkProject config
85+
, optRunSubset = not argsConservativeChangeTracking
8386
}
8487
}

‎ghcide/ghcide.cabal

+3-1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ library
4848
dependent-map,
4949
dependent-sum,
5050
dlist,
51+
exceptions,
5152
-- we can't use >= 1.7.10 while we have to use hlint == 3.2.*
5253
extra >= 1.7.4 && < 1.7.10,
5354
fuzzy,
@@ -76,7 +77,7 @@ library
7677
rope-utf16-splay,
7778
safe,
7879
safe-exceptions,
79-
hls-graph ^>= 1.4,
80+
hls-graph ^>= 1.5,
8081
sorted-list,
8182
sqlite-simple,
8283
stm,
@@ -269,6 +270,7 @@ benchmark benchHist
269270
directory,
270271
extra,
271272
filepath,
273+
lens,
272274
optparse-applicative,
273275
shake,
274276
text,

‎ghcide/session-loader/Development/IDE/Session.hs

+2-2
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,6 @@ loadSessionWithOptions SessionLoadingOptions{..} dir = do
250250

251251
IdeOptions{ optTesting = IdeTesting optTesting
252252
, optCheckProject = getCheckProject
253-
, optModifyDynFlags
254253
, optExtensions
255254
} <- getIdeOptions
256255

@@ -264,6 +263,7 @@ loadSessionWithOptions SessionLoadingOptions{..} dir = do
264263
TargetModule _ -> do
265264
found <- filterM (IO.doesFileExist . fromNormalizedFilePath) targetLocations
266265
return (targetTarget, found)
266+
recordDirtyKeys extras GetKnownTargets [emptyFilePath]
267267
modifyVarIO' knownTargetsVar $ traverseHashed $ \known -> do
268268
let known' = HM.unionWith (<>) known $ HM.fromList $ map (second Set.fromList) knownTargets
269269
when (known /= known') $
@@ -390,7 +390,7 @@ loadSessionWithOptions SessionLoadingOptions{..} dir = do
390390

391391
-- Invalidate all the existing GhcSession build nodes by restarting the Shake session
392392
invalidateShakeCache
393-
restartShakeSession []
393+
restartShakeSession "new component" []
394394

395395
-- Typecheck all files in the project on startup
396396
checkProject <- getCheckProject

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ import Development.IDE.Import.DependencyInformation
4848
import Development.IDE.Types.Diagnostics
4949
import Development.IDE.Types.Location
5050
import Development.IDE.Types.Options
51-
import Development.IDE.Types.Shake (SomeShakeValue)
5251
import HieDb.Create (deleteMissingRealFiles)
5352
import Ide.Plugin.Config (CheckParents (..),
5453
Config)
@@ -271,7 +270,7 @@ setFileModified state saved nfp = do
271270
when (isJust setVirtualFileContents) $
272271
fail "setFileModified can't be called on this type of VFSHandle"
273272
recordDirtyKeys (shakeExtras state) GetModificationTime [nfp]
274-
restartShakeSession (shakeExtras state) []
273+
restartShakeSession (shakeExtras state) (fromNormalizedFilePath nfp ++ " (modified)") []
275274
when checkParents $
276275
typecheckParents state nfp
277276

@@ -294,16 +293,16 @@ typecheckParentsAction nfp = do
294293
-- | Note that some keys have been modified and restart the session
295294
-- Only valid if the virtual file system was initialised by LSP, as that
296295
-- independently tracks which files are modified.
297-
setSomethingModified :: IdeState -> [SomeShakeValue] -> IO ()
298-
setSomethingModified state keys = do
296+
setSomethingModified :: IdeState -> [Key] -> String -> IO ()
297+
setSomethingModified state keys reason = do
299298
VFSHandle{..} <- getIdeGlobalState state
300299
when (isJust setVirtualFileContents) $
301300
fail "setSomethingModified can't be called on this type of VFSHandle"
302301
-- Update database to remove any files that might have been renamed/deleted
303302
atomically $ writeTQueue (indexQueue $ hiedbWriter $ shakeExtras state) deleteMissingRealFiles
304303
atomicModifyIORef_ (dirtyKeys $ shakeExtras state) $ \x ->
305304
foldl' (flip HSet.insert) x keys
306-
void $ restartShakeSession (shakeExtras state) []
305+
void $ restartShakeSession (shakeExtras state) reason []
307306

308307
registerFileWatches :: [String] -> LSP.LspT Config IO Bool
309308
registerFileWatches globs = do

0 commit comments

Comments
 (0)