Opened 2 years ago

Last modified 9 months ago

#13564 new task

Why does memory usage increase so much during CoreTidy?

Reported by: rwbarton Owned by:
Priority: normal Milestone: 8.10.1
Component: Compiler Version: 8.1
Keywords: Cc: MikolajKonarski
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time performance bug Test Case:
Blocked By: Blocking:
Related Tickets: #13586 Differential Rev(s): Phab:D3516, Phab:D3524
Wiki Page:

Description

I made GHC display the live data size before and after each pass and while compiling DynFlags, the live data size increased by almost 50% during the CoreTidy pass, even though the Core program size hardly changed:

...
Result size of Simplifier
  = {terms: 104,494,
     types: 264,683,
     coercions: 15,760,
     joins: 58/680}
!!! Simplifier [DynFlags]: 19.69 221248736: finished in 5337.48 milliseconds, allocated 4416.624 megabytes
*** Demand analysis [DynFlags]: 19.69 221248152:
Result size of Demand analysis
  = {terms: 104,494,
     types: 264,683,
     coercions: 15,760,
     joins: 58/680}
!!! Demand analysis [DynFlags]: 20.77 248604416: finished in 1349.45 milliseconds, allocated 2298.950 megabytes
*** CoreTidy [DynFlags]: 20.77 248603432:
Result size of Tidy Core
  = {terms: 104,440,
     types: 264,565,
     coercions: 15,700,
     joins: 58/679}
!!! CoreTidy [DynFlags]: 21.43 342594336: finished in 1124.37 milliseconds, allocated 736.209 megabytes

I find the amount of this increase surprising considering what CoreTidy does.

Attachments (3)

ghc-stage2.pdf (66.7 KB) - added by rwbarton 2 years ago.
heap profile with HEAD
ghc-stage2.2.pdf (69.8 KB) - added by rwbarton 2 years ago.
heap profile with seqUnfolding in tidyUnfolding
ghc-stage2.3.pdf (55.6 KB) - added by rwbarton 2 years ago.
heap profile with seqBinds in tidyTopBinds as well

Download all attachments as: .zip

Change History (20)

comment:1 Changed 2 years ago by rwbarton

Owner: set to rwbarton

comment:2 Changed 2 years ago by rwbarton

Happily the cause turns out to be simple. In tidyUnfolding we tidy the RHS of a stable CoreUnfolding. But CoreUnfolding also has fields uf_is_value, uf_is_conlike, etc., which are cached computations on the original RHS. Apparently it's common for these to never be used at any point (the iface will only record uf_guidance); so we should evaluate these in tidyUnfolding, or just throw away the ones that won't ever be used.

I'm in the process of verifying this fix still has the expected result with HEAD.

There might be further improvements to be made here, but indications are that this fix at least decreases memory usage during code generation to the point where it is no longer larger than the peak usage during simplification. In other words, max memory usage can't be reduced any more from this side.

Changed 2 years ago by rwbarton

Attachment: ghc-stage2.pdf added

heap profile with HEAD

Changed 2 years ago by rwbarton

Attachment: ghc-stage2.2.pdf added

heap profile with seqUnfolding in tidyUnfolding

comment:3 Changed 2 years ago by rwbarton

After adding seqUnfolding there is still a tall but narrow spike in the heap profile. I found that this spike went away with -v, so I added a seqBinds which also eliminates the spike. Trac won't let me upload another heap profile PDF though.

Changed 2 years ago by rwbarton

Attachment: ghc-stage2.3.pdf added

heap profile with seqBinds in tidyTopBinds as well

comment:4 Changed 2 years ago by rwbarton

Differential Rev(s): Phab:D3516
Status: newpatch

comment:5 Changed 2 years ago by Ben Gamari <ben@…>

In 5c602d22/ghc:

Avoid excessive space usage from unfoldings in CoreTidy

Test Plan: validate

Reviewers: austin, bgamari

Reviewed By: bgamari

Subscribers: thomie

GHC Trac Issues: #13564

Differential Revision: https://phabricator.haskell.org/D3516

comment:6 Changed 2 years ago by bgamari

Milestone: 8.2.1
Resolution: fixed
Status: patchclosed

comment:7 Changed 2 years ago by simonpj

Owner: rwbarton deleted
Resolution: fixed
Status: closednew

I found that this spike went away with -v, so I added a seqBinds which also eliminates the spike

This is ok, but it's an un-satisfying Big Hammer. It forces us to make yet another pass over the entire program, when there is probably a more refined solution to hand.

You might well be able to eliminate the other change in tidyUnfolding when you use the Big Hammer. And maybe we should eliminate it, since we are now seq'ing the unfolding twice.

But it would be nicer to find the source of the spike in the first place.

comment:8 Changed 2 years ago by bgamari

Differential Rev(s): Phab:D3516Phab:D3516, Phab:D3524

comment:9 Changed 2 years ago by Ben Gamari <ben@…>

In b3da6a6c/ghc:

CoreTidy: Don't seq unfoldings

Previously we would force uf_is_value and friends to ensure that we didn't
retain a reference to the pre-tidying template, resulting in a space leak.
Instead, we now just reinitialize these fields (despite the fact that they
should not have changed). This may result in a bit more computation, but most of
the time we won't ever evaluate them anyways, so the damage shouldn't be so bad.

See #13564.

comment:10 Changed 2 years ago by bgamari

Milestone: 8.2.18.2.2

comment:11 Changed 2 years ago by bgamari

Milestone: 8.2.28.4.1

Bumping this off to 8.4.1.

comment:12 Changed 20 months ago by bgamari

Milestone: 8.4.18.6.1

This ticket won't be resolved in 8.4; remilestoning for 8.6. Do holler if you are affected by this or would otherwise like to work on it.

comment:13 Changed 16 months ago by MikolajKonarski

Isn't this ticket fixed? Is CoreTidy pass still leaking in 8.4? Or is the ticket dangling, because the in-code docs can be improved, as in https://phabricator.haskell.org/D3524#100418?

I'm asking, because I'm trying to make #13586 more useful (split or narrow down).

comment:14 Changed 15 months ago by MikolajKonarski

comment:15 Changed 15 months ago by MikolajKonarski

Cc: MikolajKonarski added

comment:16 Changed 15 months ago by bgamari

Milestone: 8.6.18.8.1

These will not be addressed in GHC 8.6.

comment:17 Changed 9 months ago by osa1

Milestone: 8.8.18.10.1

Bumping milestones of low-priority tickets.

Note: See TracTickets for help on using tickets.