Skip to content
This repository was archived by the owner on Jan 2, 2021. It is now read-only.

Improve performance by caching conversion to NormalizedUri #384

Merged
merged 3 commits into from
Jan 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/Development/IDE/Core/Debouncer.hs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import Control.Concurrent.Extra
import Control.Concurrent.Async
import Control.Exception
import Control.Monad.Extra
import Data.Map.Strict (Map)
import qualified Data.Map.Strict as Map
import Data.Hashable
import Data.HashMap.Strict (HashMap)
import qualified Data.HashMap.Strict as Map
import System.Time.Extra

-- | A debouncer can be used to avoid triggering many events
Expand All @@ -21,7 +22,7 @@ import System.Time.Extra
-- by delaying each event for a given time. If another event
-- is registered for the same key within that timeframe,
-- only the new event will fire.
newtype Debouncer k = Debouncer (Var (Map k (Async ())))
newtype Debouncer k = Debouncer (Var (HashMap k (Async ())))

-- | Create a new empty debouncer.
newDebouncer :: IO (Debouncer k)
Expand All @@ -35,7 +36,7 @@ newDebouncer = do
-- If there is a pending event for the same key, the pending event will be killed.
-- Events are run unmasked so it is up to the user of `registerEvent`
-- to mask if required.
registerEvent :: Ord k => Debouncer k -> Seconds -> k -> IO () -> IO ()
registerEvent :: (Eq k, Hashable k) => Debouncer k -> Seconds -> k -> IO () -> IO ()
registerEvent (Debouncer d) delay k fire = modifyVar_ d $ \m -> mask_ $ do
whenJust (Map.lookup k m) cancel
a <- asyncWithUnmask $ \unmask -> unmask $ do
Expand Down
50 changes: 43 additions & 7 deletions src/Development/IDE/Types/Location.hs
Original file line number Diff line number Diff line change
Expand Up @@ -49,22 +49,55 @@ import Language.Haskell.LSP.Types as LSP (
)
import SrcLoc as GHC
import Text.ParserCombinators.ReadP as ReadP
import GHC.Generics


-- | Newtype wrapper around FilePath that always has normalized slashes.
newtype NormalizedFilePath = NormalizedFilePath FilePath
deriving (Eq, Ord, Show, Hashable, NFData, Binary)
-- The NormalizedUri and hash of the FilePath are cached to avoided
-- repeated normalisation when we need to compute them (which is a lot).
--
-- This is one of the most performance critical parts of ghcide, do not
-- modify it without profiling.
data NormalizedFilePath = NormalizedFilePath NormalizedUriWrapper !Int !FilePath
deriving (Generic, Eq, Ord)

instance NFData NormalizedFilePath where
instance Binary NormalizedFilePath where
put (NormalizedFilePath _ _ fp) = put fp
get = do
v <- Data.Binary.get :: Get FilePath
return (toNormalizedFilePath v)


instance Show NormalizedFilePath where
show (NormalizedFilePath _ _ fp) = "NormalizedFilePath " ++ show fp

instance Hashable NormalizedFilePath where
hash (NormalizedFilePath _ h _) = h

-- Just to define NFData and Binary
newtype NormalizedUriWrapper =
NormalizedUriWrapper { unwrapNormalizedFilePath :: NormalizedUri }
deriving (Show, Generic, Eq, Ord)

instance NFData NormalizedUriWrapper where
rnf = rwhnf
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That NFData instance isn't correct either. Can we just lose it?

@ndmitchell does Shake care about these NFData instances?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, it is fine



instance Hashable NormalizedUriWrapper where

instance IsString NormalizedFilePath where
fromString = toNormalizedFilePath

toNormalizedFilePath :: FilePath -> NormalizedFilePath
-- We want to keep empty paths instead of normalising them to "."
toNormalizedFilePath "" = NormalizedFilePath ""
toNormalizedFilePath fp = NormalizedFilePath $ normalise fp
toNormalizedFilePath "" = NormalizedFilePath (NormalizedUriWrapper emptyPathUri) (hash ("" :: String)) ""
toNormalizedFilePath fp =
let nfp = normalise fp
in NormalizedFilePath (NormalizedUriWrapper $ filePathToUriInternal' nfp) (hash nfp) nfp

fromNormalizedFilePath :: NormalizedFilePath -> FilePath
fromNormalizedFilePath (NormalizedFilePath fp) = fp
fromNormalizedFilePath (NormalizedFilePath _ _ fp) = fp

-- | We use an empty string as a filepath when we don’t have a file.
-- However, haskell-lsp doesn’t support that in uriToFilePath and given
Expand All @@ -76,10 +109,13 @@ uriToFilePath' uri
| otherwise = LSP.uriToFilePath uri

emptyPathUri :: NormalizedUri
emptyPathUri = filePathToUri' ""
emptyPathUri = filePathToUriInternal' ""

filePathToUri' :: NormalizedFilePath -> NormalizedUri
filePathToUri' (NormalizedFilePath fp) = toNormalizedUri $ Uri $ T.pack $ LSP.fileScheme <> "//" <> platformAdjustToUriPath fp
filePathToUri' (NormalizedFilePath (NormalizedUriWrapper u) _ _) = u

filePathToUriInternal' :: FilePath -> NormalizedUri
filePathToUriInternal' fp = toNormalizedUri $ Uri $ T.pack $ LSP.fileScheme <> "//" <> platformAdjustToUriPath fp
where
-- The definitions below are variants of the corresponding functions in Language.Haskell.LSP.Types.Uri that assume that
-- the filepath has already been normalised. This is necessary since normalising the filepath has a nontrivial cost.
Expand Down