[Git][ghc/ghc][wip/romes/graph-compact-easy] more clean up and comments

Rodrigo Mesquita (@alt-romes) gitlab at gitlab.haskell.org
Tue Jan 7 17:26:02 UTC 2025



Rodrigo Mesquita pushed to branch wip/romes/graph-compact-easy at Glasgow Haskell Compiler / GHC


Commits:
7cbac5dd by Rodrigo Mesquita at 2025-01-07T17:25:40+00:00
more clean up and comments

- - - - -


4 changed files:

- compiler/GHC/Tc/Utils/Monad.hs
- compiler/GHC/Unit/Env.hs
- compiler/GHC/Unit/Home/Graph.hs
- compiler/GHC/Unit/Home/PackageTable.hs


Changes:

=====================================
compiler/GHC/Tc/Utils/Monad.hs
=====================================
@@ -2347,9 +2347,6 @@ getCompleteMatchesTcM
 
 localAndImportedCompleteMatches :: CompleteMatches -> UnitEnv -> ExternalPackageState -> IO CompleteMatches
 localAndImportedCompleteMatches tcg_comps unit_env eps = do
-  -- ROMES:TODO: this whole could be a single query to the UnitEnv, rather than
-  -- fetching the HUG and EPS separately. this is the only occurrence of
-  -- hugcompletesigs/hptcompletesigs...
   hugCSigs <- hugCompleteSigs unit_env
   return $
        tcg_comps                -- from the current module


=====================================
compiler/GHC/Unit/Env.hs
=====================================
@@ -153,7 +153,7 @@ hugCompleteSigs :: UnitEnv -> IO CompleteMatches
 hugCompleteSigs = HUG.allCompleteSigs . ue_home_unit_graph
 
 --------------------------------------------------------------------------------
--- TODO::....
+-- UnitEnv
 --------------------------------------------------------------------------------
 
 data UnitEnv = UnitEnv


=====================================
compiler/GHC/Unit/Home/Graph.hs
=====================================
@@ -100,13 +100,10 @@ allCompleteSigs hug = foldr go (pure []) hug where
 -- Used in @tcRnImports@, to select the instances that are in the
 -- transitive closure of imports from the currently compiled module.
 allInstances :: HomeUnitGraph -> IO (InstEnv, [FamInst])
--- ROMES:TODO: Figure out why the return type of allInstances is this
--- "[FamInstance]" rather than the something like "famInstances"
 allInstances hug = foldr go (pure (emptyInstEnv, [])) hug where
   go hue = liftA2 (\(a,b) (a',b') -> (a `unionInstEnv` a', b ++ b'))
                   (hptAllInstances (homeUnitEnv_hpt hue))
 
--- | ROMES:TODO: Write a comment
 allFamInstances :: HomeUnitGraph -> IO (ModuleEnv FamInstEnv)
 allFamInstances hug = foldr go (pure emptyModuleEnv) hug where
   go hue = liftA2 plusModuleEnv (hptAllFamInstances (homeUnitEnv_hpt hue))
@@ -116,7 +113,7 @@ allAnns hug = foldr go (pure emptyAnnEnv) hug where
   go hue = liftA2 plusAnnEnv (hptAllAnnotations (homeUnitEnv_hpt hue))
 
 --------------------------------------------------------------------------------
--- OK?
+-- HomeUnitGraph (HUG)
 --------------------------------------------------------------------------------
 
 type HomeUnitGraph = UnitEnvGraph HomeUnitEnv
@@ -186,6 +183,9 @@ addHomeModInfoToHug hmi hug =
     hmi_mod  = mi_module (hm_iface hmi)
     hmi_unit = toUnitId (moduleUnit hmi_mod)
 
+-- | Thin each HPT variable to only contain keys from the given dependencies.
+-- This is used at the end of upsweep to make sure that only completely successfully loaded
+-- modules are visible for subsequent operations.
 restrictHug :: [(UnitId, [HomeModInfo])] -> HomeUnitGraph -> IO ()
 restrictHug deps hug = unitEnv_foldWithKey (\k uid hue -> restrict_one uid hue >> k) (return ()) hug
   where
