fosskers / scalaz-and-cats

Usage examples and benchmarks between Scalaz and Cats (w/ Haskell ground-truth).
489 stars 28 forks source link

monkey patch with scalaz.IList #4

Closed fommil closed 6 years ago

fommil commented 6 years ago
> bench/jmh:run -i 15 -wi 15 -f1 -t10 .*ClassScalaz

before
[info] EqualBench.equalDiffClassScalaz  avgt   15  3910.586 ± 55.692  ns/op
[info] EqualBench.equalSameClassScalaz  avgt   15  3913.138 ± 65.819  ns/op

after
[info] Benchmark                         Mode  Cnt        Score       Error  Units
[info] EqualBench.equalDiffClassScalaz  avgt   15  2953.313 ± 39.848  ns/op
[info] EqualBench.equalSameClassScalaz  avgt   15    43.534 ±  0.594  ns/op

c.f. cats
[info] EqualBench.equalDiffClassCats  avgt   15  7193.795 ± 117.522  ns/op
[info] EqualBench.equalSameClassCats  avgt   15    10.836 ±   0.152  ns/op
fommil commented 6 years ago

I'm interested in why cats appears to have a 10ns constant overhead, whereas scalaz has 40ns. Probably syntax related.

I did a memory allocation capture of the "not the same list" case and this is what yourkit says...

"Class","Objects","Shallow Size","Retained Size"
"scalaz.Equal$$anon$4 sun.misc.Launcher$AppClassLoader","4352679","69642864","69642864"
"scalaz.IListInstance0$$anon$4 sun.misc.Launcher$AppClassLoader","4352658","104463792","104463904"
"scalaz.syntax.EqualOps sun.misc.Launcher$AppClassLoader","4352653","104463672","104463672"
"scala.collection.immutable.$colon$colon sun.misc.Launcher$AppClassLoader","59940","1438560","302360152"
"scalaz.ICons sun.misc.Launcher$AppClassLoader","39960","959040","302360152"
"java.lang.Integer","35347","565552","565552"
"char[]","12074","953320","953320"

which does look like we're paying the syntax and ListEqual creation tax.

Incidentally, this whole problem of having to create a fresh implicit instance when there are type parameters is something I think we can solve with caching in https://gitlab.com/fommil/scalaz-deriving/issues/35

fommil commented 6 years ago

investigating the syntax tax by changing to

  def equalAll[A: Equal](l0: IList[A], l1: IList[A]): Boolean = Equal[IList[A]].equal(l0, l1)

gives

[info] Benchmark                        Mode  Cnt     Score    Error  Units
[info] EqualBench.equalDiffClassScalaz  avgt   15  2969.303 ± 66.155  ns/op
[info] EqualBench.equalSameClassScalaz  avgt   15    27.123 ±  0.325  ns/op

and homing in on the SameClassScalaz memory allocation we see that the problem is then that an scalaz.Equal$$anon$4 is created by each invocation, defined as

Warning: Binary file Equal$$anon$4 contains scalaz.Equal$$anon$4
Compiled from "Equal.scala"
public final class scalaz.Equal$$anon$4 implements scalaz.syntax.EqualSyntax<F> {
  public scalaz.syntax.EqualOps<F> ToEqualOps(F);
    ...

A closer look at Equal reveals this treasure

  val equalSyntax = new scalaz.syntax.EqualSyntax[F] { def F = Equal.this }

converting that into a def gives (regression for DiffClass)

[info] EqualBench.equalDiffClassScalaz  avgt   15  3265.151 ± 33.237  ns/op
[info] EqualBench.equalSameClassScalaz  avgt   15    17.344 ±  0.310  ns/op

or as lazy val (i.e. no better)

[info] Benchmark                        Mode  Cnt     Score    Error  Units
[info] EqualBench.equalDiffClassScalaz  avgt   15  3260.199 ± 74.312  ns/op
[info] EqualBench.equalSameClassScalaz  avgt   15    17.618 ±  0.260  ns/op

so all in all maybe it's best to keep the val but perhaps something to think about for scalaz8.

fommil commented 6 years ago

One more optimisation... by putting the instance based shortcut in the syntax we cover ourselves.

[info] EqualBench.equalDiffClassScalaz  avgt   15  2960.812 ± 38.178  ns/op
[info] EqualBench.equalSameClassScalaz  avgt   15    43.449 ±  0.550  ns/op
fosskers commented 6 years ago

Should this PR be left often for a bit for further experimentation?

fommil commented 6 years ago

I think it served it's purpose already :-) merge if you're happy

fommil commented 6 years ago

when new scalaz is released, we can delete the monkey patch

fosskers commented 6 years ago

master was updated to use the new ScalaZ, and at the same time I borrow the changes here manually (modulo the auto-plugin, I wasn't convinced).

From my own tests, it looks like only the while case with scalaz-deriving is acting funky. Otherwise, scalaz-deriving totally keeps up! The next step would be to do tests on a deeply nested ADT, like you suggested.