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

using strict language extensions. #752

Merged
merged 1 commit into from Jun 17, 2019

Conversation

kazu-yamamoto
Copy link
Contributor

This patch uses Strict and StrictData in wai, warp, warp-tls and auto-update.
I confirmed that this improves memory footprint.

@snoyberg Would you please explain why laziness is necessary for requestBody?

@snoyberg
Copy link
Member

I have a (perhaps irrational) concern about these language extensions, since they non-locally modify the behavior of code, making it confusing to read a source file. I'm not 100% opposed though.

For requestBody (and other fields): the advantage of lazy fields is that someone can only fill in the ones they are using, e.g. for testing. I don't think this is a strong argument. However, changing the field to strict is a breaking change, so I'd rather avoid it. Additionally: a field with an IO action in probably doesn't benefit much, if at all, from strictness.

@kazu-yamamoto
Copy link
Contributor Author

I understand your psychological barrier. :-) But honestly, I don't want to spend my time to seek the source of memory leak due to lazy evaluation anymore. :(

For experience of 10-years network programming in Haskell, I don't see any benefits of lazy evaluation except:

  • processing list data structures
  • atomicModifyIORef'

As SPJ described, lazyness is good test cases for purity. But it matters for Haskell compiler developers, not for users.

For months, I have been using Strict and StrictData. Bad things does not happen implicitly. They do happen explicitly (when running tests), so we can notice them before releasing. An interesting example is here:

https://gitlab.haskell.org/ghc/ghc/issues/16810

For soft-landing, it's OK for me to leave wai library as is and to apply these language extensions to libraries which we can convince, such as warp, warp-tls and auto-update.

What do you think?

@snoyberg
Copy link
Member

I'm definitely on board with the idea that laziness is generally a problem, I'm just worried about creating confusing code. But I'll defer to your judgement here: go for the release, but please leave wai alone for the moment.

@bitemyapp
Copy link
Contributor

bitemyapp commented Jun 13, 2019

I think @snoyberg already knows this, but I've started kicking around my code like @kazu-yamamoto where I use Strict/StrictData in projects.

It's been fine except for one annoying pattern with undefined and connection pools in the Yesod scaffold but that is due to go away w/ the yesod rio branch AIUI.

@kazu-yamamoto
Copy link
Contributor Author

@snoyberg Thank you for understanding.

I'm now tackling the space leak of my HTTP server. Strict helps me a lot. After I find the source of the problem and I'm convinced more, I will ask you to merge Strict stuff and release some packages.

@kazu-yamamoto
Copy link
Contributor Author

Tips:

I misunderstood that the heap profile file is generated after a Haskell program (with +RTS -h) is finished. But it's growing while the program is running. So, we can observe how memory is leaking without stopping a server with hp2ps.

Another issue: +RTS -hT cannot be used with GHC 8.6, why? I'm using +RTS -h now.

kazu-yamamoto added a commit to kazu-yamamoto/wai that referenced this pull request Jun 17, 2019
@kazu-yamamoto kazu-yamamoto merged commit ce607dd into yesodweb:master Jun 17, 2019
@kazu-yamamoto kazu-yamamoto deleted the strict branch June 17, 2019 03:46
@kazu-yamamoto
Copy link
Contributor Author

I have excluded wai and merged this PR.

@kazu-yamamoto
Copy link
Contributor Author

New versions of warp, warp-tls and auto-update have been released.

TristanCacqueray added a commit to TristanCacqueray/monocle that referenced this pull request Oct 6, 2021
This change enables StrictData to prevent space leaks, as Kazu Yamamoto
explained in yesodweb/wai#752 (comment)

To do that, this change removes:

- `fromMaybe (error "xx is needed")` lazy trick
- the search cli which is not useful
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants