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

Fix 2 space leaks #2774

Merged
merged 5 commits into from Mar 15, 2022
Merged

Fix 2 space leaks #2774

merged 5 commits into from Mar 15, 2022

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Mar 12, 2022

I found a space leak in hls-graph by running the edit benchmark with +RTS -hc to track down the monotonically increasing cost centre and then trying various things until ultimately locating the source of the leak.

The thing that helped ultimately was an info table profile (-hi), only available with ghc 9.2.2 and requires building with -finfo-table-map -fdistinct-constructor-tables). Thanks @mpickering and @HasuraHQ for contributing this profiling mode!

Command line, for reference:

cabal build --project-file cabal-ghc921.project -w ghc-9.2 exe:ghcide && dist-newstyle/build/x86_64-osx/ghc-9.2.2/ghcide-1.6.0.1/x/ghcide-bench/build/ghcide-bench/ghcide-bench --ghcide dist-newstyle/build/x86_64-osx/ghc-9.2.2/ghcide-1.6.0.1/x/ghcide/build/ghcide/ghcide -s edit --ghcide-options "+RTS -hc  -l -RTS"

Before

image

image

image

After

image

image

@pepeiborra
Copy link
Collaborator Author

I found a second space leak, this time in the ExportsMap code in ghcide, using the same method.

Before

image

image

After

image

image

@pepeiborra pepeiborra changed the title Fix a space leak in hls-graph Fix 2 space leaks Mar 12, 2022
@michaelpj
Copy link
Collaborator

What do you think about just turning on StrictData everywhere?

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.

Great detective work!

@pepeiborra
Copy link
Collaborator Author

What do you think about just turning on StrictData everywhere?

I am a big fan.

@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Mar 15, 2022
@mergify mergify bot merged commit 30d48ed into master Mar 15, 2022
July541 pushed a commit to July541/haskell-language-server that referenced this pull request Mar 30, 2022
* Fix a space leak in hls-graph

* Fix leak in ExportsMap and optimize
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants