[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 <ben@…>): In [changeset:"85a295d5607b5f8015bb3517601ced0d1adc29ef/ghc" 85a295d/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="85a295d5607b5f8015bb3517601ced0d1adc29ef" ghc-prim: Don't allocate a thunk for each unpacked UTF-8 character While debugging #14005 I noticed that unpackCStringUtf8# was allocating a thunk for each Unicode character that it unpacked. This seems hardly worthwhile given that the thunk's closure will be at least three words, whereas the Char itself will be only two and requires only a bit of bit twiddling to construct. Test Plan: Validate Reviewers: simonmar, austin Subscribers: dfeuer, rwbarton, thomie Differential Revision: https://phabricator.haskell.org/D3769 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/14005#comment:11> 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): 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