Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#13719 closed bug (fixed)

checkFamInstConsistency dominates compile time

Reported by: niteria Owned by:
Priority: normal Milestone:
Component: Compiler (Type checker) Version: 8.3
Keywords: Cc: simonmar, simonpj, ezyang, rwbarton, RyanGlScott
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time performance bug Test Case:
Blocked By: Blocking:
Related Tickets: #13092, #13099, #12191 Differential Rev(s): phab:D3600, phab:D3602, phab:D3603
Wiki Page:

Description

I'm looking into compile time issues on our internal code base. checkFamInstConsistency takes 50% of all compile time for :load in ghci for us.

I've created a synthetic test case that approximates the issue and in which compiling 300 modules with one small data type each takes 65s total, and checkFamInstConsistency is 87% of that and 94.1% of allocations. checkFamInstConsistency is run to ensure consistency of a type family associated with Generics, but that's only semi-relevant.

To reproduce:

./gen.sh
./inplace/bin/ghc-stage2 -keep-tmp-files DummyLevel3M100.hs

Profile with some relevant cost centres: https://phabricator.haskell.org/P150

The next step is to look into implementing: https://ghc.haskell.org/trac/ghc/ticket/13092#comment:14

I also plan to add this test case to the codebase.

Attachments (1)

gen.sh (794 bytes) - added by niteria 2 years ago.
gen.sh

Download all attachments as: .zip

Change History (9)

Changed 2 years ago by niteria

Attachment: gen.sh added

gen.sh

comment:1 Changed 2 years ago by simonpj

That does seem terrible. Thanks for continuing to investiate and explore solutions.

comment:2 Changed 2 years ago by niteria

Differential Rev(s): phab:D3600

comment:3 Changed 2 years ago by niteria

Differential Rev(s): phab:D3600phab:D3600, phab:D3602

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

In 17fef39/ghc:

Testcase for #13719

I expect to improve this, a testcase will ensure
it doesn't regress.

Test Plan: ./validate

Reviewers: simonmar, austin, bgamari

Reviewed By: bgamari

Subscribers: rwbarton, thomie

GHC Trac Issues: #13719

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

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

In 2bc3a057/ghc:

Testcase for type family consistency checks

Based on my quick search, we don't have a test
that verifies that we check the type family instances of
currently compiled module against direct or indirect
dependencies.

This adds two tests: for a direct dependency and
for an indirect dependency.

I also added a comment to make it clear what the 'Over'
test tests.

Other than completeness, it makes sense to have these
tests because if you look at
Note [The type family instance consistency story] in FamInsts
these cases are checked through different mechanisms.

Test Plan: new tests

Reviewers: simonmar, rwbarton, simonpj, austin, bgamari

Reviewed By: simonpj, bgamari

Subscribers: thomie

GHC Trac Issues: #13719

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

comment:6 Changed 2 years ago by niteria

Differential Rev(s): phab:D3600, phab:D3602phab:D3600, phab:D3602, phab:D3603

comment:7 Changed 2 years ago by Bartosz Nitka <niteria@…>

In 69d9081d/ghc:

Faster checkFamInstConsistency

This implements the idea from
https://ghc.haskell.org/trac/ghc/ticket/13092#comment:14.

It's explained in Note [Checking family instance optimization]
in more detail.

This improves the test case T13719 tenfold and
cuts down the compile time on `:load` in `ghci` on our
internal code base by half.

Test Plan: ./validate

Reviewers: simonpj, simonmar, rwbarton, austin, bgamari

Reviewed By: simonpj

Subscribers: thomie

GHC Trac Issues: #13719

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

comment:8 Changed 2 years ago by niteria

Resolution: fixed
Status: newclosed

From my perspective this is fixed.

It's a localized change with a nice upside, so we might want to merge to ghc-8.2.

This applies cleanly:

git cherry-pick -x 4e0e120bcbda6c5351d7c5aa01f7298e2198d457
git cherry-pick -x 033f897a8ad34d62aff585d9df16c640bb55f21c
git cherry-pick -x 17fef390c575c153c7e70438783e7f8fee62e451
git cherry-pick -x 69d9081d9fa3ff36fda36e4e11ef7e8f946ecf2a

I'm happy to do the legwork, but I will leave the decision to Ben.

Last edited 2 years ago by niteria (previous) (diff)
Note: See TracTickets for help on using tickets.