Get rid of lookupCont

[treeowl]

lookupCont was an ugly and barely comprehensible way to reduce the amount of compatibility code we needed to support GHC versions without unboxed sums. Since we no longer support those, I believe we should manually inline it into lookupRecordCollision# and barbecue its remains.

[treeowl]

Hrmm.... Just read the comments, which suggest there's a theoretical possibility of a performance reason too. Guess we should double check, though I think it's rather unlikely to be an issue.

[sjakobi]

How do you suggest replacing its use in isSubmapOfBy and the intersectionWithKey# function being added in #406?

[treeowl]

Is there some reason those don't use lookup? Probably my doing, but it seems overkill.

[oberblastmeister]

Am I right that lookup will materialize the unboxed sums?

[oberblastmeister]

I guess we could just inline the ones that return unboxed sums, like doing inline lookup#

[treeowl]

@oberblastmeister, lookup should always inline, and the Maybes should get erased.

[oberblastmeister]

@treeowl, the Maybe yes, but not the unboxed sums right? That's why I was thinking of making sure the lookup# inlines

[treeowl]

Yeah, the unboxed sums will stick around. But does it really matter if we return the collision index when we don't need it? I don't think we can make that go away with just inlining unless we do something like lookupCont, but that seems way overkill for something that most likely doesn't actually matter.

[treeowl]

@oberblastmeister, oh, there's one other complication... The Cont version takes an extra argument we need for isSubmapOfBy (something to do with shifts or something I can never keep straight). So we'll need to keep that aspect around.

[oberblastmeister]

But does it really matter if we return the collision index when we don't need it? I don't think we can make that go away with just inlining unless we do something like lookupCont

What do you mean by that?

[treeowl]

When lookupCont inlines (which should be always), it drops the collision index on the floor immediately. Calling lookupRecordCollision# and dropping the part we don't need won't prevent that value from being returned (in a register/on the stack), I don't believe. There's recursion down there.

[treeowl]

But ... like I said before, I would be really surprised if it mattered measurably.

[sjakobi]

Since the discussion suggests that this issue may not be entirely straightforward to address, I've removed the low-hanging-fruit label.

I agree that lookupCont is a bit awkward to use. If the alternatives are less efficient, I'd rather keep it though.

[treeowl]

Once someone (likely me) writes the alternative code, someone (likely not me) can benchmark. (I do not trust my ability to get my system quiet/stable enough to do a decent job of that in general, and my computer is kind of old.)

[sjakobi]

I can run the benchmarks. Given their noisiness (#332), I don't think this will be very helpful though.

Diffing the generated code might also give us some clues. See https://github.com/haskell-unordered-containers/unordered-containers/blob/master/CONTRIBUTING.md#inspecting-the-generated-code for instructions.