Refactor tyConUnique into a function
Needs RevisionPublic

Authored by int-index on Dec 23 2016, 7:10 PM.

Details

Summary

The tyConUnique record field in TyCon is redundant because there's
a corresponding unique in tyConName. Since both of those uniques are equal and
available for any TyCon, we can safely remove one of them.

Test Plan

./validate

int-index updated this revision to Diff 10177.Dec 23 2016, 7:10 PM
int-index retitled this revision from to Refactor tyConUnique into a function.
int-index updated this object.
int-index edited the test plan for this revision. (Show Details)
bgamari added a subscriber: simonpj.

I have also wondered about this. The only reason I can see to cache the unique like this is to save an indirection as the unique is quite often used. That being said, I would be surprised if the difference would be measurable.

On the whole this patch seems reasonable to me but I'll let @simonpj weigh in as well.

simonpj edited edge metadata.Jan 4 2017, 2:47 AM

Yes, this is purely an efficiency question. It applies to Var as well.

If you'd like to pursue it, could you do some careful measurements please? Any effects are likely to be small, so using Criterion might be called for.

Thanks!

bgamari requested changes to this revision.Jan 5 2017, 2:30 PM
bgamari edited edge metadata.

@int-index, I think we might be able to make our cake and eat it too if we put a bang on the Name, which would allow GHC to unpack it, eliminating the indirection.

This revision now requires changes to proceed.Jan 5 2017, 2:30 PM
austin resigned from this revision.Nov 9 2017, 11:31 AM