on master help sign in
Make runActions return Return as soon as results are available (#1736)

* Make runActions return Return as soon as results are available Previously, we were waiting for all rules to finish, in particular the ofInterestRule. That doesn’t really make any sense, e.g., a goto definition request should not be waiting for all scenarios to run. The next step will be to change the LSP side such that requests and notifications are processed in parallel where possible.

commitebbe9800b5834c0520e96684d1e6c0ba1f80351d authorMoritz Kiefer <moritz.kiefer@purelyfunctional.org> dateTue 18 Jun 2019, 14:08 (7 years ago) parentd0ab8cb
3 files changed+41−11
src/Development/IDE/State/FileStore.hs+1−1 src/Development/IDE/State/Service.hs+23−4 src/Development/IDE/State/Shake.hs+17−6
src/Development/IDE/State/FileStore.hs+1−1
@@ -161,7 +161,7 @@ setBufferModified state absFile mbContents = do
161161 case mbContents of
162162 Nothing -> removeVirtualFile (filePathToUri' absFile)
163163 Just contents -> setVirtualFileContents (filePathToUri' absFile) contents
164− void $ shakeRun state []
164+ void $ shakeRun state [] (const $ pure ())
165165
166166
167167-- would be nice to do this more efficiently...
src/Development/IDE/State/Service.hs+23−4
@@ -13,6 +13,7 @@ module Development.IDE.State.Service(
1313 getServiceEnv,
1414 IdeState, initialise, shutdown,
1515 runAction, runActions,
16+ runActionSync, runActionsSync,
1617 setFilesOfInterest, modifyFilesOfInterest,
1718 writeProfile,
1819 getDiagnostics, unsafeClearDiagnostics,
@@ -100,14 +101,32 @@ setProfiling opts shakeOpts =
100101shutdown :: IdeState -> IO ()
101102shutdown = shakeShut
102103
103−-- | Run a single action using the supplied service.
104+-- | Run a single action using the supplied service. See `runActions`
105+-- for more details.
104106runAction :: IdeState -> Action a -> IO a
105107runAction service action = head <$> runActions service [action]
106108
107109-- | Run a list of actions in parallel using the supplied service.
110+-- This will return as soon as the results of the actions are
111+-- available. There might still be other rules running at this point,
112+-- e.g., the ofInterestRule.
108113runActions :: IdeState -> [Action a] -> IO [a]
109−runActions x = join . shakeRun x
110−
114+runActions x acts = do
115+ var <- newBarrier
116+ _ <- shakeRun x acts (signalBarrier var)
117+ waitBarrier var
118+
119+-- | This is a synchronous variant of `runAction`. See
120+-- `runActionsSync` of more details.
121+runActionSync :: IdeState -> Action a -> IO a
122+runActionSync s a = head <$> runActionsSync s [a]
123+
124+-- | `runActionsSync` is similar to `runActions` but it will
125+-- wait for all rules (so in particular the `ofInterestRule`) to
126+-- finish running. This is mainly useful in tests, where you want
127+-- to wait for all rules to fire so you can check diagnostics.
128+runActionsSync :: IdeState -> [Action a] -> IO [a]
129+runActionsSync s acts = join $ shakeRun s acts (const $ pure ())
111130
112131-- | Set the files-of-interest which will be built and kept-up-to-date.
113132setFilesOfInterest :: IdeState -> Set NormalizedFilePath -> IO ()
@@ -118,7 +137,7 @@ modifyFilesOfInterest state f = do
118137 Env{..} <- getIdeGlobalState state
119138 files <- modifyVar envOfInterestVar $ pure . dupe . f
120139 logDebug state $ "Set files of interest to: " <> T.pack (show $ Set.toList files)
121− void $ shakeRun state []
140+ void $ shakeRun state [] (const $ pure ())
122141
123142getServiceEnv :: Action Env
124143getServiceEnv = getIdeGlobalAction
src/Development/IDE/State/Shake.hs+17−6
@@ -52,7 +52,7 @@ import qualified Data.HashMap.Strict as Map
5252import qualified Data.ByteString.Char8 as BS
5353import Data.Dynamic
5454import Data.Maybe
55−import Data.Either
55+import Data.Either.Extra
5656import Data.List.Extra
5757import qualified Data.Text as T
5858import Development.IDE.Logger as Logger
@@ -232,20 +232,31 @@ shakeProfile :: IdeState -> FilePath -> IO ()
232232shakeProfile IdeState{..} = shakeProfileDatabase shakeDb
233233
234234shakeShut :: IdeState -> IO ()
235−shakeShut = shakeClose
235+shakeShut IdeState{..} = withVar shakeAbort $ \stop -> do
236+ -- Shake gets unhappy if you try to close when there is a running
237+ -- request so we first abort that.
238+ stop
239+ shakeClose
236240
237241-- | Spawn immediately, add an action to collect the results syncronously.
238242-- If you are already inside a call to shakeRun that will be aborted with an exception.
239−shakeRun :: IdeState -> [Action a] -> IO (IO [a])
243+-- The callback will be fired as soon as the results are available
244+-- even if there are still other rules running while the IO action that is
245+-- being returned will wait for all rules to finish.
246+shakeRun :: IdeState -> [Action a] -> ([a] -> IO ()) -> IO (IO [a])
240247-- FIXME: If there is already a shakeRun queued up and waiting to send me a kill, I should probably
241248-- not even start, which would make issues with async exceptions less problematic.
242−shakeRun IdeState{shakeExtras=ShakeExtras{..}, ..} acts = modifyVar shakeAbort $ \stop -> do
249+shakeRun IdeState{shakeExtras=ShakeExtras{..}, ..} acts callback = modifyVar shakeAbort $ \stop -> do
243250 (stopTime,_) <- duration stop
244251 Logger.logDebug logger $ T.pack $ "Starting shakeRun (aborting the previous one took " ++ showDuration stopTime ++ ")"
245252 bar <- newBarrier
246253 start <- offsetTime
247− thread <- forkFinally (shakeRunDatabaseProfile shakeDb acts) $ \res -> do
248− signalBarrier bar res
254+ let act = do
255+ res <- parallel acts
256+ liftIO $ callback res
257+ pure res
258+ thread <- forkFinally (shakeRunDatabaseProfile shakeDb [act]) $ \res -> do
259+ signalBarrier bar (mapRight head res)
249260 runTime <- start
250261 Logger.logDebug logger $ T.pack $
251262 "Finishing shakeRun (took " ++ showDuration runTime ++ ", " ++ (if isLeft res then "exception" else "completed") ++ ")"