[GHC] #14005: Explore moving from pretty to prettyprinter

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: None/Unknown Unknown/Multiple | Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: | wiki:PrettyErrors -------------------------------------+------------------------------------- GHC has for a long time used the `pretty` library for pretty-printing. However, for a number of reasons (explained in wiki:PrettyErrors) it leaves some things to be desired. The `prettyprinter` library is a performant, well-supported alternative that looks quite promising. Explore moving to `prettyprinter` -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Changes (by bgamari): * differential: => Phab:D3650 Comment: @bollu has offered to look into moving GHC to it and even has a branch (Phab:D3650) starting the process. Unfortunately, he ran into a rather tricky correctness issue: the inplace `ghc-pkg` fails during the build with, {{{ /home/ben/ghc/ghc-testing/inplace/lib/package.conf.d/package.cache: GHC.PackageDb.readPackageDb: inappropriate type (not a ghc-pkg db file, wrong file magic number) }}} It seems possible that this this due to faulty code generation due to minor differences in pretty-printer output, but it's really quite unclear. This needs further investigation. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Changes (by bgamari): * cc: bollu (added) -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bgamari): After adding some extra debug output to `GHC.PackageDb.getHeader` I see this, {{{ /home/ben/ghc/ghc-testing/inplace/lib/package.conf.d/package.cache: GHC.PackageDb.readPackageDb: inappropriate type (not a ghc-pkg db file, wrong file magic number (saw "\NULghcpkg\NUL\NUL\NUL\NUL\SOH\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\NUL\SOH\NUL", expected "\192\231hcpkg\192\128dbModuleUnitId")) }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bgamari): Interesting: so `BS.length headerMagic` apparently evaluates to 23. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bgamari): One issue is that `prettyprinter` lacks an analogue to `pretty`'s `($$)` operator, which has `empty` as an identity. See https://github.com/quchen/prettyprinter/issues/34. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bgamari): The problem appears to be the encoding of the string literal itself in the object code. Previously we would find this, {{{ starts here 0531220 - b o o t - 8 . 3 \0 300 200 g h c p 0531240 k g 300 200 \0 d b M o d u l e U n i ^ ends here }}} Now we find this, {{{ starts here 0534520 o o t - 8 . 3 \0 303 200 200 g h c p k 0534540 g 303 200 200 \0 d b M o d u l e U n i ^ ends here }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bgamari): Alright, well the issue was actually fairly obvious: We are now producing `.s` files in UTF-8 instead of ISO 8859-1. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bgamari): For the record, my [[https://github.com/bgamari/ghc/commit/5256c89d3172d353d4ebc595b8c38c7ae5a796c9|branch]] contains a rather hacky fix for this in issue (at least in the case of the NCG). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bollu): Would the same fix work for the other backend? Also, why does changing the prettyprinter cause this issue? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bgamari): I'm honestly not sure *where* in the compiler it was determined that assembler output should be ISO 8859-1. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter
-------------------------------------+-------------------------------------
Reporter: bgamari | Owner: (none)
Type: task | Status: new
Priority: normal | Milestone: 8.4.1
Component: Compiler | Version: 8.0.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
| Unknown/Multiple
Type of failure: None/Unknown | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D3650
Wiki Page: |
wiki:PrettyErrors |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by mpickering): Now that this patch builds, it is also necessary to patch haddock which uses the old interface for pretty in the latex backend. https://github.com/mpickering/haddock/tree/wip/T14005 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by mpickering): The perf results don't look very promising for this branch. https://perf.haskell.org/ghc/#compare/fefcbfa86b73517d5002366d0703ce694c6d22... -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: 8.4.1 Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by bgamari):
The perf results don't look very promising for this branch.
Perhaps, but I'm not sure whether this is really all due to `prettyprinter`. Afterall, we also now build `text` as well. Really the branch needs to be rebased before we can really say much about its compilation time impact. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Changes (by bgamari): * milestone: 8.6.1 => Comment: Indeed I've recently rebased this and found that performance is even worse now than it was previously. I'm rather doubting that this is a direction that we will want to pursue further. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:16 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler

#14005: Explore moving from pretty to prettyprinter -------------------------------------+------------------------------------- Reporter: bgamari | Owner: (none) Type: task | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.0.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D3650 Wiki Page: | wiki:PrettyErrors | -------------------------------------+------------------------------------- Comment (by simonpj): Worth adding data? Just saying "perf is worse" doesn't tell us much. Indeed, standing back a bit, it's strange that pretty-printer perf has ''any'' impact on normal compilation. What do we pretty-print? Assembly code perhaps? That can't be hard! Perhaps we could * Improve perf (perhaps a lot) by using a no-op pretty printer for the fast path * And improve modularity by using an existing library for the slow path -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/14005#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC