Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

Fix UriCaches being leaked #1292

Merged
merged 1 commit into from
Jun 16, 2019
Merged

Conversation

lukel97
Copy link
Collaborator

@lukel97 lukel97 commented Jun 16, 2019

This fixes UriCaches from being leaked via GhcModuleCaches. This is the result of 72 hours at ZuriHac between 3 people and a lot of time spent in gdb.

Fixes #412, #806, #665
Blog post coming soon

Co-Authored-By: Matthew Pickering matthewtpickering@gmail.com
Co-Authored-By: Daniel Gröber dxld@darkboxed.org

This fixes UriCaches from being leaked via GhcModuleCaches.
This is the result of 72 hours at ZuriHac between 3 people and a lot of
time spent in gdb.

Blog post coming soon

Co-Authored-By: Matthew Pickering <matthewtpickering@gmail.com>
Co-Authored-By: Daniel Gröber <dxld@darkboxed.org>
@alanz
Copy link
Collaborator

alanz commented Jun 16, 2019

This leaves me wondering how we strayed from best practice of making all data structure fields strict, unless there is a good reason not to.

Copy link
Collaborator

@wz1000 wz1000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Any concrete before/after measurements?

@lukel97
Copy link
Collaborator Author

lukel97 commented Jun 16, 2019

@wz1000 Running with an lsp-test test-scenario that makes 10 edits:

Before 🚰

memleak

After

memfixed

@lukel97
Copy link
Collaborator Author

lukel97 commented Jun 16, 2019

@wz1000 I will tidy up the test executable and make a separate PR for it

@lorenzo
Copy link
Collaborator

lorenzo commented Jun 16, 2019

Should we just add StrictData as default extension to the project?

@alanz
Copy link
Collaborator

alanz commented Jun 16, 2019

Should we just add StrictData as default extension to the project?

So long as there is a way to mark fields as lazy, if required.

@alanz
Copy link
Collaborator

alanz commented Jun 16, 2019

And it is great to have the ability to profile in the project knowledge base. Well done.

@lorenzo
Copy link
Collaborator

lorenzo commented Jun 16, 2019

So long as there is a way to mark fields as lazy, if required.

Yes it is possible, you can make any field lazy by prepending ~ to it. It has the opposite effect of !

@lorenzo
Copy link
Collaborator

lorenzo commented Jun 16, 2019

@bubba Would you mind profiling again after adding StrictData by default?

@lukel97
Copy link
Collaborator Author

lukel97 commented Jun 16, 2019

@lorenzo sure! I’ll see if I can convert my test case into a benchmark actually

@alanz alanz merged commit b818cfa into haskell:master Jun 16, 2019
@alanz alanz added this to the 2019-06 milestone Jul 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory characteristics questions
5 participants