AState: Use foldr and cons to avoid an O(n^2) blowup when building the fblocks

[nanavati]

No description provided.

[quark17]

I'm not sure that Sequence is the most efficient here. I believe that we're only ever appending singletons, so not only do we know which side is smaller, it's a single element that we can just cons onto the front. If I'm not mistaken?

If I understand, we have pairs (a,b) and we want to group them by the first element into (a,[b]). So the high-level question is what's the most efficient way to do that. The existing code converts the pairs into singletons, then uses a map to group them, combining with flip (++) so that each singleton is appended to the end of the list, to preserve the order they were in. I notice that the documentation for fromListWith discusses performance and gives this exact situation, suggesting to replace flip with an initial reverse:

fromListWith (++) $ reverse $ map (\(k, v) -> (k, [v])) someListOfTuples

Although, would it be better to reverse the resulting pairs at the end, rather than the whole list at first?

Since we're only ever appending singletons, I would have suggested cons, but then you'd need a fromListWith that was more general than a -> a -> a. But maybe GHC is smart enough to notice that ++ is only ever applied to a singleton first argument and reduce it to cons?

[nanavati]

I didn't want to leave code with ++ in there even if it worked because it seems fragile. If our assumptions about list size become wrong for some reason the blowup will be silently reintroduced.

Instead I made a custom fold using foldr and cons to build up the lists for the map. Happily building with foldr and cons kept everything in the right order so no reverse was required. And this way our assumptions about relative sizes can't break because putting a larger list in the wrong position just won't typecheck.

[quark17]

Looks good, thanks!