remove garbage collection of not visited keys

The "visited age" metric is not accurate in hls-graph because of reverse-dependencies-guided work avoidance
haskell · Oct 17, 2021 · 2897dd3 · 2897dd3
1 parent 1e2a291
commit 2897dd3
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 34 deletions.
diff --git a/ghcide/src/Development/IDE/Core/OfInterest.hs b/ghcide/src/Development/IDE/Core/OfInterest.hs
@@ -111,9 +111,6 @@ kick = do
 
     liftIO $ progressUpdate progress KickCompleted
 
-    -- if idle, perform garbage collection
+    -- if idle, perform garbage collection of dirty keys
     liftIO $ sleep 5
     void garbageCollectDirtyKeys
-
-    -- if still idle, collect unpopular keys
-    void garbageCollectKeysNotVisited
diff --git a/ghcide/src/Development/IDE/Core/Shake.hs b/ghcide/src/Development/IDE/Core/Shake.hs
@@ -76,8 +76,6 @@ module Development.IDE.Core.Shake(
     addPersistentRule,
     garbageCollectDirtyKeys,
     garbageCollectDirtyKeysOlderThan,
-    garbageCollectKeysNotVisited,
-    garbageCollectKeysNotVisitedFor
     ) where
 
 import           Control.Concurrent.Async
@@ -765,22 +763,11 @@ garbageCollectDirtyKeys = do
     checkParents <- liftIO optCheckParents
     garbageCollectDirtyKeysOlderThan optMaxDirtyAge checkParents
 
-garbageCollectKeysNotVisited :: Action [Key]
-garbageCollectKeysNotVisited = do
-    IdeOptions{optCheckParents, optMaxDirtyAge} <- getIdeOptions
-    checkParents <- liftIO optCheckParents
-    garbageCollectKeysNotVisitedFor optMaxDirtyAge checkParents
-
 garbageCollectDirtyKeysOlderThan :: Int -> CheckParents -> Action [Key]
 garbageCollectDirtyKeysOlderThan maxAge checkParents = otTracedGarbageCollection "dirty GC" $ do
     dirtySet <- fromMaybe [] <$> getDirtySet
     garbageCollectKeys "dirty GC" maxAge checkParents dirtySet
 
-garbageCollectKeysNotVisitedFor :: Int -> CheckParents -> Action [Key]
-garbageCollectKeysNotVisitedFor maxAge checkParents = otTracedGarbageCollection "not visited GC" $ do
-    keys <- getKeysAndVisitedAge
-    garbageCollectKeys "not visited GC" maxAge checkParents keys
-
 garbageCollectKeys :: String -> Int -> CheckParents -> [(Key, Int)] -> Action [Key]
 garbageCollectKeys label maxAge checkParents agedKeys = do
     start <- liftIO offsetTime

diff --git a/ghcide/src/Development/IDE/Plugin/Test.hs b/ghcide/src/Development/IDE/Plugin/Test.hs
@@ -48,7 +48,6 @@ data TestRequest
     | WaitForShakeQueue -- ^ Block until the Shake queue is empty. Returns Null
     | WaitForIdeRule String Uri      -- ^ :: WaitForIdeRuleResult
     | GarbageCollectDirtyKeys CheckParents Age    -- ^ :: [String] (list of keys collected)
-    | GarbageCollectNotVisitedKeys CheckParents Age -- ^ :: [String]
     | GetStoredKeys                  -- ^ :: [String] (list of keys in store)
     | GetFilesOfInterest             -- ^ :: [FilePath]
     deriving Generic
@@ -100,9 +99,6 @@ testRequestHandler s (WaitForIdeRule k file) = liftIO $ do
 testRequestHandler s (GarbageCollectDirtyKeys parents age) = do
     res <- liftIO $ runAction "garbage collect dirty" s $ garbageCollectDirtyKeysOlderThan age parents
     return $ Right $ toJSON $ map show res