@@ -222,7 +222,6 @@ updateUnitFlags uid f = unitEnv_adjust update uid
 -- | Compute the transitive closure of a unit in the 'HomeUnitGraph'.
 -- If the argument unit is not present in the graph returns Nothing.
 transitiveHomeDeps :: UnitId -> HomeUnitGraph -> Maybe [UnitId]
--- todo: is it worth it caching this computation further?
 transitiveHomeDeps uid hug = case lookupHugUnit uid hug of
   Nothing -> Nothing
   Just hue -> Just $


=====================================
compiler/GHC/Unit/Home/PackageTable.hs
=====================================
@@ -117,14 +117,17 @@ newtype HomePackageTable = HPT {
     -- ^ Domain = modules in the home unit that have been fully compiled
     -- "home" unit id cached (implicit) here for convenience.
     --
-    -- This is an IORef because the HPT musn't leak; We want to always augment
-    -- it, and handle rehydration such that rehydrated modules point to the
-    -- actual modules rather than to hs-boot files... Previously we did this by
-    -- tying a knot on the lazy HPT, but this leaked the HPT, ...
+    -- This is an IORef because we want to avoid leaking HPTs (see the particularly bad #25511).
+    -- Moreover, the HPT invariant allows mutability in this table without compromising thread safety or soundness.
+    -- To recall:
+    --   A query to the HPT should depend only on data relevant to that query, such that
+    --   there being more or less unrelated entries in the HPT does not influence the result in any way.
     --
-    -- The elements of this table may be updated (e.g. on rehydration).
+    -- Note that the HPT increases monotonically, except at certain barrier
+    -- points like when 'restrictHpt' is called. At these barriers, it is safe
+    -- to temporarily violate the HPT monotonicity.
     --
-    -- ROMES:TODO: Explain!!!!!
+    -- The elements of this table may be updated (e.g. on rehydration).
   }
 
 -- | Create a new 'HomePackageTable'.
@@ -183,6 +186,14 @@ addToHpt HPT{table=hptr} mn hmi = do
 addHomeModInfosToHpt :: HomePackageTable -> [HomeModInfo] -> IO ()
 addHomeModInfosToHpt hpt = mapM_ (flip addHomeModInfoToHpt hpt)
 
+-- | Thin each HPT variable to only contain keys from the given dependencies.
+-- This is used at the end of upsweep to make sure that only completely successfully loaded
+-- modules are visible for subsequent operations.
+--
+-- This is an exception to the invariant of the HPT -- that it grows
+-- monotonically, never removing entries -- which is safe as long as it is only
+-- called at barrier points, such as the end of upsweep, when all threads are
+-- done and we want to clean up failed entries.
 restrictHpt :: HomePackageTable -> [HomeModInfo] -> IO ()
 restrictHpt HPT{table=hptr} hmis =
   let key_set = map (getKey . getUnique . hmi_mod) hmis
@@ -203,11 +214,7 @@ addListToHpt hpt = mapM_ (uncurry (addToHpt hpt))
 hptCompleteSigs :: HomePackageTable -> IO CompleteMatches
 hptCompleteSigs = concatHpt (md_complete_matches . hm_details)
 
--- | Find all the instance declarations (of classes and families) from
--- the Home Package Table filtered by the provided predicate function.
--- Used in @tcRnImports@, to select the instances that are in the
--- transitive closure of imports from the currently compiled module.
--- ROMES:TODO: wait what?
+-- | Find all the instance declarations (of classes and families) from this Home Package Table
 hptAllInstances :: HomePackageTable -> IO (InstEnv, [FamInst])
 hptAllInstances hpt = do
   hits <- flip concatHpt hpt $ \mod_info -> do



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/7cbac5dd4212441e68b54e3774843689ece27d64

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/7cbac5dd4212441e68b54e3774843689ece27d64
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20250107/94056eff/attachment-0001.html>


More information about the ghc-commits mailing list