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
persistent-qq
weird memory bug
#1434
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very odd behavior. Gonna try in the work codebase...
TH.infixE | ||
(Just [| uncurry $(fun) . second concat |]) | ||
([| (=<<) |]) | ||
(Just $ go toks) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote the TH in this function to be a bit more concise and use more quotes vs manually splicing TH functions together.
Building the
😮 70 second reduction. Wow. |
This looks awesome 👍 presumably the reduction in compile time in the test suite for persistent carries over to compiling a project that uses persistent? Do you have any numbers to illustrate the impact of this if so? I'm also wondering (forgive me if you've done this elsewhere and I have missed this) if this kind of thing could be benchmarked and managed in CI or something (or even just reported in the project). It seems to me like these performance things with TH are so easy to creep in unnoticed and are hard to spot just through looking at the code, is there potential here for things to change again and something like this to re-appear? Unfortunately, I also feel like these kinds of things are hard to test / enforce, in the way that specs or static analysis is for instance. If there is something we could do here with this I would be happy to help out with implementing something, compile times is a bit of a bugbear of mine so I'm happy to help out with this where I can 😁 I know there is a ticket I opened about the benchmarking not running, potentially that could be resurrected a bit and I try to implement something that would cover this ticket? Great work anyway @parsonsmatt 👍 this looks like a big win |
We could probably just require Our With this patch + reverting back to the many-splices that OOMed our build box, we're down to 28 minutes and 24GB of RAM. Huge improvement in resource utilization. |
Before submitting your PR, check that you've:
@since
declarations to the Haddockstylish-haskell
on any changed files..editorconfig
file for details)After submitting your PR:
(unreleased)
on the Changelogpersistent-qq
memory testingWe had an issue at work where a
sqlQQ
quasi quoter was using way too much memory.We switched away from the
^{}
and@{}
splices for field and column names, and that fixed it.I decided to try and diagnose it.
First step is getting a "baseline": a memory use from GHC that tells me how much mem we're using.
I built the test with
stack build --test --no-run-tests --ghc-options "-O2 +RTS -s -RTS"
, which gives me a memory output.A run without any QQ at all is:
When we add a query with a bunch of stuff,
958 MiB of use! Dang.
When
query0
is changed topure []
, the build looks quite different:So, 157mb is probably the "default" or good case.
With a QQ and no splices, we ge tthis:
151MB. Less than with out any splices! Intriguing.
If we introduce a
#{"asdf" :: Text}
instead of'asdf'
in the quoter, we don't notice much change:156MB, well within ordinary.
Adding a second paramater causes much more memory to be used:
230MB! 50MB for a single parameter. A third parameter adds another bunch of space used:
270MiB. So we're looking at 40-50MiB per parameter.
Is it just text? Let's try with an int. Testing with an int gives the same result.
The Template Haskell isn't necessary to trigger anything.
This does just fine!
Looking at the code, we have a bunch of
fmap
.Is that the problem?
What if I combine those
fmap
s into one?Well, turns out the
fmap
count doesn't matter. I inlined a bunch of the functions into a singlefmap (f1 . f2 . f3 . f4)
, with no reduction in compilation time.I then inlined all those functions and produced a direct lambda. No change.
I then ... changed a big chain of
<>
into anmconcat
and it dropped 100MB.I replcaed some
,
in the list with<>
to see if the overall<>
count was the issue, but it didn't matter until I hadmconcat [ a <> b <> c <> d ]
at which point it went up a shitload again.It seems like the inliner is getting really happy with certain combinations of
<>
forText
. The<>
forString
doesn't appear to cause any issues at all, and memory use holds prety consistent around 150MB.Wild.