Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exponential runtime for layoutSmart + fillSep + sep #205

Open
brianhuffman opened this issue Sep 2, 2021 · 4 comments
Open

Exponential runtime for layoutSmart + fillSep + sep #205

brianhuffman opened this issue Sep 2, 2021 · 4 comments

Comments

@brianhuffman
Copy link

This issue is based on GaloisInc/cryptol#1274. The story is that we recently ported our cryptol language interpreter to use prettyprinter, but later we noticed a severe slowdown that occurs when we print a list of tuples of any significant length. Below is a minimized example based on our pretty-printing code.

import Prettyprinter
import Prettyprinter.Render.String

main :: IO ()
main = putStrLn (renderString (layoutSmart defaultLayoutOptions d))
  where d = fillSep (replicate 30 (sep [pretty "abc", pretty "xyz"]))

The above program takes about 7 seconds to run on my machine. The run-time is exponential in the length of the list passed to fillSep: Incrementing from 30 to 31 doubles the run-time. 33 takes about a minute.

The exponential slowdown seems to occur only with layoutSmart; layoutPretty prints this example quickly at any size.

I did not expect layoutSmart to have exponential worst case behavior, as I assumed that this would have been mentioned in the documentation if it was known to be the case.

@sjakobi
Copy link
Collaborator

sjakobi commented Sep 5, 2021 via email

@sjakobi
Copy link
Collaborator

sjakobi commented Sep 15, 2021

Indeed this appears to be a regression: Bisection leads to 9635a5d. I'll continue investigating this soon.

@sjakobi
Copy link
Collaborator

sjakobi commented Sep 16, 2021

I suspect the problem is that the x in SLine i' x is forced too eagerly here:

Line -> let x = best i i ds
-- Don't produce indentation if there's no
-- following text on the same line.
-- This prevents trailing whitespace.
i' = case x of
SEmpty -> 0
SLine{} -> 0
_ -> i
in SLine i' x

I wonder whether making SLine lazy in the first Int field might be sufficient to fix the issue…

EDIT: I should look at a Core diff.

EDIT 2: Why exactly is only layoutSmart affected?

@quchen
Copy link
Owner

quchen commented Dec 3, 2021

For solving this issue: Promising find. Making SLine non-strict sounds like a plan. I think it’s always been strict, maybe I added it in order to avoid the pathological »foldl (+) chain of + evaluations« performance pitfall, but that case does not seem to be very relevant in practice, we’re not stacking hundreds of line breaks in practice, certainly not to the extend that they would overflow. (We could also liberally+selectively seq most things that go into SLine later again).

The bigger picture: Trimming whitespace in postprocessing might not be such a bad idea after all. Whitespace trimming is the reason for multiple hacks and complications, making everything much more complicated, especially when annotations come into play.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants