Hadrian: merge sections in profiling _p.a to .p_o for ghci
AbandonedPublic

Authored by watashi on Oct 27 2018, 1:32 AM.

Details

Summary

This is the hadrain version of D5169: Merge sections in profiling .a to .p_o and use it whenever it exists

  • We build squashed .o and .p_o for ghci when dynamicGhcPrograms is False
  • We no longer build them for rts as ghci never loads it

we need https://github.com/haskell/cabal/pull/5592 for cabal to copy the built .p_o file.

Test Plan
$ grep dynamicGhc hadrian/UserSettings.hs
  , dynamicGhcPrograms = return False
$ touch ...
$ hadrian/build.sh --flavour=user -j --digest-or
$ find _build/stage1/libraries/ -name 'HS*-*.*o' | wc
     62      62    3664
$ grep -C3 dynamicGhc hadrian/UserSettings.hs
userFlavour :: Flavour
userFlavour = performanceFlavour
  { name = "user"
  , dynamicGhcPrograms = return False
  }
$ hadrian/build.sh -j --flavour=user test --verbose
Unexpected results from:
TEST="T3807 T9208 T9293 annth_make ghci057 haddock.Cabal haddock.base haddock.compiler"

SUMMARY for test run started at Wed Dec  5 17:45:39 2018 PST
 0:03:16 spent to go through
    6708 total tests, which gave rise to
   26015 test cases, of which
   19290 were skipped

      29 had missing libraries
    6600 expected passes
      88 expected failures

       3 caused framework failures
       0 caused framework warnings
       1 unexpected passes
       7 unexpected failures
       0 unexpected stat failures
$ find _build -name 'HSbase*.*o'
_build/stage1/lib/x86_64-linux-ghc-8.7.20181204/base-4.12.0.0/HSbase-4.12.0.0.o
_build/stage1/lib/x86_64-linux-ghc-8.7.20181204/base-4.12.0.0/HSbase-4.12.0.0.p_o
_build/stage1/libraries/base/build/HSbase-4.12.0.0.o
_build/stage1/libraries/base/build/HSbase-4.12.0.0.p_o
watashi created this revision.Oct 27 2018, 1:32 AM
watashi retitled this revision from Hadrain: merge sections in profiling _p.a to .p_o to Hadrain: merge sections in profiling _p.a to .p_o for ghci.Oct 27 2018, 1:33 AM
watashi updated the Trac tickets for this revision.
watashi added inline comments.Oct 27 2018, 1:39 AM
hadrian/src/Settings/Builders/Cabal.hs
73–77

the similar change in cabal is https://github.com/haskell/cabal/pull/5592

snowleopard accepted this revision.Oct 27 2018, 7:02 AM
snowleopard added a subscriber: alpmestan.

@watashi Thank you! Generally this looks good, but if possible I would like to avoid baking in the special case package /= rts (see my comment). Can this be specified in rts.cabal.in instead?

hadrian/src/Rules/Library.hs
228

These changes look good to me, but pinging @alpmestan just in case, since he wrote the library path parsers.

hadrian/src/Utilities.hs
67

Should the package /= rts special case be somehow specified in the rts.cabal.in file instead of baking it into Hadrian? We set buildGhciLib flag for each package by parsing its Cabal file -- the then branch below does use it to decide whether to build GHCi library.

Here is the where we obtain this flag from Cabal:

https://github.com/ghc/ghc/blob/master/hadrian/src/Hadrian/Haskell/Cabal/Parse.hs#L283

This revision is now accepted and ready to land.Oct 27 2018, 7:02 AM
alpmestan added inline comments.Oct 27 2018, 2:50 PM
hadrian/src/Rules/Library.hs
228

I think a complete build with the default or perf flavours are good enough to prove everything's fine, as IIRC they involve building ghci libs. Even better would be to do hadrian/build.sh -j<N> --flavour=perf test --verbose and check that the numbe of failures is reasonable (should be around 31 IIRC).

watashi edited the test plan for this revision. (Show Details)Oct 28 2018, 1:33 AM
watashi edited the summary of this revision. (Show Details)
watashi edited the test plan for this revision. (Show Details)
watashi updated this revision to Diff 18490.Oct 28 2018, 1:37 AM

move package /= rts to cabal configure step
this also fix failure at cabal copy step due to missing HSrts-*.o

watashi marked 2 inline comments as done.Oct 28 2018, 1:38 AM
bgamari requested changes to this revision.Nov 1 2018, 5:27 PM
bgamari retitled this revision from Hadrain: merge sections in profiling _p.a to .p_o for ghci to Hadrian: merge sections in profiling _p.a to .p_o for ghci.

Bumping out of merge queue until the Cabal patch makes it in. @watashi, can you ping me when that happens?

This revision now requires changes to proceed.Nov 1 2018, 5:27 PM

The cabal changes has been pushed to master and 2.4 branches on github: https://github.com/haskell/cabal/commits/2.4
But the 2.4 branch on haskell.org is still 2 weeks old: http://git.haskell.org/packages/Cabal.git/log/refs/heads/2.4
Is only master branch automatically synced?

There's a diff that bumps the Cabal submodule off the 2.4 branch, to track master: https://phabricator.haskell.org/D5329

It will probably be easier to wait for this to land, as it did "all the hard work".

watashi edited the test plan for this revision. (Show Details)Dec 5 2018, 11:16 PM
watashi updated this revision to Diff 19028.Dec 5 2018, 11:18 PM

rebase onto recent Cabal release
@bgamari this handles .p_o now, see updated test plan

watashi marked an inline comment as done.Dec 5 2018, 11:18 PM
watashi marked an inline comment as done.
bgamari requested changes to this revision.Dec 11 2018, 12:30 PM

Can you rebase this @watashi?

This revision now requires changes to proceed.Dec 11 2018, 12:30 PM
watashi updated this revision to Diff 19114.Dec 11 2018, 6:28 PM

rebase and rerun tests

Unexpected results from:
TEST="T3807 T9208 T9293 T9329 cgrun069 ghci057"

SUMMARY for test run started at Tue Dec 11 15:53:19 2018 PST
 0:03:01 spent to go through
    6716 total tests, which gave rise to
   26038 test cases, of which
   19311 were skipped

      29 had missing libraries
    6604 expected passes
      88 expected failures

       0 caused framework failures
       0 caused framework warnings
       3 unexpected passes
       3 unexpected failures
       0 unexpected stat failures