quchen / prettyprinter

A modern, extensible and well-documented prettyprinter.
BSD 2-Clause "Simplified" License
293 stars 34 forks source link

Is it possible to cut on expensive computations when the layout is unbounded? #215

Open Ptival opened 2 years ago

Ptival commented 2 years ago

Hello,

I have a very large document that, when printed, takes about 30 seconds if I define commas = fillSep . punctuate commas, but only takes 3 seconds if I define it as commas = hsep . puncuate commas.

Now this discrepancy is possibly acceptable, given fillSep ought to do more work in general. But even when setting the layout to Unbounded, we seem to pay the hefty price of fillSep, even though, unless I'm mistaken, we get the same output that hsep would have given.

This can be annoying if a project provides a fillSep-based printer, but lets the layout to its clients, who then pay the layout price even if they don't really need to.

I may try to give a minimized example later, but my actual project has 30MB input files and a large code base so it's not the ideal setting to minimize.

Also note that the same problem happens with the pretty package.

sjakobi commented 2 years ago

Hi @Ptival!

It does sound like prettyprinter should be able to do better here.

A reproducer would be very helpful though.

It would also be useful if you could check whether there's a regression involved. The perf regression that led to #205 was introduced in v1.7.0. Other potentially buggy layout changes were introduced since v1.5.0.

Ptival commented 2 years ago

Tested on 1.6.2 and seems to be the same.

I will try and see if a regression is not too hard to make!

Ptival commented 2 years ago

Unless I'm doing something silly, it's actually very simple to reproduce.

On my machine, the following program run in 100s:

  renderIO stdout (layoutPretty @() (LayoutOptions Unbounded) (fillSep (replicate 100000 (pretty "Hello"))))

While the following program runs in 0.05s:

  renderIO stdout (layoutPretty @() (LayoutOptions Unbounded) (hsep (replicate 100000 (pretty "Hello"))))

They produce the exact same output.