Skip to content

Commit

Permalink
Fix concurrent cache invalidation bug, code cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
josephsumabat committed Nov 9, 2024
1 parent a2e2ffe commit 66d784a
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 17 deletions.
1 change: 1 addition & 0 deletions package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ dependencies:
- sqlite-simple >= 0.4.18 && < 0.5
- template-haskell >= 2.19.0 && < 2.23
- text >= 2.0.1 && < 2.2
- time >= 1.0 && < 2.0
- bytestring >=0.10 && <0.13
- transformers >= 0.5.6.2 && < 0.7
- unliftio-core >= 0.2.1 && < 0.3
Expand Down
11 changes: 11 additions & 0 deletions src/Data/ConcurrentCache.hs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@
module Data.ConcurrentCache (
ConcurrentCache,
new,
Data.ConcurrentCache.lookup,
remove,
insert,
) where

import Control.Monad.IO.Class (MonadIO (..))
import Control.Monad.IO.Unlift (MonadUnliftIO)
import Control.Monad.Trans.Class
import Control.Monad.Trans.Maybe
import Data.HashMap.Strict (HashMap)
import Data.HashMap.Strict qualified as HashMap
import Data.Hashable (Hashable)
import StaticLS.Maybe
import UnliftIO (onException)
import UnliftIO.Exception (mask)
import UnliftIO.IORef (IORef)
Expand All @@ -37,6 +41,13 @@ remove k cache = do
IORef.atomicModifyIORef' cache.map \m -> do
(HashMap.delete k m, ())

lookup :: (MonadIO m, Hashable k) => k -> ConcurrentCache k v -> m (Maybe v)
lookup k cache = do
runMaybeT $ do
cacheMap <- lift $ IORef.readIORef cache.map
(m :: MVar (Maybe v)) <- toAlt $ HashMap.lookup k cacheMap
MaybeT $ MVar.readMVar m

insert :: (Hashable k, MonadUnliftIO m) => k -> m v -> ConcurrentCache k v -> m v
insert k act cache = mask \restore -> do
var <- MVar.newEmptyMVar
Expand Down
11 changes: 10 additions & 1 deletion src/StaticLS/FilePath.hs
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
module StaticLS.FilePath (modToFilePath, subRootExtensionFilepath) where
module StaticLS.FilePath (modToFilePath, subRootExtensionFilepath, getFileModifiedAt) where

import Control.Monad.IO.Class
import Control.Monad.Trans.Maybe
import Data.List qualified as List
import Data.Path (AbsPath, Path (..), RelPath)
import Data.Path qualified as Path
import Data.Time
import GHC.Plugins qualified as GHC
import StaticLS.SrcFiles
import System.Directory qualified as Dir
import System.FilePath ((-<.>))
import System.IO.Error

getFileModifiedAt :: (MonadIO m) => AbsPath -> MaybeT m UTCTime
getFileModifiedAt path = do
MaybeT $
liftIO $
(Just <$> Dir.getModificationTime (Path.toFilePath path))
`catchIOError` const (pure Nothing)

modToFilePath :: GHC.ModuleName -> String -> RelPath
modToFilePath modName ext =
Expand Down
5 changes: 3 additions & 2 deletions src/StaticLS/Handlers.hs
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,9 @@ handleDidChange = LSP.notificationHandler LSP.SMethod_TextDocumentDidChange $ \m
handleDidSave :: Handlers (LspT c StaticLsM)
handleDidSave = LSP.notificationHandler LSP.SMethod_TextDocumentDidSave $ \message -> do
let params = message._params
let _uri = params._textDocument._uri
pure ()
let uri = params._textDocument._uri
-- Useful to invalidate for file watchers if a branch checkout invalidates the file state cache
updateFileStateForUri uri

handleDidClose :: Handlers (LspT c StaticLsM)
handleDidClose = LSP.notificationHandler LSP.SMethod_TextDocumentDidClose $ \_ -> do
Expand Down
2 changes: 1 addition & 1 deletion src/StaticLS/HieView/Query.hs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ fileEvidenceBinds file =
)
file
where
evSourceNames (EvInstBind {cls}) = Nothing
evSourceNames (EvInstBind {}) = Nothing
evSourceNames (EvLetBind (EvBindDeps {deps})) = Just deps
evSourceNames EvOther = Nothing

Expand Down
4 changes: 2 additions & 2 deletions src/StaticLS/HieView/Type.hs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module StaticLS.HieView.Type (
Type,
TypeArray,
FlatType,
TypeArray (..),
FlatType (..),
TypeIndex (..),
fromGHCHieType,
fromGHCHieTypeFlat,
Expand Down
12 changes: 6 additions & 6 deletions src/StaticLS/Hir/Make.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ mkModuleText parts =
, text = T.intercalate "." (NE.toList parts)
}

mkModuleName :: NonEmpty Text -> ModuleName
mkModuleName parts =
ModuleName
{ mod = mkModuleText parts
-- , node = AST.defaultNode
}
-- mkModuleName :: NonEmpty Text -> ModuleName
-- mkModuleName parts =
-- ModuleName
-- { mod = mkModuleText parts
-- , node = AST.defaultNode
-- }
2 changes: 1 addition & 1 deletion src/StaticLS/IDE/CodeActions/AddTypeSig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type BindName = Haskell.PrefixId :+ Haskell.Variable :+ Nil

-- For now, it only works with top level declarations
getDeclarationNameAtPos :: Haskell.Haskell -> Pos -> LineCol -> AST.Err (Maybe BindName)
getDeclarationNameAtPos haskell pos lineCol = do
getDeclarationNameAtPos haskell pos _lineCol = do
let node = AST.getDeepestContaining @AddTypeContext (Range.point pos) haskell.dynNode
case node of
Just bind
Expand Down
2 changes: 1 addition & 1 deletion src/StaticLS/IDE/CodeActions/AutoExport.hs
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,5 @@ getDeclarationsAtPoint range decls =
codeAction :: CodeActionContext -> StaticLsM [Assist]
codeAction cx = do
hir <- getHir cx.path
let decls = getDeclarationsAtPoint (Range.point cx.pos) hir.decls
let _decls = getDeclarationsAtPoint (Range.point cx.pos) hir.decls
pure []
4 changes: 2 additions & 2 deletions src/StaticLS/IDE/CodeActions/AutoImport.hs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ createAutoImportCodeActions path mQualifier importMod =
]

codeAction :: CodeActionContext -> StaticLsM [Assist]
codeAction CodeActionContext {path, lineCol, pos} = do
codeAction CodeActionContext {path, pos} = do
hir <- getHir path
modulesToImport <- getModulesToImport path pos

Expand Down Expand Up @@ -135,7 +135,7 @@ data ImportInsertPoint
| AfterImportInsertPoint !Pos

getImportsInsertPoint :: Rope -> Haskell.Haskell -> AST.Err ImportInsertPoint
getImportsInsertPoint rope hs = do
getImportsInsertPoint _rope hs = do
imports <- Tree.getImports hs
header <- Tree.getHeader hs
let headerPos =
Expand Down
2 changes: 1 addition & 1 deletion src/StaticLS/IDE/Definition.hs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ findDefString qual = do
hieDbFindDefString :: (HasStaticEnv m, MonadIO m) => Text -> Maybe Hir.ModuleName -> MaybeT m [HieDb.DefRow]
hieDbFindDefString name mod = do
-- we need to resolve the mod first
let modText = (.mod.text) <$> mod
let _modText = (.mod.text) <$> mod
runHieDbMaybeT
( \hieDb ->
SQL.queryNamed
Expand Down
24 changes: 24 additions & 0 deletions src/StaticLS/IDE/Monad.hs
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@ where

import AST.Haskell qualified as Haskell
import AST.Traversal qualified as AST
import Control.Monad
import Control.Monad.IO.Class
import Control.Monad.Reader
import Control.Monad.Trans.Maybe
import Data.ConcurrentCache (ConcurrentCache)
import Data.ConcurrentCache qualified as ConcurrentCache
import Data.HashMap.Strict qualified as HashMap
import Data.LineCol (LineCol)
import Data.Maybe
import Data.Path (AbsPath, toFilePath)
import Data.Path qualified as Path
import Data.Pos (Pos)
Expand All @@ -57,6 +59,8 @@ import Data.Rope qualified as Rope
import Data.Text (Text)
import Data.Text qualified as T
import Data.Text.IO qualified as T
import Data.Time
import StaticLS.FilePath
import StaticLS.HIE.File qualified as HIE.File
import StaticLS.HieView qualified as HieView
import StaticLS.Hir qualified as Hir
Expand Down Expand Up @@ -193,11 +197,13 @@ data CachedHieFile = CachedHieFile
, file :: HIE.File.HieFile
, fileView :: HieView.File
, hieTokenMap :: RangeMap PositionDiff.Token
, modifiedAt :: UTCTime
}

