Skip to content

Decide on default value for -Xmixin-force-forwarders in 2.12.0 #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lrytz opened this issue Sep 21, 2016 · 3 comments
Closed

Decide on default value for -Xmixin-force-forwarders in 2.12.0 #231

lrytz opened this issue Sep 21, 2016 · 3 comments
Labels
Milestone

Comments

@lrytz
Copy link
Member

lrytz commented Sep 21, 2016

Ticket to make sure it doesn't get forgotten.

Jason's latest benchmark shows ~10% improvement in cold performance, none when warm scala/scala#5390 (comment).

It seems that changing the value should be binary compatible, in which case we could change the default (in either direction) in a minor release.

Should think more about and test binary compatibility

  • Is the signature of a forwarder always the same as the callee?
  • Any special cases with bridges? Specialization?
@lrytz lrytz added the task label Sep 21, 2016
@lrytz lrytz added this to the 2.12.0-RC2 milestone Sep 21, 2016
@lrytz
Copy link
Member Author

lrytz commented Sep 28, 2016

We decided to set the default value to true for 2.12.0.

Still to do: run compiler benchmarks combinations of

@retronym
Copy link
Member

retronym commented Sep 28, 2016

Benchmarking with static accessors enabled:

⚡ git log head --not origin/2.12.0 --oneline  --graph
* b68c7c2 Emit all mixin forwarders when building Scala
* c2bd533 Bump to prerelease SBT
* 01fb73c Bump sbt.version to 0.13.12, without breaking
* e08b88f Make isSeparateCompiled... robust against rogue phase time travel
* 95234e7 SD-226 Be lazier in Fields info transform for better performance
* b28bc8c Cast more pro-actively in synthetic accessor trees.
* 733aaf4 Avoid mismatched symbols in fields phase
[info] Benchmark                            (_scalaVersion)  (extraArgs)  (source)  Mode  Cnt     Score     Error  Units
[info] ColdScalacBenchmark.compile  2.12.0-b68c7c2-SNAPSHOT                 scalap    ss   16  6583.898 ± 133.473  ms/op
[info] ColdScalacBenchmark.compile  2.12.0-c2bd533-SNAPSHOT                 scalap    ss   16  7664.687 ± 578.979  ms/op
[info] Benchmark                            (_scalaVersion)  (extraArgs)                               (source)  Mode  Cnt      Score      Error  Units
[info] ColdScalacBenchmark.compile  2.12.0-b68c7c2-SNAPSHOT               @/code/scala/sandbox/args-compile.txt    ss    8  37457.935 ± 1532.913  ms/op
[info] ColdScalacBenchmark.compile  2.12.0-c2bd533-SNAPSHOT               @/code/scala/sandbox/args-compile.txt    ss    8  38863.001 ± 1388.303  ms/op

10-15% speedups for cold performance, similar to our previous measurements.

Hot performance seems very close for scalap and better-files. Using src/compiler as another benchmark, I saw an improvement mixin forwarders, but I think that I was observing a difference in warmup speed, moreso than a difference in peak performance.

I don't currently have a good way to quantify warmup speed. Would be good to gather kloc/s compile data for 0-5s, 5-10s etc and compare. But it is hard to fit large codebases (e.g. src/compiler) into these buckets.

@adriaanm adriaanm added the has-pr label Oct 5, 2016
@adriaanm
Copy link
Contributor

scala/scala#5429

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants