Refactor Generic ToJSON

[Lysxia]

This merges the code for GToEncoding and GToJSON which look awfully similar.

I decided to keep exposing GToEncoding and GToJSON with the original kind as constraint synonyms for backwards compatibility, as they seem to be used in some places for type annotations. I don't think their methods are in use externally however.

This will conflict with #522. I guess I'll resolve it depending on whichever gets merged first.

[phadej]

There is a performance reason they are separate, ping @RyanGlScott

[RyanGlScott]

The only thing critical for compilation speed is that GToJSON has exactly one class method (see #335), which this PR preserves.

That being said, @Lysxia, can you run this code on a generics-related benchmark (like this one) to see if the compilation time/runtime are about the same? If so, 👍 from me.

[phadej]

If I might ask, than would be nice to run benchmark on old GHCs as well, e.g 7.8.4 and 7.10.3 too, would be great that performance doesn't degrade on old compilers dramatically

[Lysxia]

Sure!

[bergmark]

Looks like there are some build errors on older GHC's

[Lysxia]

Here are some benchmarks. One file per GHC version. master is on the left, this PR is on the right. There are two parts, the first with the flag fast turned on (the default), and the second with it off. I compare the time to compile the library, then the time to compile the benchmark executable, first without optimization, then with. I then run the optimized one.

https://gist.github.com/Lysxia/bb3e6dd68f6121b904363dc48e11da0d

aeson takes about the same time to build in any case. To compile the final executable with an optimized aeson, this PR seems to slow down a bit, except with 8.0.2.

7.8.4 user	0m12.077s	user	0m12.967s
7.10.3 user	0m15.960s	user	0m18.560s
8.0.1 user	0m19.010s	user	0m22.313s
8.0.2 user	0m19.000s	user	0m18.223s

At runtime there are cases where this PR is slower if aeson is not optimized, but faster if it is optimized. For instance:

 7.8.4
D/toJSON/generic
aeson+fast 28.52 μs 32.91 μs
aeson-fast 9.335 μs 5.926 μs

I found one case where the reverse happens:

8.0.2
BigRecord/encode/generic
aeson+fast 63.22 μs 50.10 μs
aeson-fast 3.886 μs 10.15 μs

Not sure what's going on yet but I can look into it later.

[phadej]

@Lysxia what are the base point times, i.e. before the patch?

EDIT, Ah I see. confused user user with system user

[phadej]

Great job anyways, tell if you cannot find a reason for BigRecord digression

[bergmark]

the first with the flag fast turned on (the default)

What do you mean by default? when compiling aeson normally fast is off and -02 is used.

To compile the final executable with an optimized aeson, this PR seems to slow down a bit, except with 8.0.2.

So it actually got faster on 8.0.2!? I'm inclined to say that we can limit performance requirements to the latest ghc, do others agree?

[phadej]

@begrmark, I don't care about compilation performance, but run-time performance shouldn't be drastically different on at least three previous major GHC releases (i.e. 7.8.4-8.0.2 atm)

[bergmark]

I don't care about compilation performance, but run-time performance shouldn't be drastically different on at least three previous major GHC releases

Oops, I misread the first portion as stating everything got faster on 8.0.2 :)

[Lysxia]

What do you mean by default? when compiling aeson normally fast is off and -02 is used.

Oops, you are right. I got it confused with the fact that it is turned on by the stack-lts*.yaml files.

As for performance there are unfortunately some regressions to investigate, but there is hope that this update will be at least as fast as it is currently.

[Lysxia]

GHC.Generics.from and to are not inlined for large types (GHC issue), and this causes the inliner to behave somewhat erratically.

Adding INLINE annotations in aeson helps, but I think a more effective solution would be for users to somehow derive Generic instances with INLINEto and from.

[bergmark]

Related to inlining: #424

[Lysxia]

It turns out inlining from isn't sufficient. But by selectively inlining a few more functions I think I've managed to fix all regressions without affecting compile times much. At least the one case I mentioned above is now a speed up.

[Lysxia]

Latest comparison benchmarks, left is before patch, right is after.

https://gist.github.com/Lysxia/7ea97543e1821ae7f73758e82d35b6a9