Major Data.Foldable cleanup
AbandonedPublic

Authored by dfeuer on Oct 29 2014, 3:56 PM.

Details

Reviewers
nomeata
austin
ekmett
Trac Issues
#9742
Summary

The default implementations of foldl1 and foldr1 in Data.Foldable were spine-strict, probably by accident. Make them lazy.

Make default implementations somewhat less right-handed.

Use explicit lambdas in fold helpers to (hopefully) help with inlining and (certainly) increase clarity.

Remove redundant (and illegal) constraints on the class variable that crept in as copypasta.

Use (safe) coercions to improve efficiency of some newtype wrapping/unwrapping, as suggested by Edward Kmett.

NOTE: This should not land until sufficiently approved. Edward Kmett has asked for the proposal process to be used regarding foldl1/foldr1 strictness, at least.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
foldable
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 1518
Build 1524: GHC Patch Validation (amd64/Linux)
dfeuer updated this revision to Diff 1099.Oct 29 2014, 3:56 PM
dfeuer retitled this revision from to Fix laziness of foldl1 and foldr1.
dfeuer updated this object.
dfeuer edited the test plan for this revision. (Show Details)
dfeuer added reviewers: ekmett, austin.
dfeuer updated the Trac tickets for this revision.
dfeuer updated this object.Oct 29 2014, 3:59 PM
dfeuer edited edge metadata.
dfeuer added a project: GHC.
dfeuer updated this object.Oct 29 2014, 4:03 PM
ekmett edited edge metadata.Oct 29 2014, 6:14 PM

I'm personally +1 on this, but I'd like to have cooler heads review it before we proceed.

dfeuer updated this revision to Diff 1116.Oct 30 2014, 1:46 AM
dfeuer edited edge metadata.

Lots more bias cleanup. Also used (safe) coercions as suggested
by Edward Kmett to avoid potential problems with Trac #7542.

dfeuer updated this revision to Diff 1117.Oct 30 2014, 2:42 AM

Rebase; use coerce for some more functions that can benefit.

dfeuer retitled this revision from Fix laziness of foldl1 and foldr1 to Major Data.Foldable cleanup.Oct 30 2014, 2:46 AM
dfeuer updated this object.
dfeuer added a reviewer: nomeata.
dfeuer updated this revision to Diff 1118.Oct 30 2014, 3:12 AM

Try more rebasing and such.

dfeuer updated this revision to Diff 1119.Oct 30 2014, 4:18 AM

No idea.

dfeuer updated this revision to Diff 1126.Oct 30 2014, 11:06 AM

Coding while sleeping is hazardous to both health and code.
Let's see if this fixes it.

dfeuer updated this revision to Diff 1127.Oct 30 2014, 11:47 AM

Remove type added in troubleshooting.

dfeuer updated this revision to Diff 1132.Oct 30 2014, 3:58 PM

Redid the array instance, etc. Hopefully this will help.
In any case, I will probably need to split this CR up some.

dfeuer updated this revision to Diff 1133.Oct 30 2014, 4:25 PM

Revert traverse_ and sequenceA_ changes to see if that's the
problem.

dfeuer updated this revision to Diff 1134.Oct 30 2014, 4:35 PM

Silly warning

dfeuer updated this revision to Diff 1136.Oct 30 2014, 10:31 PM

See if using GND for one of the Foldable instances in the
compiler clears up the performance problem.

dfeuer updated this revision to Diff 1137.Oct 30 2014, 10:36 PM

Stupid warning.

dfeuer updated this revision to Diff 1138.Oct 30 2014, 11:12 PM

-funbox-strict-fields shouldn't have any effect here, but
maybe it does?

ekmett accepted this revision.Nov 7 2014, 9:58 AM
ekmett edited edge metadata.

LGTM

hvr added a subscriber: hvr.Nov 7 2014, 10:31 AM

@dfeuer is this code-review still current or was it obsoleted by newer ones?

This definitely needs a significant update/replacement.

dfeuer abandoned this revision.Nov 7 2014, 10:43 AM

Yeah, this has already been replaced, essentially. Sorry.