[Git][ghc/ghc][wip/T8095-spj] 2 commits: Two perf wibbles

[Simon Peyton Jones]

Simon Peyton Jones pushed to branch wip/T8095-spj at Glasgow Haskell Compiler / GHC


Commits:
72570fae by Simon Peyton Jones at 2020-08-05T11:53:56+01:00
Two perf wibbles

- - - - -
d35f1fef by Simon Peyton Jones at 2020-08-05T12:57:42+01:00
Further wibbles

- - - - -


3 changed files:

- compiler/GHC/Core/TyCo/FVs.hs
- compiler/GHC/Core/TyCo/Rep.hs
- compiler/GHC/Core/Unify.hs


Changes:

=====================================
compiler/GHC/Core/TyCo/FVs.hs
=====================================
@@ -268,13 +268,17 @@ type InScopeBndrs  = TyCoVarSet
 newtype TyCoAcc acc = TCA' { runTCA :: InScopeBndrs -> acc -> acc }
 
 pattern TCA :: forall acc. (InScopeBndrs -> acc -> acc) -> TyCoAcc acc
+-- See GHC.Core.Unify Note [The one-shot state monad trick]
 {-# COMPLETE TCA #-}
 pattern TCA f <- TCA' f
   where
     TCA f = TCA' (oneShot f)
 
 instance Semigroup (TyCoAcc acc) where
-    (TCA f1) <> (TCA f2) = TCA (\is acc -> f2 is (f1 is acc))
+    (TCA f1) <> (TCA f2) = TCA (\is acc -> f1 is (f2 is acc))
+       -- It makes a perf difference going f1 (f2 acc), rather
+       -- than f2 (f1 acc).  In a short-cutting situation (e.g.
+       -- a boolean accumulator, people put the short-cut first.
     {-# INLINE (<>) #-}
 
 instance Monoid (TyCoAcc acc) where
@@ -287,21 +291,25 @@ emptyInScope :: InScopeBndrs
 emptyInScope = emptyVarSet
 
 addTyCoVar :: TyCoVar -> TyCoAcc TyCoVarSet -> TyCoAcc TyCoVarSet
--- Add a variable tcv, and the extras, to the free vars unless
--- tcv is in the in-scope
---  or is already in the in-scope set-
--- The 'extras' start from an empty in-scope set;
---    see Note [Closing over free variable kinds]
-addTyCoVar tcv (TCA add_extras) = TCA add_it
+-- If 'tcv' is (a) in scope or (b) in the accumulator, then do nothing
+-- Otherwise:
+--   - Add 'tcv' to the accumulator, and
+--   - Run 'add_kind_fvs' starting from an empty in-scope set;
+--     see Note [Closing over free variable kinds]
+addTyCoVar tcv (TCA add_kind_fvs) = TCA add_it
   where
     add_it is acc
       | tcv `elemVarSet` is  = acc
       | tcv `elemVarSet` acc = acc
-      | otherwise            = add_extras emptyVarSet (acc `extendVarSet` tcv)
+      | otherwise            = add_kind_fvs emptyVarSet (acc `extendVarSet` tcv)
 
 extendInScope :: TyCoVar -> TyCoAcc acc -> TyCoAcc acc
 -- Gather the argument free vars in an empty in-scope set
-extendInScope tcv (TCA f) = TCA (\is acc -> f (is `extendVarSet` tcv) acc)
+extendInScope tcv (TCA f) = TCA (\is acc -> let !is' = is `extendVarSet` tcv
+                                            in f is' acc)
+      -- Making this strict removes a thunk, which reduces
+      -- allocation slightly; and the in-scope set is consulted
+      -- at every variable occurrence anyway
 
 whenNotInScope :: TyCoVar -> TyCoAcc acc -> TyCoAcc acc
 -- If tcv is not in the in-scope set, compute


=====================================
compiler/GHC/Core/TyCo/Rep.hs
=====================================
@@ -1819,11 +1819,15 @@ coercion witnessing the reduction. The reduction will proceed as follows:
     ~> S (S (S (Z + S Z)))     |>              CoAx2 (CoAx2 (CoAx2 Refl))
     ~> S (S (S (S (S Z))))     |>              CoAx1 (CoAx2 (CoAx2 (CoAx2 Refl)))
 
-Moreover, coercions are really only useful when validating core transformations
-(i.e. by the Core Linter). To avoid burdening users who aren't linting with the
-cost of maintaining these structures, we replace coercions with placeholders
-("zap" them) them unless -dcore-lint is enabled. These placeholders are
-represented by UnivCo with ZappedProv provenance. Concretely, a coercion
+This looks linear, but the type arguments to CoAx2 are of size N, N-1
+etc; hence quadratic.
+
+Moreover, coercions are really only useful when validating core
+transformations (i.e. by the Core Linter). To avoid burdening users
+who aren't linting with the cost of maintaining these structures, we
+replace coercions with placeholders ("zap" them) them unless
+-dcore-lint is enabled. These placeholders are represented by UnivCo
+with ZappedProv provenance. Concretely, a coercion
 
     co :: t1 ~r t2
 
@@ -1974,6 +1978,8 @@ keyword):
    to avoid unsound floating. This means that the free variable lists of zapped
    coercions loaded from interface files will lack top-level things (e.g. type
    constructors) that appear only in the unzapped proof.
+
+* coercionSize gives a smaller size, of course.
 -}
 
 
@@ -2077,6 +2083,7 @@ data TyCoFolder a
           -- See Note [Coercion holes] in "GHC.Core.TyCo.Rep".
 
       , tcf_tycobinder :: TyCoVar -> ArgFlag -> a -> a
+          -- ^ Modify the result of folding over the body of the forall
       }
 
 noView :: Type -> Maybe Type  -- Simplest form of tcf_view
@@ -2107,7 +2114,7 @@ foldTyCo (TyCoFolder { tcf_view       = view
     go_ty (ForAllTy (Bndr tv vis) inner)
       = go_ty (varType tv) `mappend` tycobinder tv vis (go_ty inner)
 
-    -- Explicit recursion becuase using foldr builds a local
+    -- Explicit recursion because using foldr builds a local
     -- loop (with free) and I'm not confident it'll be
     -- lambda lifted in the end
     go_tys []     = mempty
@@ -2147,8 +2154,9 @@ foldTyCo (TyCoFolder { tcf_view       = view
     go_prov (ZapCoProv cvs)     = go_cvs (dVarSetElems cvs)
       -- This use of dVarSetElems does a sort, which seems very
       -- inefficient.  But it seems required for determinism, alas.
-      -- Unless the 'a' is instantiated with a non-deterministic kind
-      -- of thing...could we exploit that somehow?  ToDo.
+      -- Unless the 'a' is instantiated with a commutative monoid,
+      -- such as (VarSet -> VarSet) ...could we exploit that somehow?
+      -- ToDo.
 
     go_cvs []       = mempty
     go_cvs (cv:cvs) = covar cv `mappend` go_cvs cvs


=====================================
compiler/GHC/Core/Unify.hs
=====================================
@@ -1278,7 +1278,7 @@ and voila!  In summary:
   described here is applicable on a monad-by-monad basis under
   programmer control.
 
-* Beware: itt changes the behaviour of
+* Beware: it changes the behaviour of
      map (foo 3) xs
   ToDo: explain what to do if you want to do this
 -}



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/c5deeadeb1f90b47beab9d91dcb5095b22581efd...d35f1fefd24846dee79a6572c6b957309814d3fd

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/c5deeadeb1f90b47beab9d91dcb5095b22581efd...d35f1fefd24846dee79a6572c6b957309814d3fd
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/20200805/d2493ef8/attachment-0001.html>