Speed up changesUponFlattening#100
Speed up changesUponFlattening#100sjakobi wants to merge 1 commit intohaskell-prettyprinter:masterfrom
Conversation
0ee3916 to
fb35079
Compare
I believe the property should be Does this look right @quchen? I could add a property test for this. |
|
Ping @quchen. This could help alleviate some performance issues in |
| FlatAlt _ y -> Just (flatten y) | ||
| Line -> Just Fail | ||
| Union x _ -> changesUponFlattening x <|> Just x | ||
| Union x _ -> Just x |
There was a problem hiding this comment.
Another idea:
| Union x _ -> Just x | |
| Union x _ -> Nothing |
There was a problem hiding this comment.
Ah, no. This causes the fusion tests to fail with the classic
»SFail« must not appear in a rendered »SimpleDocStream«.
I'm not quite sure why though. Proper shrinking and diagnostic output would help… :/
I have written a test for this on top of #96 at master...sjakobi:99-group-tests. At least for the |
Since the first Union alternative was produced by the previous call to to changesUponFlattening, it seems unnecessary to scrutinize it again. This speeds up some prettyprinter-heavy applications in dhall by 20 to 42%. This assumes some kind of idempotence in changesUponFlattening. Fixes haskell-prettyprinter#99.
fb35079 to
55469a6
Compare
|
While looking at some > diag $ group . group . group $ hardline
Union Fail (Union Fail (Union Fail Line))In the layouter, this will still be slow since any overly wide This patch would fix that: @@ -523,6 +523,7 @@ hardline = Line
-- use of it.
group :: Doc ann -> Doc ann
-- See note [Group: special flattening]
+group x@Union{} = x
group x = case changesUponFlattening x of
Nothing -> x
Just x' -> Union x' xAnd maybe we can even do something smarter and look whether there's anything flattenable before we reach a first |
|
Superseded by #116. |
Since the first Union alternative was produced by the previous
call to to changesUponFlattening, it seems unnecessary to
scrutinize it again.
This speeds up some prettyprinter-heavy applications in dhall
by 20 to 42%.
This assumes some kind of idempotence in changesUponFlattening.
Fixes #99.