Rework GFromJSON instances to enable elimination of Generics

[remyoudompheng]

The Parser monad signature makes inlining very difficult
because GHC only inlines saturated applications, which is
hard to achieve with Parser.

We work around this by defining an alternate method in GFromJSON
which manipulates the semantically equivalent IResult monad
(Parser is technically more expressive but not for the terms appearing
in GFromJSON). The existing method is left for API compatibility.

The resulting code achieves rougly the same compilation speed but
reaches large performance improvements, for a generated code size
not far from the manually written instances in benchmarks.

The object file for Twitter.Generic grows to 332kB (from 285kB),
but the Twitter.Manual module is 300kB large. The inlining level of
typeMismatch has been reduced to curb code blow-up.

Due to inlining levels differences, the Generic fromJSON is now
often quicker than its TH or Manual counterpart in benchmarks.

Benchmark results (Core i5 Haswell)

	TH	Generic (before)	Generic (after)	Manual
D/fromJSON	1.720 µs	5.076 µs	1.505 µs	-
BigRecord/fromJSON	3.720 µs	8.450 µs	2.964 µs	-
BigProduct/fromJSON	2.412 µs	3.427 µs	1.812 µs	-
BigSum/fromJSON	173 ns	1230 ns	171 ns	-
twitter100	1.611 ms	2.063 ms	1.334 ms	1.516 ms
jp100	1.844 ms	2.18 ms	1.481 ms	1.677 ms

[Lysxia]

That's very cool!

It's unfortunate that gParseJSON is exported in the toplevel modules Data.Aeson/Data.Aeson.Types. Users should really not depend on these Generics internals. It would be a good idea to hide this at some point, if not now, so we can just remove gParseJSON.

[remyoudompheng]

What is the process for contributions ?

[Lysxia]

Just wait for bergmark to look at the PR.

[bergmark]

Thanks for the contribution! And sorry for the delay. I'll try to review this and #652 tonight.

[bergmark]

I suppose this is not a breaking change since the method is not exported and it has a default?

[bergmark]

This looks great to me! As for #652 I was surprised that it didn't make compilation speed a lot worse. If someone else could do a review as well, I'd appreciate it.

It may be that this will cause issues with older GHCs, as with #652, but if so we can consider dropping some support.

Re gparseJson being exported - I wonder if anyone uses it? I can create a separate ticket for that as this change doesn't seem to require a major bump.

[remyoudompheng]

This patch may increase compilation time of records on GHC 7.10 and 8.0 (see #652)

This may be alleviated by adding {-# NOINLINE gParseJSON #-} whenever gParseJSON' is already defined (since the other one becomes unused). Tests on Text.Pandoc.Definition from pandoc-types show 500MB of resident memory reduction out of 1.5-2GB.

[bergmark]

I'm not sure I understand what you mean by "whenever gParseJSON is already defined", Is that something we could do as a part of this PR?

[guaraqe]

The result of Generics here seem even better than the TH ones. What is blocking this PR?

[bergmark]

Sorry for the delay. I had a question in my last comment, and i just internalized this PR as "needs more work" but really I think we should merge this. I'll try to look into the merge conflict some time unless someone beats me to it.

[phadej]

This conflicts, and I'm worried that class now has two-fields. It's most likely affects optimisation as it's no more newtype-like.

After a bit of thought, let us leave this from 1.5 (It's indeed could be fixed in minor version, as we don't export internals).

[arybczak]

I'm worried that class now has two-fields. It's most likely affects optimisation as it's no more newtype-like.

The OP demonstrates a significant runtime performance increase though.

This PR isn't very complicated, might be a good idea to solve conflicts and revisit it. I might do that at some point (and simultaneously look into compile time performance of generic deriving of instances too as it's currently slow).

[runeksvendsen]

Is it possible for me to modify my instances to realize these performance gains, thus avoiding needing to wait for this PR to be merged?

[phadej]

@runeksvendsen don't use aesons Generics. You may use TH. But in general I advice to not use any meta-programming derivation of To/FromJSON instance if you care about performance or stability of your API.

TL;DR The existing Generics in aeson are bad, and are there mostly to not break existing code.

[Lysxia]

You could check out https://hackage.haskell.org/package/deriving-aeson

[runeksvendsen]

You could check out https://hackage.haskell.org/package/deriving-aeson

@Lysxia does this library incorporate the performance improvements in this PR? Or is there another reason you recommend it?

[Lysxia]

I don't know but I would expect that it would be easier to push for those changes if they are still necessary; the library may already naturally be more efficient due to its more modern architecture.