prettyprinter icon indicating copy to clipboard operation
prettyprinter copied to clipboard

Published 20 hours ago • verified publisher icon quchen

Optimization potential

Open sjakobi opened this issue 5 years ago • 3 comments

In https://github.com/quchen/prettyprinter/pull/116#issuecomment-577204724 I shared a profile from a prettyprinter-heavy dhall task. Multiple functions from prettyprinter and prettyprinter-ansi-terminal show up:

COST CENTRE               MODULE                                             SRC                                                                         %time %alloc
layoutWadlerLeijen        Data.Text.Prettyprint.Doc.Internal                 src/Data/Text/Prettyprint/Doc/Internal.hs:(1775,1)-(1843,60)                  5.5    3.0
renderLazy                Data.Text.Prettyprint.Doc.Render.Terminal.Internal src/Data/Text/Prettyprint/Doc/Render/Terminal/Internal.hs:(108,1)-(148,60)    3.7    3.7
removeTrailingWhitespace  Data.Text.Prettyprint.Doc.Internal                 src/Data/Text/Prettyprint/Doc/Internal.hs:(1477,1)-(1537,119)                 2.3    1.1
styleToRawText            Data.Text.Prettyprint.Doc.Render.Terminal.Internal src/Data/Text/Prettyprint/Doc/Render/Terminal/Internal.hs:(278,1)-(304,29)    1.8    2.0
layoutSmart               Data.Text.Prettyprint.Doc.Internal                 src/Data/Text/Prettyprint/Doc/Internal.hs:(1741,1)-(1767,59)                  1.6    0.2
changesUponFlattening     Data.Text.Prettyprint.Doc.Internal                 src/Data/Text/Prettyprint/Doc/Internal.hs:(567,1)-(607,21)                    1.5    1.1

The most interesting candidates are IMHO removeTrailingWhitespace and styleToRawText:

https://github.com/quchen/prettyprinter/blob/fa8ed27217af75b1515cf1ef0b0c0be33dc2f5b0/prettyprinter/src/Data/Text/Prettyprint/Doc/Internal.hs#L1458-L1519

https://github.com/quchen/prettyprinter/blob/fa8ed27217af75b1515cf1ef0b0c0be33dc2f5b0/prettyprinter-ansi-terminal/src/Data/Text/Prettyprint/Doc/Render/Terminal/Internal.hs#L291-L318

sjakobi avatar Jan 22 '20 14:01 sjakobi

A few remarks regarding styleToRawText:

  • styleToRawText is only used internally by renderLazy and renderIO:

    https://github.com/quchen/prettyprinter/blob/fa8ed27217af75b1515cf1ef0b0c0be33dc2f5b0/prettyprinter-ansi-terminal/src/Data/Text/Prettyprint/Doc/Render/Terminal/Internal.hs#L147-L157 https://github.com/quchen/prettyprinter/blob/fa8ed27217af75b1515cf1ef0b0c0be33dc2f5b0/prettyprinter-ansi-terminal/src/Data/Text/Prettyprint/Doc/Render/Terminal/Internal.hs#L209-L219

  • Typical documents probably use only a very limited number of AnsiStyles. It might make sense to cache the corresponding SGRs or even Texts.

  • renderIO could skip translation to Text by using hSetSGR.

sjakobi avatar Jan 22 '20 15:01 sjakobi

Regarding what's happening inside styleToRawText, I wonder why there are convertIntensity and convertColor:

https://github.com/quchen/prettyprinter/blob/fa8ed27217af75b1515cf1ef0b0c0be33dc2f5b0/prettyprinter-ansi-terminal/src/Data/Text/Prettyprint/Doc/Render/Terminal/Internal.hs#L304-L318

These convert to the ansi-terminal types with the very same constructors:

Why does prettyprinter-ansi-terminal define its own versions of these at all, when it could simply re-export those from ansi-terminal?

https://github.com/quchen/prettyprinter/blob/fa8ed27217af75b1515cf1ef0b0c0be33dc2f5b0/prettyprinter-ansi-terminal/src/Data/Text/Prettyprint/Doc/Render/Terminal/Internal.hs#L56-L62

sjakobi avatar Jan 22 '20 15:01 sjakobi

Regarding styleToRawText, maybe we could take some ideas from basement's ANSI color support.

sjakobi avatar Jan 27 '20 12:01 sjakobi