getHieCacheResult :: (MonadIde m, MonadIO m) => AbsPath -> MaybeT m CachedHieFile
getHieCacheResult path = do
file <- HIE.File.getHieFileFromPath path
modifiedAt <- getFileModifiedAt path
let fileView = HieView.viewHieFile file
let hieSource = fileView.source
let tokens = PositionDiff.lex $ T.unpack hieSource
Expand All @@ -208,12 +214,26 @@ getHieCacheResult path = do
, file = file
, fileView
, hieTokenMap = PositionDiff.tokensToRangeMap tokens
, modifiedAt = modifiedAt
}
pure hieFile

invalidateStaleHieCacheFile :: (MonadIde m, MonadIO m) => AbsPath -> m ()
invalidateStaleHieCacheFile path = do
fmap (fromMaybe ()) $ runMaybeT $ do
env <- getIdeEnv
latestHieModifiedAt <- getFileModifiedAt path
cachedHieFile <- MaybeT $ ConcurrentCache.lookup path env.hieCache
case cachedHieFile of
Just hieFile -> do
when (hieFile.modifiedAt < latestHieModifiedAt) $
ConcurrentCache.remove path env.hieCache
Nothing -> pure ()

getHieCache :: (MonadIde m, MonadIO m) => AbsPath -> MaybeT m CachedHieFile
getHieCache path = do
env <- getIdeEnv
_ <- lift $ invalidateStaleHieCacheFile path
MaybeT $
ConcurrentCache.insert
path
Expand All @@ -228,13 +248,15 @@ forceCachedHieFile
, file = !file
, fileView = !fileView
, hieTokenMap = hieTokenMap
, modifiedAt = !modifiedAt
} =
CachedHieFile
{ hieSource
, hieSourceRope
, file
, fileView
, hieTokenMap
, modifiedAt
}

getHieCacheWithMap :: (MonadIO m, HasStaticEnv m) => AbsPath -> HieCacheMap -> MaybeT m CachedHieFile
Expand All @@ -244,6 +266,7 @@ getHieCacheWithMap path hieCacheMap =
MaybeT $ pure $ Just hieFile
Nothing -> do
file <- HIE.File.getHieFileFromPath path
modifiedAt <- getFileModifiedAt path
let fileView = HieView.viewHieFile file
let hieSource = fileView.source
let tokens = PositionDiff.lex $ T.unpack hieSource
Expand All @@ -254,6 +277,7 @@ getHieCacheWithMap path hieCacheMap =
, file = file
, fileView
, hieTokenMap = PositionDiff.tokensToRangeMap tokens
, modifiedAt = modifiedAt
}
pure hieFile

Expand Down
5 changes: 5 additions & 0 deletions static-ls.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ library
, text >=2.0.1 && <2.2
, text-range
, text-rope ==0.2
, time >=1.0 && <2.0
, transformers >=0.5.6.2 && <0.7
, tree-sitter-ast
, tree-sitter-haskell
Expand Down Expand Up @@ -234,6 +235,7 @@ executable print-hie
, text >=2.0.1 && <2.2
, text-range
, text-rope ==0.2
, time >=1.0 && <2.0
, transformers >=0.5.6.2 && <0.7
, tree-sitter-ast
, tree-sitter-haskell
Expand Down Expand Up @@ -302,6 +304,7 @@ executable static-ls
, text >=2.0.1 && <2.2
, text-range
, text-rope ==0.2
, time >=1.0 && <2.0
, transformers >=0.5.6.2 && <0.7
, tree-sitter-ast
, tree-sitter-haskell
Expand Down Expand Up @@ -387,6 +390,7 @@ test-suite expect_tests
, text >=2.0.1 && <2.2
, text-range
, text-rope ==0.2
, time >=1.0 && <2.0
, transformers >=0.5.6.2 && <0.7
, tree-sitter-ast
, tree-sitter-haskell
Expand Down Expand Up @@ -458,6 +462,7 @@ test-suite static-ls-test
, text >=2.0.1 && <2.2
, text-range
, text-rope ==0.2
, time >=1.0 && <2.0
, transformers >=0.5.6.2 && <0.7
, tree-sitter-ast
, tree-sitter-haskell
Expand Down

0 comments on commit 66d784a

Please sign in to comment.