Opened 4 years ago

Closed 3 years ago

#11598 closed task (wontfix)

Cache coercion kinds and roles

Reported by: goldfire Owned by: goldfire
Priority: normal Milestone:
Component: Compiler Version: 7.10.3
Keywords: Cc: RyanGlScott
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time performance bug Test Case:
Blocked By: Blocking:
Related Tickets: #8095 Differential Rev(s):
Wiki Page:

Description (last modified by simonpj)

Because we never inspect the structure of a coercion, it is easy to cache coercions' kinds and role at the top level, using something like this:

data Coercion = MkCoercion { coercionKind :: Pair Type
                           , coercionRole :: Role
                           , coercionRep  :: CoercionRep }
data CoercionRep = -- the long, recursive datatype we have now

In the process, various role and type information currently stored in the recursive structure could be dropped.

This would make #8095 much easier.

See also #11735.

Change History (6)

comment:1 Changed 3 years ago by goldfire

This will make #8095 much easier.

comment:2 Changed 3 years ago by goldfire

Milestone: 8.2.1
Type of failure: None/UnknownCompile-time performance bug

I have made a real solid attempt at this, but I am giving up for now. My work is here (the cache-coercion-kind tag on my github fork).

The code there has a roughly 10% slowdown on most of the perf/compiler performance tests, along with more drastic slowdowns on the T9872x tests. (It has a 16% improvement on T5030, though.)

It is unclear to me where the slowdown is coming from. I have explored T9872d in some detail, though. It is slowed by the fact that phantom coercions are not always made anymore by mkPhantomCo. They can now be SubCo Phantom, too. This leads to a ton of work zonking a coercion hole proving a phantom equality. But I suspect phantoms are rare, and so this is not the cause of the general slowdown. Unfortunately, because PhantomProv needs a kind coercion, we are required to have coercionRepKind (a function that calculates an uncached kind). This is doable, of course, but I'm wary of sinking more time into this.

It's quite possible that savings are realized only in concert with #8095, but the hour is too late for that.

Just to record some thoughts on #8095: That ticket proposes looking at every mkTcXXXCo call to see if -dcore-lint is set and to drop coercions if not. I now think this is the wrong approach. Instead, let's use laziness: we just have to be careful about when coercionReps are forced. As long as we never force them, we'll never have to calculate them. We should, when convenient, replace the thunked coercions with OmittedCo or something. For example, during zonking and simplification. And we'll need to write OmittedCo to interface files. But it shouldn't be too bad. To pull this off, we'll have to cache a few more things than my work already does: specifically, we'll need to cache the set of tycons mentioned in a coercion and whether or not the coercion is reflexive.

I'm also worried about Rules.match_co which examines the structure of a coercion, so we may need a bit of re-engineering around that. This is a welcome problem, though, because the fact that match_co looks at coercions is just awful.

Bottom line: I'm punting to 8.2, and lowering my expectations for performance improvements that will go into 8.0.

comment:3 Changed 3 years ago by RyanGlScott

Cc: RyanGlScott added

comment:4 Changed 3 years ago by simonpj

Description: modified (diff)

comment:5 Changed 3 years ago by bgamari

Milestone: 8.2.1
Type: bugtask

Richard, what do we want to do about this? It sounds like this approach didn't really work out.

comment:6 Changed 3 years ago by goldfire

Resolution: wontfix
Status: newclosed

That's true. I'm abandoning this attempt, though the knowledge I gained and recorded above will be helpful in tackling #8095, which is still very much on the table.

Note: See TracTickets for help on using tickets.