Enumeration improvement proposition: avoid unnecessary synchronized block

[Paul Lysak]

Hi guys

I was digging in internals of Scala enums and noticed that every call to enum value's toString() hits synchronization on enum's singleton object:

Val.toString() -> Enumeration.nameOf() -> synchronized { ... } - and the purpose of synchronization is to initialize a mutable map, which is only initialized once per enum. I thought it might have sense to use such tools as lazy val and immutable map to avoid synchronization overhead. 

Here is commit to forked repository with proposed improvement: https://github.com/paul-lysak/scala/commit/10f58fdbbff79c8aa54c29b3bda3e4cfccba407a.

I've done some performance estimates with a simple test: https://gist.github.com/paul-lysak/59e54e3f811cc216bea8

On my machine it shows:

with 10 mln iterations and one thread:

original: +total: wait= 454, time=448
with fix: +total: wait= 411, time=405

So just a small improvement here.

with 10 mln iterations and 3 threads:

original: +total: wait= 3008, time=8736

with fix: +total: wait= 574, time=1604

Here lock-free implementation is 6 times faster.

I understand that this case is rather specific and test might be not very thorough, but IMHO lock on global resource just to retrieve String representation of enum isn't a right way to go and eventually might be bad for performance.

Would such improvement be considered for merging? 

If yes - then I would need some advice on this because of following issues:

1. When I tried to apply this fix on 2.11.x branch I've got false alarm about binary incompatibility because private Scala methods and fields after compilation end up as public methods in byte-code and compatibility check tool looks only at byte-code. I've described the case in comment of related issue https://issues.scala-lang.org/browse/SI-6882 and considered that it's better to target the improvement to 2.12
2. But tests for freshly fetched branch 2.12.x didn't run on my machine, neither on jdk 7, nor on jdk 8 - just as described in https://issues.scala-lang.org/browse/SI-9077, all though Scala 2.12 is promised to run with Java 8 only (http://www.scala-lang.org/news/2.12-roadmap)

Which steps would you suggest to make further?

Thanks!

[Nils Kilden-Pedersen]

Seems to me a better approach would be to make sure Val always has a name that’s not null. This will avoid both the synchronization and any map lookup.

--
You received this message because you are subscribed to the Google Groups "scala-internals" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-interna...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

​

[Paul Lysak]

 @Nils, do you have an idea how to make it possible without requiring everyone to specify name explicitly for each enum Val? 

Enumeration relies on reflection API to find out what was the name of the method containing Val and use it as name of the Val - and this information isn't available until val is assigned proper value.