-testRequestHandler s (GarbageCollectNotVisitedKeys parents age) = do
-    res <- liftIO $ runAction "garbage collect not visited" s $ garbageCollectKeysNotVisitedFor age parents
-    return $ Right $ toJSON $ map show res
 testRequestHandler s GetStoredKeys = do
     keys <- liftIO $ HM.keys <$> readVar (state $ shakeExtras s)
     return $ Right $ toJSON $ map show keys

diff --git a/ghcide/test/exe/Main.hs b/ghcide/test/exe/Main.hs
@@ -55,8 +55,7 @@ import           Development.IDE.Test                     (Cursor,
                                                            getStoredKeys,
                                                            waitForTypecheck,
                                                            getFilesOfInterest,
-                                                           waitForBuildQueue,
-                                                           garbageCollectNotVisitedKeys)
+                                                           waitForBuildQueue)
 import           Development.IDE.Test.Runfiles
 import qualified Development.IDE.Types.Diagnostics        as Diagnostics
 import           Development.IDE.Types.Location
@@ -5847,7 +5846,6 @@ unitTests = do
 garbageCollectionTests :: TestTree
 garbageCollectionTests = testGroup "garbage collection"
   [ testGroup "dirty keys" (sharedGCtests $ garbageCollectDirtyKeys CheckOnSaveAndClose)
-  , testGroup "unvisited keys" (sharedGCtests $ garbageCollectNotVisitedKeys CheckOnSaveAndClose)
   ]
   where
     sharedGCtests gc =
@@ -5870,19 +5868,19 @@ garbageCollectionTests = testGroup "garbage collection"
             liftIO $ writeFile (dir </> "hie.yaml") "cradle: {direct: {arguments: [A.hs, B.hs]}}"
             void $ generateGarbage "A" dir
 
-            keysA <- getStoredKeys
-
             reopenB <- generateGarbage "B" dir
             -- garbage collect A keys
-            garbage <- gc 1
+            keysBeforeGC <- getStoredKeys
+            garbage <- gc 2
             liftIO $ assertBool "something is wrong with this test - no garbage found" $ not $ null garbage
-            keysB <- getStoredKeys
-            liftIO $ assertBool "something is wrong with this test - keys were not deleted from the state" (length keysB < length keysA)
+            keysAfterGC <- getStoredKeys
+            liftIO $ assertBool "something is wrong with this test - keys were not deleted from the state" (length keysAfterGC < length keysBeforeGC)
             ff <- getFilesOfInterest
             liftIO $ assertBool ("something is wrong with this test - files of interest is " <> show ff) (null ff)
 
             -- typecheck B again
-            _ <- reopenB
+            doc <- reopenB
+            void $ waitForTypecheck doc
 
             -- review the keys in store now to validate that A keys have not been regenerated
             keysB' <- getStoredKeys
@@ -5922,6 +5920,7 @@ garbageCollectionTests = testGroup "garbage collection"
         -- dirty the garbage
         sendNotification SWorkspaceDidChangeWatchedFiles $ DidChangeWatchedFilesParams $
             List [FileEvent (filePathToUri $ dir </> modName <> ".hs") FcChanged ]
+
         return $ openDoc (modName <> ".hs") "haskell"
 
 findResolution_us :: Int -> IO Int

diff --git a/ghcide/test/src/Development/IDE/Test.hs b/ghcide/test/src/Development/IDE/Test.hs
@@ -25,7 +25,6 @@ module Development.IDE.Test
   , waitForTypecheck
   , waitForBuildQueue
   , getStoredKeys
-  , garbageCollectNotVisitedKeys
   ) where
 
 import           Control.Applicative.Combinators
@@ -194,9 +193,6 @@ waitForAction key TextDocumentIdentifier{_uri} = callTestPlugin (WaitForIdeRule
 garbageCollectDirtyKeys :: CheckParents -> Int -> Session [String]
 garbageCollectDirtyKeys parents age = callTestPlugin (GarbageCollectDirtyKeys parents age)
 
-garbageCollectNotVisitedKeys :: CheckParents -> Int -> Session [String]
-garbageCollectNotVisitedKeys parents age = callTestPlugin (GarbageCollectNotVisitedKeys parents age)
-
 getStoredKeys :: Session [String]
 getStoredKeys = callTestPlugin GetStoredKeys