reboot - use of @inline

[mdedetrich]

@rossabaker Has understandable issues in regards to using @inline for the AST, since it makes the API quite brittle (see http://stackoverflow.com/questions/4593710/when-should-i-and-should-i-not-use-scalas-inline-annotation)

Due to this, I have implemented benchmarks to see if @inline is worth it, using https://github.com/scalameter/scalameter (which is a testing framework, that also automatically deals for things like JVM warmup). Long story short, @inline does provide significant performance improvements, which vary depending on which methods are called.

As an example, Method "D" in https://github.com/mdedetrich/json4s-ast/blob/reboot/jvm/src/test/scala/InlineTest.scala, @inline has a performance increase of 38% (which is huge)

Here is a Gist of my latest run (you can just run sbt test to run the test suite) to provide results for your machine. My one is the most powerful version of Macbook Pro from one year ago, so interested to see results on weaker machines.
https://gist.github.com/mdedetrich/86887d2bfa7a3c28e0b9

As it stands, my current position is to leave @inline in there. It actually makes more sense for something like json-ast, since it will be updated incredibly rarely, so brittle API breakage isn't an issue for a library that updates for something like once a year.

Would also be handy to add more tests for less trivial methods, I am sure there are other examples where @inline makes a big impact.

[rossabaker]

i7-3840QM @ 2.8 GHz
java version "1.8.0_40"
https://gist.github.com/rossabaker/13cfd4a9d236cce7bb97

Inline almost never won, but Noinline always got to go second. Microbenchmarking is hard.

[mdedetrich]

I have updated the benchmark suite so its more accurate, it now uses the Microbenchmark which means each test in run in a separate JVM, rather than in the same one SBT is (which provides wild results)

Here are my latest results
https://gist.github.com/mdedetrich/e6b7611696f557fe3d8f

They seem to be neck and neck, in some cases inline won, in other cases inline didn't win. In each case, the margin is around 0.5-5%. This is probably more expected, since scala-meter does account for warmup. If @inline gives any advantage, its that it means that it will be fast before warming up, will have a look to see if you can show the warmup difference with scala-meter and how significant it is. Looks like I will end up removing @inline though

[mdedetrich]

Hmm, it seems that due to scalameter/scalameter#143, I can't test how effective the @inline is

[rossabaker]

jmh is a tool many people like for microbenchmarking.

My results on fce35fb, for whatever they're worth: https://gist.github.com/rossabaker/f66ca9c2e265215bdc2b

I'm not closing the door on this, but my default position is that this annotation is not necessary. The JVM does a reasonable job of optimizing this, and most people find, as we are, that it's hard to measure an improvement being explicit.

[sirthias]

+1 on not using @inline.
The JVM does a really good job with inlining based on the runtime profile, which is exceptionally difficult to quantify in any kind of artificial benchmark.
IMHO the question of what data structures we use for backing the various AST elements is of much more importance to performance than any potential applications of @inline would ever be.

[mdedetrich]

Removing inline now