Lysxia / generic-data

Generic data types in Haskell, utilities for GHC.Generics
https://hackage.haskell.org/package/generic-data
MIT License
44 stars 9 forks source link

Generic Semigroup instance is not optimised #44

Closed sheaf closed 4 years ago

sheaf commented 4 years ago

The following Semigroup instance seems to optimise poorly:

data Point2D a = Point2D !a !a
  deriving stock ( Show, Eq, Ord, Generic )

newtype Vector2D a = Vector2D ( Point2D a )
  deriving stock   ( Show, Generic )
  deriving newtype ( Eq, Ord )
  deriving Semigroup
    via GenericProduct ( Point2D ( Sum a ) )

I noticed this from the profiling output of an application which uses the above instance, which contained the following lines:

COST CENTRE   MODULE                        SRC                                                %time %alloc
gmappend      Generic.Data.Internal.Prelude src\Generic\Data\Internal\Prelude.hs:56:1-42         9.9    5.6
from'         Generic.Data.Internal.Utils   src\Generic\Data\Internal\Utils.hs:50:1-12           4.8    0.0

In this application, the Semigroup ( Vector2D a ) was the only instance using gmappend; moreover, all the occurrences of from' are associated with this single instance.
Switching to a hand-written instance made the other functions in the profiling report take up about 10% more of the total time, which corresponds to a significant speed up from writing mappend manually.

Note that the same issue occurs if we use the newtype Generically instead of GenericProduct (with the necessary adjustments to the code to allow the instance to be derived).

I've attached a quick (failing) inspection test using inspection-testing.

gmonoid-inspection.zip

Forgive me if it isn't expected that the generics in this instance should be optimised away; please let me know if there are better ways to go about this. Thanks.

sheaf commented 4 years ago

Actually, it seems I spoke too soon: the inspection test passes as long as one compiles with -O. The problem is that profiling disables optimisations, causing these spurious inflated cost centres. Sorry for the noise.

Lysxia commented 4 years ago

No problem!

Forgive me if it isn't expected that the generics in this instance should be optimised away; please let me know if there are better ways to go about this. Thanks.

This is definitely one of the goals of this library. So don't hesitate to report any discrepancy you may find!

For instance, I believe there are issues with inlining for large types (around 10 fields or more). I would love to know if that's a problem for someone.