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

Redundant annotation handling in prettyprinter's renderLazy #165

Closed
sjakobi opened this issue Jun 12, 2020 · 1 comment · Fixed by #166
Closed

Redundant annotation handling in prettyprinter's renderLazy #165

sjakobi opened this issue Jun 12, 2020 · 1 comment · Fixed by #166

Comments

@sjakobi
Copy link
Collaborator

sjakobi commented Jun 12, 2020

If the annotation handling with (pure mempty) has a performance cost, we should probably try to get rid of it.

renderLazy :: SimpleDocStream ann -> TL.Text
renderLazy = TLB.toLazyText . renderSimplyDecorated TLB.fromText (pure mempty) (pure mempty)

renderSimplyDecorated
:: Monoid out
=> (Text -> out) -- ^ Render plain 'Text'
-> (ann -> out) -- ^ How to render an annotation
-> (ann -> out) -- ^ How to render the removed annotation
-> SimpleDocStream ann
-> out
renderSimplyDecorated text push pop = go []
where
go _ SFail = panicUncaughtFail
go [] SEmpty = mempty
go (_:_) SEmpty = panicInputNotFullyConsumed
go stack (SChar c rest) = text (T.singleton c) <> go stack rest
go stack (SText _l t rest) = text t <> go stack rest
go stack (SLine i rest) = text (T.singleton '\n') <> text (textSpaces i) <> go stack rest
go stack (SAnnPush ann rest) = push ann <> go (ann : stack) rest
go (ann:stack) (SAnnPop rest) = pop ann <> go stack rest
go [] SAnnPop{} = panicUnpairedPop
{-# INLINE renderSimplyDecorated #-}

@sjakobi
Copy link
Collaborator Author

sjakobi commented Jun 12, 2020

I've had a brief look at the Core and couldn't find any sign of concatenating empty Text blobs. I noticed that we still do all the annotation stack handling though, which probably causes unnecessary allocations. We might want to try getting rid of that.

sjakobi added a commit that referenced this issue Jun 12, 2020
…to avoid the annotation stack handling overhead of the stack machine
renderer.

Addresses #165.
sjakobi added a commit that referenced this issue Jun 13, 2020
renderSimplyDecorated's keeping track of the annotation stack wasn't
actually necessary here. With the custom render loop, we speed up the
large-output benchmarks by roughly 10%.

Note that this also removes some error detection functionality for
SimpleDocStreams with unbalanced SAnnPush and SAnnPop constructors.

Fixes #165.
sjakobi added a commit that referenced this issue Jun 13, 2020
renderSimplyDecorated's keeping track of the annotation stack wasn't
actually necessary here. With the new custom render loop, we speed up
the large-output benchmarks by roughly 10%.

Note that this also removes some error detection functionality for
SimpleDocStreams with unbalanced SAnnPush and SAnnPop constructors.

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

Successfully merging a pull request may close this issue.

1 participant