Skip to content
This repository was archived by the owner on Aug 3, 2024. It is now read-only.

Conversation

parsonsmatt
Copy link

@parsonsmatt parsonsmatt commented Dec 9, 2022

Based on #1541

This PR refactors the WriterT logging to use the CPS version. The MonadWriter interface is scrapped in favor of a custom class for reporting errors. ErrMsg is now Builder instead of String.

Overall this is a pretty decent improvement in performance. Testing this on the persistent repository and persistent-test in particular (since that package has a ton of exports and not a lot of docs). You have to remember to do the step twice, because it'll build persistent too on the first time due to configuration changing.

plain

With cabal haddock persistent-test --haddock-options "+RTS -s -RTS":

  41,908,794,344 bytes allocated in the heap
   6,099,171,264 bytes copied during GC
     305,718,464 bytes maximum residency (20 sample(s))
       7,396,160 bytes maximum slop
             744 MiB total memory in use (0 MB lost due to fragmentation)

                                     Tot time (elapsed)  Avg pause  Max pause
  Gen  0      9993 colls,     0 par    3.845s   3.851s     0.0004s    0.0039s
  Gen  1        20 colls,     0 par    1.914s   1.915s     0.0958s    0.2327s

  TASKS: 5 (1 bound, 4 peak workers (4 total), using -N1)

  SPARKS: 0 (0 converted, 0 overflowed, 0 dud, 0 GC'd, 0 fizzled)

  INIT    time    0.001s  (  0.000s elapsed)
  MUT     time   16.285s  ( 19.616s elapsed)
  GC      time    5.759s  (  5.766s elapsed)
  EXIT    time    0.002s  (  0.008s elapsed)
  Total   time   22.046s  ( 25.390s elapsed)

  Alloc rate    2,573,483,835 bytes per MUT second

  Productivity  73.9% of total user, 77.3% of total elapsed

Documentation created:
/home/matt/Projects/persistent/dist-newstyle/build/x86_64-linux/ghc-9.4.2/persistent-test-2.13.1.3/doc/html/persistent-test/index.html
cabal haddock persistent-test --haddock-options "+RTS -s -RTS"  24.80s user 1.27s system 100% cpu 25.974 total

This branch

cabal haddock persistent-test --with-haddock=/home/matt/.cabal/bin/haddock --haddock-options "+RTS -s -RTS"

  25,263,032,248 bytes allocated in the heap
   4,645,938,312 bytes copied during GC
     309,366,440 bytes maximum residency (18 sample(s))
       7,397,720 bytes maximum slop
             716 MiB total memory in use (0 MB lost due to fragmentation)

                                     Tot time (elapsed)  Avg pause  Max pause
  Gen  0      6017 colls,     0 par    2.607s   2.611s     0.0004s    0.0039s
  Gen  1        18 colls,     0 par    1.515s   1.515s     0.0842s    0.2277s

  TASKS: 5 (1 bound, 4 peak workers (4 total), using -N1)

  SPARKS: 0 (0 converted, 0 overflowed, 0 dud, 0 GC'd, 0 fizzled)

  INIT    time    0.000s  (  0.000s elapsed)
  MUT     time   11.759s  ( 13.185s elapsed)
  GC      time    4.122s  (  4.126s elapsed)
  EXIT    time    0.001s  (  0.009s elapsed)
  Total   time   15.883s  ( 17.320s elapsed)

  Alloc rate    2,148,472,781 bytes per MUT second

  Productivity  74.0% of total user, 76.1% of total elapsed

Documentation created:
/home/matt/Projects/persistent/dist-newstyle/build/x86_64-linux/ghc-9.4.2/persistent-test-2.13.1.3/doc/html/persistent-test/index.html
cabal haddock --with-haddock=/home/matt/.cabal/bin/haddock persistent-test    16.77s user 1.17s system 100% cpu 17.933 total

This change alone results in a pretty nice performance improvement.
8 seconds faster to build docs for persistent-test, down to 18 seconds from 26 seconds - 30% improvement.
Total memory use is ~4% better (not huge, but not nothing).
Allocations are way down (42GB to 25GB).

@alexbiehl
Copy link
Member

alexbiehl commented Dec 9, 2022

Nice improvement! Though I think the new dependencies will be a problem for GHCs build system as they are not boot libraries and as such GHC can’t bootstrap them.

Is it possible to use the bytestring builder and inline the dlist stuff and use the CPS writer from mtl?

@parsonsmatt
Copy link
Author

Ah, that's unfortunate! Is there a way to statically ensure that only boot packages are allowed?

I can use the Builder that's in ByteString, inline DList, and pick the later mtl which has that module - cabal was giving mysterious errors about it due to the cabal.project's hackage index state field.

@parsonsmatt
Copy link
Author

Ah, can't have a newer mtl just yet due to the GHC boot package requirement. CPPed in the instance definition.

@parsonsmatt
Copy link
Author

Noticed that this PR also did a nub => ordNub translation. Tested it out and that was responsible for an inordinate amount of the benefit. I took the liberty of removing the last few nub calls. No big difference on the persistent test.

Comment on lines +464 to 467
( ordNub
. map fst
. filter ((== Visible) . piVisibility . snd)
$ packages)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This nub call in particular is troubling - packages is probably a very large list in some cases.

Copy link
Member

@alexbiehl alexbiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice let’s go!

@alexbiehl alexbiehl merged commit 8d41a6c into haskell:ghc-9.4 Dec 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants