#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: scpmw Type: bug | Status: new Priority: high | Milestone: 7.10.1 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961 | Differential Revisions: Phab:D616 | Phab:D636 -------------------------------------+------------------------------------- Comment (by Simon Marlow ): In [changeset:"daed18c35cda114d8a5303bcb645195e1fd397e3/ghc"]: {{{ #!CommitTicketReference repository="ghc" revision="daed18c35cda114d8a5303bcb645195e1fd397e3" Fix a profiling bug Summary: We were erroneously discarding SCCs on function-typed variables. These can affect the call stack, so we have to retain them. The bug was introduced during the recent SourceNote refactoring. This is an alternative to the fix proposed in D616. I also added the scc005 test from that diff, which works with this change. While I was here, I also fixed up the other profiling tests, marking a few as expect_broken_for(10037) where the opt/unopt output differs in non-fatal ways. Test Plan: profiling tests Reviewers: scpmw, ezyang, austin Subscribers: thomie Differential Revision: https://phabricator.haskell.org/D636 GHC Trac Issues: #10007 }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: scpmw Type: bug | Status: merge Priority: high | Milestone: 7.10.1 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961 | Differential Revisions: Phab:D616 | Phab:D636 -------------------------------------+------------------------------------- Comment (by scpmw): Well, it's the wrong fix, as far as I'm concerned. Yes, it fixes the erroneous attribution to `lintAnnots`, but only because we change the program's (RTS) cost semantics twice during optimisations, with both instances un-doing each other. This is not guaranteed, see the new test case for `D616`. I also didn't feel it would help the discussion to mention this - but this also doesn't fix rather silly double-annotations on parameters, which (I think) only don't appear because we automatically eliminate repeating cost centres in stacks. If needed, I could produce another test case that demonstrates that. Short version: This is only a band-aid. And as far as those go, I would still prefer `D616`. At least until I have figured out how to *actually* implement `Eval2`, which would fix the whole situation from the RTS end. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: scpmw Type: bug | Status: closed Priority: high | Milestone: 7.10.1 Component: Profiling | Version: 7.10.1-rc1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961 | Differential Revisions: Phab:D616 | Phab:D636 -------------------------------------+------------------------------------- Changes (by thoughtpolice): * status: merge => closed * resolution: => fixed Comment: Merged to `ghc-7.10` (via 1d401b4384b5f9c7429140320e1d0bc120172b8b). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: Type: bug | Status: new Priority: high | Milestone: 7.10.1 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961 | Differential Revisions: Phab:D616 | Phab:D636 -------------------------------------+------------------------------------- Comment (by scpmw): Yes - I investigated this again a month ago, and found that SM's fix didn't correct the whole problem. In fact, if I remember correctly what we end up with is something like {{{ case scctick<SpecConstr> specConstrProgram of ... }}} In this situation the tick will fail to actually capture any cost in `specConstrProgram`, which will instead end up in the cost-centre next in the hierarchy, which happens to be `lintAnnots` (`Core2Core` before). As explained in D616 and #5654, this is a fairly fundamental problem: GHC simply doesn't implement the cost-centre semantics for function values correctly. Correcting this isn't exactly easy - I have been experimenting in the past weeks, and the best solution I can come up with (still?) involves a new closure type that wraps function values when they escape cost-centre scopes. Two possible solutions here: We could attempt another stopgap measure. For example, the above code should probably in-line as it is only used once. Building the full solution will still need a bit more tinkering, but I might be able to make a proposal soon-ish (... just need to get it to stop crashing. Seems getting register saving right can be surprisingly tricky). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: scpmw Type: bug | Status: new Priority: high | Milestone: 7.10.3 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961 | Differential Revisions: Phab:D616 | Phab:D636 -------------------------------------+------------------------------------- Comment (by simonpj): Try the effect of changing (in `SimplCore`): {{{ ; guts' <- lintAnnots (ppr pass) (doCorePass pass) guts }}} to {{{ ; guts' <- lintAnnots (ppr pass) (\p -> doCorePass pass p) guts }}} Simon M speculates that this might work around the problem. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: scpmw Type: bug | Status: new Priority: high | Milestone: 7.10.3 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961 | Differential Revisions: Phab:D616 | Phab:D636 -------------------------------------+------------------------------------- Comment (by bgamari): scpmw, I've tested Simon's suggestion on `master` and it appears to work for me. Costs are correctly attributed to cost-centers beneath `lintAnnots`. I'll be merging this to `master`. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: scpmw Type: bug | Status: new Priority: high | Milestone: 7.10.3 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961 | Differential Revisions: Phab:D616 | Phab:D636 -------------------------------------+------------------------------------- Comment (by bgamari): Hmm, actually it seems that `master` is now doing the right thing even without my patch. I am quite confused. I'll return to this after the release. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: scpmw Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961,#5654 | Differential Rev(s): Phab:D616 Wiki Page: | Phab:D636 -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 7.10.3 => 8.2.1 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: scpmw Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961,#5654 | Differential Rev(s): Phab:D616 Wiki Page: | Phab:D636 -------------------------------------+------------------------------------- Changes (by simonmar): * milestone: 8.4.1 => 8.2.1 Comment: As it happens, I was just working on this! -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:18 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#10007: Fix misattribution of Cost Centre profiles to lintAnnots -------------------------------------+------------------------------------- Reporter: thoughtpolice | Owner: simonmar Type: bug | Status: new Priority: high | Milestone: 8.2.1 Component: Profiling | Version: 7.10.1-rc1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #9961,#5654 | Differential Rev(s): Phab:D616 Wiki Page: | Phab:D636 -------------------------------------+------------------------------------- Comment (by Ben Gamari ): In [changeset:"394231b301efb6b56654b0a480ab794fe3b7e4db/ghc" 394231b3/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="394231b301efb6b56654b0a480ab794fe3b7e4db" Fix cost-centre-stacks bug (#5654) This fixes some cases of wrong stacks being generated by the profiler. For background and details on the fix see `Note [Evaluating functions with profiling]` in `rts/Apply.cmm`. This does have an impact on allocations for some programs when profiling. nofib results: ``` k-nucleotide +0.0% +8.8% +11.0% +11.0% 0.0% puzzle +0.0% +12.5% 0.244 0.246 0.0% typecheck 0.0% +8.7% +16.1% +16.2% 0.0% ------------------------------------------------------------------------ -------- Min -0.0% -0.0% -34.4% -35.5% -25.0% Max +0.0% +12.5% +48.9% +49.4% +10.6% Geometric Mean +0.0% +0.6% +2.0% +1.8% -0.3% ``` But runtimes don't seem to be affected much, and the examples I looked at were completely legitimate. For example, in puzzle we have this: ``` position :: ItemType -> StateType -> BankType position Bono = bonoPos position Edge = edgePos position Larry = larryPos position Adam = adamPos ``` where the identifiers on the rhs are all record selectors. Previously the profiler gave a stack that looked like ``` position bonoPos ... ``` i.e. `bonoPos` was at the same level of the call stack as `position`, but now it looks like ``` position bonoPos ... ``` I used the normaliser from the testsuite to diff the profiling output from other nofib programs and they all looked better. Test Plan: * the broken test passes * validate * compiled and ran all of nofib, measured perf, diff'd several .prof files Reviewers: niteria, erikd, austin, scpmw, bgamari Reviewed By: bgamari Subscribers: thomie Differential Revision: https://phabricator.haskell.org/D2804 GHC Trac Issues: #5654, #10007 }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10007#comment:20 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler