Marge Bot pushed to branch wip/marge_bot_batch_merge_job at Glasgow Haskell Compiler / GHC Commits: 93a2b20f by Andreas Klebinger at 2026-08-18T04:31:04-04:00 Fix a number of incorrect module references: Fix module reference in Note [DataCon wrappers are conlike]. Fix module reference in Note [Detailed InertCans Invariants]. Fix module reference in Note [GHC's data format representations]. Fix module reference in Note [Grand plan for static forms]. Fix module reference in Note [How tuples work]. Fix module reference in Note [Solved dictionaries]. Fix module reference in Note [Stacktraces from Info Table Provenance Entries (IPE based stack unwinding)]. Fix module reference in Note [The VarBndr type and its uses]. Found the incorrect references with a llm. - - - - - eb0dfb01 by Simon Jakobi at 2026-08-18T04:31:44-04:00 ci: Run stack-hadrian-build only in full-ci pipelines The job exists to catch changes that break hadrian/build-stack (#18726), but nothing in the pipeline depends on it, and it can only break when hadrian's dependencies change. Restricting it to full-ci (like hadrian-multi) still covers marge-bot merge batches, so such breakage cannot reach master unnoticed, while ordinary validate pipelines skip the job. Assisted-by: Claude Fable 5 - - - - - 4ef2b850 by Simon Jakobi at 2026-08-18T07:42:17-04:00 testsuite: Show baseline sample count and range in perf failures A perf baseline is the mean of all samples recorded for a commit, and it prints as a single number, hiding how far the samples spread. When the spread is wide, this can indicate an unstable metric that isn't actually useful as a signal for the perf tests. For example, in #27602, T27336's peak_megabytes_allocated baseline showed as 757 when the underlying samples were 605 and 909. When the baseline is averaged from more than one sample, say so in the failure output: the one-line stat-failure reason shows the sample range, and the detail block lists the raw samples. Single-sample baselines print exactly as before. Context: #27602 Assisted-by: Claude Fable 5 - - - - - 3117ea9e by Simon Jakobi at 2026-08-18T07:42:17-04:00 testsuite: Fold Baseline into CommitMetric A Baseline was just a CommitMetric plus the commit it came from, built by copying fields across. Since get_commit_metric already knows that commit, record it on CommitMetric itself and drop Baseline. This also collapses both branches of find_baseline into plain returns. Assisted-by: Claude Fable 5 - - - - - dd5bdc95 by Simon Jakobi at 2026-08-18T07:42:17-04:00 ci: Clarify comment on pushing perf notes after failures Context: #27602 Assisted-by: Claude Fable 5 - - - - - bb63527b by Alan Zimmerman at 2026-08-18T07:42:18-04:00 EPA: Remove LocatedBC / SrcSpanBF The custom annotations are now in the BooleanFormula TTG extension points, so LBooleanFormula can now use the standard LocatedA. - - - - - 20 changed files: - .gitlab-ci.yml - .gitlab/ci.sh - compiler/GHC/Builtin/WiredIn/Types.hs - compiler/GHC/CmmToAsm/Format.hs - compiler/GHC/Data/BooleanFormula.hs - compiler/GHC/Driver/GenerateCgIPEStub.hs - compiler/GHC/Hs/Dump.hs - compiler/GHC/Iface/Ext/Ast.hs - compiler/GHC/Iface/Syntax.hs - compiler/GHC/Iface/Tidy/StaticPtrTable.hs - compiler/GHC/IfaceToCore.hs - compiler/GHC/Parser.y - compiler/GHC/Parser/Annotation.hs - compiler/GHC/Tc/Solver/InertSet.hs - compiler/GHC/Types/Id/Make.hs - compiler/GHC/Types/Var.hs - testsuite/driver/perf_notes.py - testsuite/driver/testglobals.py - utils/check-exact/ExactPrint.hs - utils/haddock/haddock-api/src/Haddock/Types.hs Changes: ===================================== .gitlab-ci.yml ===================================== @@ -552,6 +552,8 @@ stack-hadrian-build: - . .gitlab/ci.sh setup - . .gitlab/ci.sh configure - hadrian/build-stack --version + rules: + - *full-ci #################################### # Testing reinstallable ghc codepath ===================================== .gitlab/ci.sh ===================================== @@ -1120,9 +1120,10 @@ case ${1:-help} in setup) setup && cleanup_submodules ;; configure) time_it "configure" configure ;; build_hadrian) time_it "build" build_hadrian ;; - # N.B. Always push notes, even if the build fails. This is okay to do as the - # testsuite driver doesn't record notes for tests that fail due to - # correctness. + # N.B. Always push notes, even if the build fails. Metrics from runs failing + # a perf stat check are deliberately recorded too — discarding them would + # bias the baseline towards whichever sample came first. Only correctness + # failures record nothing. test_hadrian) fetch_perf_notes res=0 ===================================== compiler/GHC/Builtin/WiredIn/Types.hs ===================================== @@ -783,7 +783,7 @@ typeSymbolKind = mkTyConTy typeSymbolKindCon Note [How tuples work] ~~~~~~~~~~~~~~~~~~~~~~ * There are three families of tuple TyCons and corresponding - DataCons, expressed by the type BasicTypes.TupleSort: + DataCons, expressed by the type GHC.Types.Basic.TupleSort: data TupleSort = BoxedTuple | UnboxedTuple | ConstraintTuple * All three families are AlgTyCons, whose AlgTyConRhs is TupleTyCon @@ -818,7 +818,7 @@ Note [How tuples work] stores them in the context field of types like HsQualTy. * In quite a lot of places things are restricted just to - BoxedTuple/UnboxedTuple, and then we used BasicTypes.Boxity to distinguish + BoxedTuple/UnboxedTuple, and then we used GHC.Types.Basic.Boxity to distinguish E.g. tupleTyCon has a Boxity argument * When looking up an OccName in the original-name cache ===================================== compiler/GHC/CmmToAsm/Format.hs ===================================== @@ -52,20 +52,20 @@ import GHC.Utils.Panic GHC has severals types that represent various aspects of data format. These include: - * 'CmmType.CmmType': The data classification used throughout the C-- + * 'GHC.Cmm.Type.CmmType': The data classification used throughout the C-- pipeline. This is a pair of a CmmCat and a Width. - * 'CmmType.CmmCat': What the bits in a C-- value mean (e.g. a pointer, integer, or floating-point value) + * 'GHC.Cmm.Type.CmmCat': What the bits in a C-- value mean (e.g. a pointer, integer, or floating-point value) - * 'CmmType.Width': The width of a C-- value. + * 'GHC.Cmm.Type.Width': The width of a C-- value. - * 'CmmType.Length': The width (measured in number of scalars) of a vector value. + * 'GHC.Cmm.Type.Length': The width (measured in number of scalars) of a vector value. * 'Format.Format': The data format representation used by much of the backend. - * 'Format.ScalarFormat': The format of a 'Format.VecFormat'\'s scalar. + * 'GHC.CmmToAsm.Format.ScalarFormat': The format of a 'GHC.CmmToAsm.Format.VecFormat'\'s scalar. - * 'RegClass.RegClass': Whether a register is an integer or a floating point/vector register. + * 'GHC.Platform.Reg.Class.RegClass': Whether a register is an integer or a floating point/vector register. -} -- It looks very like the old MachRep, but it's now of purely local ===================================== compiler/GHC/Data/BooleanFormula.hs ===================================== @@ -25,7 +25,7 @@ import GHC.Types.Unique import GHC.Types.Unique.Set import GHC.Types.SrcLoc (unLoc) import GHC.Utils.Outputable -import GHC.Parser.Annotation ( SrcSpanAnnBF ) +import GHC.Parser.Annotation ( SrcSpanAnnA, EpToken(..) ) import GHC.Hs.Extension (GhcPass (..), OutputableBndrId) import Language.Haskell.Syntax.Extension (Anno, LIdP, IdP, noExtField, NoExtField, DataConCantHappen, @@ -38,12 +38,12 @@ import Language.Haskell.Syntax.BooleanFormula -- Boolean formula type and smart constructors ---------------------------------------------------------------------- -type instance Anno (BooleanFormula (GhcPass p)) = SrcSpanAnnBF +type instance Anno (BooleanFormula (GhcPass p)) = SrcSpanAnnA type instance XBFVar (GhcPass _) = NoExtField type instance XBFAnd (GhcPass _) = NoExtField type instance XBFOr (GhcPass _) = NoExtField -type instance XBFParens (GhcPass _) = NoExtField +type instance XBFParens (GhcPass _) = (EpToken "(", EpToken ")") type instance XXBooleanFormula (GhcPass _) = DataConCantHappen instance BooleanFormulaDefault (GhcPass p) where ===================================== compiler/GHC/Driver/GenerateCgIPEStub.hs ===================================== @@ -62,7 +62,7 @@ looking up source locations for stack info tables in the map generated during th The rest of this note will document exactly how the first pass generates the map from labels to estimated source positions. The algorithms are different depending on whether tables-next-to-code -is on or off. Both algorithms have in common that we are looking for a `CmmNode.CmmTick` +is on or off. Both algorithms have in common that we are looking for a `GHC.Cmm.Node.CmmTick` (containing a `SourceNote`) that is near what we estimate to be the label of a return stack frame. With tables-next-to-code @@ -112,14 +112,14 @@ open or closed on exit (one can fallthrough from them to the next node). Please refer to the paper "Hoopl: A Modular, Reusable Library for Dataflow Analysis and Transformation" for a detailed explanation. -Here we use the fact, that calls (represented by `CmmNode.CmmCall`) are always closed on exit +Here we use the fact, that calls (represented by `GHC.Cmm.Node.CmmCall`) are always closed on exit (`CmmNode O C`, `O` means open, `C` closed). In other words, they are always at the end of a block. So, given a `CmmGraph`: - - Look at the end of every block: If it is a `CmmNode.CmmCall` returning to some label, lookup - the nearest `CmmNode.CmmTick` by traversing the middle part of the block backwards (from end to + - Look at the end of every block: If it is a `GHC.Cmm.Node.CmmCall` returning to some label, lookup + the nearest `GHC.Cmm.Node.CmmTick` by traversing the middle part of the block backwards (from end to beginning). - - Take the first `CmmNode.CmmTick` that contains a `Tickish.SourceNote` and map the label we + - Take the first `GHC.Cmm.Node.CmmTick` that contains a `Tickish.SourceNote` and map the label we found to it's payload as an `IpeSourceLocation`. (There are other `Tickish` constructors like `ProfNote` or `HpcTick`, these are ignored.) ===================================== compiler/GHC/Hs/Dump.hs ===================================== @@ -97,7 +97,6 @@ showAstData bs ba a0 = blankLine $$ showAstData' a0 `ext2Q` located `extQ` srcSpanAnnA `extQ` srcSpanAnnN - `extQ` srcSpanAnnBF where generic :: Data a => a -> SDoc generic t = parens $ text (showConstr (toConstr t)) @@ -396,10 +395,6 @@ showAstData bs ba a0 = blankLine $$ showAstData' a0 srcSpanAnnN :: EpAnn NameAnn -> SDoc srcSpanAnnN = locatedAnn'' (text "SrcSpanAnnN") - srcSpanAnnBF :: EpAnn AnnBooleanFormula -> SDoc - srcSpanAnnBF = locatedAnn'' (text "SrcSpanAnnBF") - - locatedAnn'' :: forall a. (Typeable a, Data a) => SDoc -> EpAnn a -> SDoc locatedAnn'' tag ss = parens $ ===================================== compiler/GHC/Iface/Ext/Ast.hs ===================================== @@ -2081,7 +2081,7 @@ instance ToHie PendingRnSplice where toHie (PendingRnSplice _ e) = toHie e instance (HiePass p, Data (IdGhcP p)) - => ToHie (GenLocated SrcSpanAnnBF (BooleanFormula (GhcPass p))) where + => ToHie (GenLocated SrcSpanAnnA (BooleanFormula (GhcPass p))) where toHie (L span form) = concatM $ makeNode form (locA span) : case form of Var _ a -> [ toHie $ C Use a ===================================== compiler/GHC/Iface/Syntax.hs ===================================== @@ -1330,7 +1330,7 @@ pprIfaceDecl ss decl@(IfaceClass { ifName = clas fromIfaceBooleanFormula (IfVar nm ) = Var noExtField $ noLocA . mkUnboundName . mkVarOccFS . ifLclNameFS $ nm fromIfaceBooleanFormula (IfAnd bfs ) = And noExtField $ map (noLocA . fromIfaceBooleanFormula) bfs fromIfaceBooleanFormula (IfOr bfs ) = Or noExtField $ map (noLocA . fromIfaceBooleanFormula) bfs - fromIfaceBooleanFormula (IfParens bf) = Parens noExtField $ (noLocA . fromIfaceBooleanFormula) bf + fromIfaceBooleanFormula (IfParens bf) = Parens noAnn $ (noLocA . fromIfaceBooleanFormula) bf -- See Note [Suppressing binder signatures] in GHC.Iface.Type ===================================== compiler/GHC/Iface/Tidy/StaticPtrTable.hs ===================================== @@ -82,7 +82,7 @@ Here is a running example: (SF4) The desugarer replaces a nested expression (static e) with a top-level binding for an application of the function 'makeStatic' (defined in module - GHC.StaticPtr.Internal of base). So we get + GHC.Internal.StaticPtr.Internal of base). So we get s = /\abc. makeStatic location e f x = ...(fromStaticPtr s)... ===================================== compiler/GHC/IfaceToCore.hs ===================================== @@ -121,7 +121,7 @@ import GHC.Types.Tickish import GHC.Types.TyThing import GHC.Types.Error -import GHC.Parser.Annotation (noLocA) +import GHC.Parser.Annotation (noLocA, noAnn) import GHC.Fingerprint @@ -885,7 +885,7 @@ tc_iface_decl _parent ignore_prags tc_boolean_formula :: IfaceBooleanFormula -> IfL (BooleanFormula GhcRn) tc_boolean_formula (IfAnd ibfs ) = BF.And NoExtField . map noLocA <$> traverse tc_boolean_formula ibfs tc_boolean_formula (IfOr ibfs ) = BF.Or NoExtField . map noLocA <$> traverse tc_boolean_formula ibfs - tc_boolean_formula (IfParens ibf) = BF.Parens NoExtField . noLocA <$> tc_boolean_formula ibf + tc_boolean_formula (IfParens ibf) = BF.Parens noAnn . noLocA <$> tc_boolean_formula ibf tc_boolean_formula (IfVar nm ) = BF.Var NoExtField . noLocA <$> (lookupIfaceTop . mkVarOccFS . ifLclNameFS $ nm) mk_sc_doc pred = text "Superclass" <+> ppr pred ===================================== compiler/GHC/Parser.y ===================================== @@ -3820,7 +3820,7 @@ name_boolformula_opt :: { LBooleanFormula GhcPs } name_boolformula :: { LBooleanFormula GhcPs } : name_boolformula_and { $1 } | name_boolformula_and '|' name_boolformula - {% do { h <- addTrailingVbarBF $1 (epTok $2) + {% do { h <- addTrailingVbarA $1 (epTok $2) ; return (sLLa $1 $> (Or noExtField [h,$3])) } } name_boolformula_and :: { LBooleanFormula GhcPs } @@ -3830,12 +3830,11 @@ name_boolformula_and :: { LBooleanFormula GhcPs } name_boolformula_and_list :: { NonEmpty (LBooleanFormula GhcPs) } : name_boolformula_atom { NE.singleton $1 } | name_boolformula_atom ',' name_boolformula_and_list - {% do { h <- addTrailingCommaBF $1 (epTok $2) + {% do { h <- addTrailingCommaA $1 (epTok $2) ; return (h NE.<| $3) } } name_boolformula_atom :: { LBooleanFormula GhcPs } - : '(' name_boolformula ')' {% amsr (sLL $1 $> (Parens noExtField $2)) - (AnnBooleanFormula (epTok $1) (epTok $3) []) } + : '(' name_boolformula ')' {% amsA' (sLL $1 $> (Parens (epTok $1, epTok $3) $2)) } | name_var { sL1a $1 (Var noExtField $1) } namelist :: { Located [LocatedN RdrName] } @@ -4794,20 +4793,6 @@ addTrailingAnnA (L anns a) tok ta = do -- ------------------------------------- -addTrailingVbarBF :: MonadP m => LocatedBF a -> EpToken "|" -> m (LocatedBF a) -addTrailingVbarBF la tok = addTrailingAnnBF la (AddVbarAnn tok) - -addTrailingCommaBF :: MonadP m => LocatedBF a -> EpToken "," -> m (LocatedBF a) -addTrailingCommaBF la tok = addTrailingAnnBF la (AddCommaAnn tok) - -addTrailingAnnBF :: MonadP m => LocatedBF a -> TrailingAnn -> m (LocatedBF a) -addTrailingAnnBF (L anns a) ta = do - !cs <- getCommentsFor (locA anns) - let anns' = addTrailingAnnToBF ta cs anns - return (L anns' a) - --- ------------------------------------- - -- Mostly use to add AnnComma, special case it to NOP if adding a zero-width annotation addTrailingCommaN :: MonadP m => LocatedN a -> SrcSpan -> m (LocatedN a) addTrailingCommaN (L anns a) span = do ===================================== compiler/GHC/Parser/Annotation.hs ===================================== @@ -28,23 +28,19 @@ module GHC.Parser.Annotation ( -- ** Annotations in 'GenLocated' LocatedA, LocatedN, LocatedAn, - LocatedBF, SrcSpanAnnA, SrcSpanAnnN, - SrcSpanAnnBF, -- ** Annotation data types used in 'GenLocated' AnnList(..), AnnListBrackets(..), AnnParen(..), AnnCType(..),AnnWarningTxt(..),AnnOverlap(..),AnnAnnDecl(..),AnnPragSCC(..), - AnnBooleanFormula(..), NameAnn(..), NameAdornment(..), NoEpAnns(..), -- ** Trailing annotations in lists TrailingAnn(..), ta_location, addTrailingAnnToA, addTrailingCommaToN, - addTrailingAnnToBF, noTrailingN, -- ** Utilities for converting between different 'GenLocated' when @@ -430,8 +426,6 @@ emptyComments = EpaComments [] type LocatedA = GenLocated SrcSpanAnnA type LocatedN = GenLocated SrcSpanAnnN -type LocatedBF = GenLocated SrcSpanAnnBF - -- | Annotation for items appearing in a list. They can have one or -- more trailing punctuations items, such as commas or semicolons. type SrcSpanAnnA = EpAnn [TrailingAnn] @@ -440,8 +434,6 @@ type SrcSpanAnnA = EpAnn [TrailingAnn] -- on the context, such as backticks. type SrcSpanAnnN = EpAnn NameAnn -type SrcSpanAnnBF = EpAnn AnnBooleanFormula - -- | General representation of a 'GenLocated' type carrying a -- parameterised annotation type. type LocatedAn an = GenLocated (EpAnn an) @@ -551,17 +543,6 @@ data AnnParen | AnnParensHash (EpToken "(#") (EpToken "#)") -- ^ '(#', '#)' deriving Data --- --------------------------------------------------------------------- --- | Exact print annotation for the 'BooleanFormula' data type. - -data AnnBooleanFormula - = AnnBooleanFormula { - abf_open :: (EpToken "("), -- ^ opening parenthesis. - abf_close :: (EpToken ")"), -- ^ closing parenthesis. - abf_trailing :: ![TrailingAnn] -- ^ items appearing after the - -- item, such as '|', ',' - } deriving (Data,Eq) - -- --------------------------------------------------------------------- -- Annotations for names -- --------------------------------------------------------------------- @@ -669,14 +650,6 @@ data AnnPragSCC -- --------------------------------------------------------------------- -addTrailingAnnToBF :: TrailingAnn -> EpAnnComments - -> EpAnn AnnBooleanFormula -> EpAnn AnnBooleanFormula -addTrailingAnnToBF t cs n = n { anns = addTrailing (anns n) - , comments = comments n <> cs } - where - -- See Note [list append in addTrailing*] - addTrailing n = n { abf_trailing = abf_trailing n ++ [t]} - -- | Helper function used in the parser to add a 'TrailingAnn' items -- to an existing annotation. addTrailingAnnToA :: TrailingAnn -> EpAnnComments @@ -1030,9 +1003,6 @@ instance (NoAnn ann) => NoAnn (EpAnn ann) where instance NoAnn NoEpAnns where noAnn = NoEpAnns -instance NoAnn AnnBooleanFormula where - noAnn = AnnBooleanFormula noAnn noAnn [] - instance NoAnn AnnList where noAnn = AnnList Nothing ListNone noAnn ===================================== compiler/GHC/Tc/Solver/InertSet.hs ===================================== @@ -460,7 +460,7 @@ In implementation terms - It is only called when applying an instance decl, in GHC.Tc.Solver.Dict.tryInstances - - ClsInst.InstanceWhat says what kind of instance was + - GHC.Tc.Instance.Class.InstanceWhat says what kind of instance was used to solve the constraint. In particular * LocalInstance identifies quantified constraints * BuiltinEqInstance identifies the strange built-in @@ -800,7 +800,7 @@ The InertCans represents a collection of constraints with the following properti eg a wanted cannot rewrite a given) * CEqCan equalities: see Note [inert_eqs: the inert equalities] - Also see documentation in Constraint.Ct for a list of invariants + Also see documentation in GHC.Tc.Types.Constraint.Ct for a list of invariants Note [inert_eqs: the inert equalities] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ===================================== compiler/GHC/Types/Id/Make.hs ===================================== @@ -987,7 +987,7 @@ until the final simplifier phase; see Note [Activation for data constructor wrappers]. For further reading, see: - * (IA1) in Note [Interesting arguments] in GHC.Core.Op.Simplify.Utils + * (IA1) in Note [Interesting arguments] in GHC.Core.Opt.Simplify.Utils * Note [Lone variables] in GHC.Core.Unfold * Note [exprIsConApp_maybe on data constructors with wrappers] in GHC.Core.SimpleOpt ===================================== compiler/GHC/Types/Var.hs ===================================== @@ -621,27 +621,27 @@ Note [Types for coercions, predicates, and evidence] VarBndr is polymorphic in both var and visibility fields. Currently there are nine different uses of 'VarBndr': -* Var.ForAllTyBinder = VarBndr TyCoVar ForAllTyFlag +* GHC.Types.Var.ForAllTyBinder = VarBndr TyCoVar ForAllTyFlag Binder of a forall-type; see ForAllTy in GHC.Core.TyCo.Rep -* Var.TyVarBinder = VarBndr TyVar ForAllTyFlag +* GHC.Types.Var.TyVarBinder = VarBndr TyVar ForAllTyFlag Subset of ForAllTyBinder when we are sure the binder is a TyVar -* Var.InvisTVBinder = VarBndr TyVar Specificity +* GHC.Types.Var.InvisTVBinder = VarBndr TyVar Specificity Specialised form of TyVarBinder, when ForAllTyFlag = Invisible s See GHC.Core.Type.splitForAllInvisTVBinders -* Var.ReqTVBinder = VarBndr TyVar () +* GHC.Types.Var.ReqTVBinder = VarBndr TyVar () Specialised form of TyVarBinder, when ForAllTyFlag = Required See GHC.Core.Type.splitForAllReqTVBinders This one is barely used -* TyCon.TyConBinder = VarBndr TyVar TyConBndrVis +* GHC.Core.TyCon.TyConBinder = VarBndr TyVar TyConBndrVis Binders of a TyCon; see TyCon in GHC.Core.TyCon -* IfaceType.IfaceForAllBndr = VarBndr IfaceBndr ForAllTyFlag -* IfaceType.IfaceForAllSpecBndr = VarBndr IfaceBndr Specificity -* IfaceType.IfaceTyConBinder = VarBndr IfaceBndr TyConBndrVis +* GHC.Iface.Type.IfaceForAllBndr = VarBndr IfaceBndr ForAllTyFlag +* GHC.Iface.Type.IfaceForAllSpecBndr = VarBndr IfaceBndr Specificity +* GHC.Iface.Type.IfaceTyConBinder = VarBndr IfaceBndr TyConBndrVis -} data VarBndr var argf = Bndr var argf ===================================== testsuite/driver/perf_notes.py ===================================== @@ -83,9 +83,13 @@ PerfStat = NamedTuple('PerfStat', [('test_env', TestEnv), ('metric', MetricName), ('value', float)]) -# A baseline recovered form stored metrics. -Baseline = NamedTuple('Baseline', [('perfStat', PerfStat), - ('commit', GitHash)]) +# A test's metric recovered from a commit's git note: the raw sample values +# recorded there, and a PerfStat whose value is their mean. Serves as the +# baseline when comparing a test run against an earlier commit. +class CommitMetric(NamedTuple): + perfStat: PerfStat + commit: GitHash + samples: List[float] # The type of exceptions which are thrown when computing the current stat value # fails. @@ -460,10 +464,10 @@ def get_allowed_changes(baseline_ref: Optional[GitRef]) -> Dict[TestName, List[A else: return get_allowed_perf_changes() -# Cache of baseline values. This is a dict of dicts indexed on: -# (useCiNamespace, commit) -> (test_env, test, metric, way) -> baseline -# (bool , str ) -> (str , str , str , str) -> float -_commit_metric_cache = {} # type: ignore +# Cache of commit metrics. +_commit_metric_cache: Dict[Tuple[NoteNamespace, GitHash], + Dict[Tuple[TestEnv, TestName, MetricName, WayName], + CommitMetric]] = {} # Get the baseline of a test at a given commit. This is the expected value # *before* the commit is applied (i.e. on the parent commit). @@ -477,7 +481,7 @@ _commit_metric_cache = {} # type: ignore # instead when looking for ci results) # metric: str - test metric # way: str - test way -# returns: the Baseline or None if no metric was found within +# returns: the baseline CommitMetric or None if no metric was found within # BaselineSearchDepth commits and since the last expected change # (ignoring any expected change in the given commit). def baseline_metric(commit: GitHash, @@ -486,7 +490,7 @@ def baseline_metric(commit: GitHash, metric: MetricName, way: WayName, baseline_ref: Optional[GitRef] - ) -> Optional[Baseline]: + ) -> Optional[CommitMetric]: # For performance reasons (in order to avoid calling commit_hash), we assert # commit is already a commit hash. assert is_commit_hash(commit) @@ -502,20 +506,16 @@ def baseline_metric(commit: GitHash, # Searches through previous commits trying local then ci for each commit in. def find_baseline(namespace: NoteNamespace, test_env: TestEnv - ) -> Optional[Baseline]: + ) -> Optional[CommitMetric]: if baseline_commit is not None: - current_metric = get_commit_metric(namespace, baseline_commit, test_env, name, metric, way) - if current_metric is not None: - return Baseline(current_metric, baseline_commit) - else: - return None + return get_commit_metric(namespace, baseline_commit, test_env, name, metric, way) for depth, current_commit in list(enumerate(commit_hashes)): if current_commit == commit: continue # Check for a metric on this commit. current_metric = get_commit_metric(namespace, current_commit, test_env, name, metric, way) if current_metric is not None: - return Baseline(current_metric, current_commit) + return current_metric # Stop if there is an expected change at this commit. In that case # metrics on ancestor commits will not be a valid baseline. @@ -527,7 +527,7 @@ def baseline_metric(commit: GitHash, # Test environment to use when comparing against CI namespace ci_test_env = best_fit_ci_test_env() - baseline = find_baseline(LocalNamespace, test_env) # type: Optional[Baseline] + baseline = find_baseline(LocalNamespace, test_env) # type: Optional[CommitMetric] if baseline is None and ci_test_env is not None: baseline = find_baseline(CiNamespace, ci_test_env) @@ -545,23 +545,23 @@ def get_commit_metric_value_str_or_none(gitNoteRef, result = get_commit_metric(gitNoteRef, commit, test_env, name, metric, way) if result is None: return None - return str(result.value) + return str(result.perfStat.value) -# gets the average commit metric from git notes. +# gets the commit metric (average and raw samples) from git notes. # gitNoteRef: git notes ref space e.g. "perf" or "ci/perf" # ref: git commit # test_env: test environment # name: test name # metric: test metric # way: test way -# returns: PerfStat | None if stats don't exist for the given input +# returns: CommitMetric | None if stats don't exist for the given input def get_commit_metric(gitNoteRef, ref: Union[GitRef, GitHash], test_env: TestEnv, name: TestName, metric: MetricName, way: WayName - ) -> Optional[PerfStat]: + ) -> Optional[CommitMetric]: global _commit_metric_cache assert test_env != None commit = commit_hash(ref) @@ -573,9 +573,9 @@ def get_commit_metric(gitNoteRef, return _commit_metric_cache[cacheKeyA].get(cacheKeyB) # Cache miss. - # Calculate baselines from the current commit's git note. + # Calculate metrics from the current commit's git note. # Note that the git note may contain data for other tests. All tests' - # baselines will be collected and cached for future use. + # metrics will be collected and cached for future use. allCommitMetrics = get_perf_stats(ref, gitNoteRef) # Collect recorded values by cacheKeyB. @@ -586,22 +586,32 @@ def get_commit_metric(gitNoteRef, currentValues = values_by_cache_key_b.setdefault(currentCacheKey, []) currentValues.append(float(perfStat.value)) - # Calculate and baseline (average of values) by cacheKeyB. - baseline_by_cache_key_b = {} + # Calculate the metric (average of values, plus the values themselves) + # by cacheKeyB. + metric_by_cache_key_b = {} for currentCacheKey, currentValues in values_by_cache_key_b.items(): - baseline_by_cache_key_b[currentCacheKey] = PerfStat( \ - currentCacheKey[0], - currentCacheKey[1], - currentCacheKey[3], - currentCacheKey[2], - sum(currentValues) / len(currentValues)) - - # Save baselines to the cache. - _commit_metric_cache[cacheKeyA] = baseline_by_cache_key_b - return baseline_by_cache_key_b.get(cacheKeyB) + metric_by_cache_key_b[currentCacheKey] = CommitMetric( + PerfStat( + currentCacheKey[0], + currentCacheKey[1], + currentCacheKey[3], + currentCacheKey[2], + sum(currentValues) / len(currentValues)), + commit, + currentValues) + + # Save metrics to the cache. + _commit_metric_cache[cacheKeyA] = metric_by_cache_key_b + return metric_by_cache_key_b.get(cacheKeyB) + +def format_sample(s: float) -> str: + return str(int(s)) if s == int(s) else str(s) + +def format_samples(samples: List[float]) -> str: + return ', '.join(format_sample(s) for s in samples) def check_stats_change(actual: PerfStat, - baseline: Baseline, + baseline: CommitMetric, acceptance_window: MetricAcceptanceWindow, allowed_perf_changes: Dict[TestName, List[AllowedPerfChange]] = {}, force_print = False @@ -611,8 +621,8 @@ def check_stats_change(actual: PerfStat, Parameters: actual: the PerfStat with actual value - baseline: the expected Baseline value (this should generally be derived - from baseline_metric()) + baseline: the CommitMetric to compare against (this should generally be + derived from baseline_metric()) acceptance_window: allowed deviation of the actual value from the expected value. allowed_perf_changes: allowed changes in stats. This is a dictionary as @@ -654,9 +664,17 @@ def check_stats_change(actual: PerfStat, ' baseline @ %s' % baseline.commit print(actual.metric, error + ':') dev = 100.0 if expected_val == 0 else round(((float(actual.value) * 100) / int(expected_val)) - 100, 1) + # Show the sample spread so unreliable baselines become visible (#27602). + if len(baseline.samples) > 1: + samples_note = ('; baseline is mean of %d samples spanning %s..%s' + % (len(baseline.samples), + format_sample(min(baseline.samples)), + format_sample(max(baseline.samples)))) + else: + samples_note = '' change_line = (f'{actual.metric} {change.value} from {baseline.perfStat.test_env} ' f'baseline @ {baseline.commit[:7]}: {expected_val} -> {actual.value} ' - f'({dev:+g}%, allowed {acceptance_window.describe()})') + f'({dev:+g}%, allowed {acceptance_window.describe()}{samples_note})') result = failBecause('stat ' + change_line, tag='stat') if not change_allowed or force_print: @@ -666,6 +684,10 @@ def check_stats_change(actual: PerfStat, print(descr, str(val).rjust(length), extra) display(' Expected ' + full_name + ' ' + actual.metric + ':', expected_val, acceptance_window.describe()) + if len(baseline.samples) > 1: + display(' Samples ' + full_name + ' ' + actual.metric + ':', + len(baseline.samples), + '(' + format_samples(baseline.samples) + ')') display(' Lower bound ' + full_name + ' ' + actual.metric + ':', lowerBound, '') display(' Upper bound ' + full_name + ' ' + actual.metric + ':', upperBound, '') display(' Actual ' + full_name + ' ' + actual.metric + ':', actual.value, '') @@ -866,7 +888,7 @@ def main() -> None: # HEAD~2 21234 21234 # HEAD~3 20000 20000 def strMetric(x): - return '{:.2f}'.format(x.value) if x != None else "" + return '{:.2f}'.format(x.perfStat.value) if x != None else "" # Data is in column major format, so transpose and pass to print_table. T = TypeVar('T') def transpose(xss: List[List[T]]) -> List[List[T]]: ===================================== testsuite/driver/testglobals.py ===================================== @@ -4,7 +4,7 @@ from my_typing import * from pathlib import Path -from perf_notes import MetricChange, PerfStat, Baseline, GitRef +from perf_notes import MetricChange, PerfStat, CommitMetric, GitRef from datetime import datetime # ----------------------------------------------------------------------------- @@ -312,7 +312,7 @@ class TestResult: PerfMetric = NamedTuple('PerfMetric', [('change', MetricChange), ('stat', PerfStat), - ('baseline', Optional[Baseline]) ]) + ('baseline', Optional[CommitMetric]) ]) class TestRun: def __init__(self) -> None: ===================================== utils/check-exact/ExactPrint.hs ===================================== @@ -353,10 +353,6 @@ instance HasTrailing (EpToken "{", EpToken "}") where trailing _ = [] setTrailing a _ = a -instance HasTrailing (AnnBooleanFormula) where - trailing bf = abf_trailing bf - setTrailing a ts = a { abf_trailing = ts } - -- --------------------------------------------------------------------- fromAnn' :: (HasEntry a) => a -> Entry @@ -2731,9 +2727,11 @@ instance ExactPrint (BF.BooleanFormula GhcPs) where exact (BF.And e ls) = do ls' <- mapM markAnnotated ls return (BF.And e ls') - exact (BF.Parens e x) = do + exact (BF.Parens (o,c) x) = do + o' <- markEpToken o x' <- markAnnotated x - return (BF.Parens e x') + c' <- markEpToken c + return (BF.Parens (o',c') x') -- --------------------------------------------------------------------- @@ -4474,17 +4472,6 @@ instance ExactPrint [LocatedA (StmtLR GhcPs GhcPs (LocatedA (HsCmd GhcPs)))] whe stmts' <- markAnnotated stmts return stmts' -instance ExactPrint (LocatedBF (BF.BooleanFormula GhcPs)) where - getAnnotationEntry = entryFromLocatedA - setAnnotationAnchor = setAnchorAn - exact (L an bf) = do - debugM $ "LocatedCB [LBooleanFormula" - let (AnnBooleanFormula op cp ta) = anns an - op' <- markEpToken op - bf' <- markAnnotated bf - cp' <- markEpToken cp - return (L (an {anns = AnnBooleanFormula op' cp' ta}) bf') - instance ExactPrint [Located HsDocStringChunk] where getAnnotationEntry _ = NoEntryVal setAnnotationAnchor a _ _ _ = a ===================================== utils/haddock/haddock-api/src/Haddock/Types.hs ===================================== @@ -834,7 +834,7 @@ type instance Anno (HsDecl DocNameI) = SrcSpanAnnA type instance Anno (FamilyResultSig DocNameI) = EpAnn NoEpAnns type instance Anno (HsOuterTyVarBndrs Specificity DocNameI) = SrcSpanAnnA type instance Anno (HsSigType DocNameI) = SrcSpanAnnA -type instance Anno (BooleanFormula DocNameI) = SrcSpanAnnBF +type instance Anno (BooleanFormula DocNameI) = SrcSpanAnnA type instance Anno (OverlapMode DocNameI) = SrcSpanAnnA type instance Anno (CType DocNameI) = SrcSpanAnnA type instance Anno (Header DocNameI) = SrcSpanAnnA @@ -1041,7 +1041,7 @@ type instance XXHsContextDetails DocNameI = DataConCantHappen type instance XBFVar DocNameI = NoExtField type instance XBFAnd DocNameI = NoExtField type instance XBFOr DocNameI = NoExtField -type instance XBFParens DocNameI = NoExtField +type instance XBFParens DocNameI = (EpToken "(", EpToken ")") type instance XXBooleanFormula DocNameI = DataConCantHappen ----------------------------------------------------------------------------- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/bb344b5fc94b4472864f5013db8d106... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/bb344b5fc94b4472864f5013db8d106... You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications: https://gitlab.haskell.org/-/profile/notifications | Help: https://gitlab.haskell.org/help