-
Notifications
You must be signed in to change notification settings - Fork 42
Description
In dhall-lang/dhall-haskell#1496 (comment) Gabriel describes a case where rendering a type error takes multiple seconds.
You can reproduce the issue with the following command, after adding a type error, for example by removing the Some on this line.
$ dhall type --file stable/jenkins/index.dhall --explain
These are the top cost centers:
COST CENTRE MODULE SRC %time %alloc
getDecodeAction Codec.CBOR.Decoding src/Codec/CBOR/Decoding.hs:311:1-55 27.1 34.8
changesUponFlattening Data.Text.Prettyprint.Doc.Internal src/Data/Text/Prettyprint/Doc/Internal.hs:(553,1)-(591,21) 19.3 22.6
prettyCharacterSet Dhall.Pretty.Internal src/Dhall/Pretty/Internal.hs:(502,1)-(1139,62) 6.5 5.1
renderLazy Data.Text.Prettyprint.Doc.Render.Text src/Data/Text/Prettyprint/Doc/Render/Text.hs:59:1-92 6.5 6.1
deserialiseIncremental Codec.CBOR.Read src/Codec/CBOR/Read.hs:(165,1)-(167,46) 4.7 3.5
Note that changesUponFlattening takes nearly 20% of the time!
So I had a closer look at this function and noticed this case:
Union is introduced by group which dhall apparently uses a lot: the same profile contains over 600000 entries for group!
So I wondered: If changesUponFlattening is only used by group, and the Union constructor is also only introduced by group, why does changesUponFlattening have to scrutinize it at all? So I tried this change:
- Union x _ -> changesUponFlattening x <|> Just x
+ Union x _ -> Just xThis shaves about 18% off the total time when profiling and about 42% when using the unprofiled executable (and I'm not sure where that difference comes from)!
Unfortunately it also breaks the fusion property tests, which now fail with the classic
»SFail« must not appear in a rendered »SimpleDocStream«. This is a bug in the layout algorithm! Please report this as a bug
I haven't figured out yet quite how it breaks fuse though!