SI-1448 Unify casting behavior for AnyVals

[gzm0]

Attention: This is an alternative PR to #3295. Do NOT merge both.

Casts of AnyVals had inconsitent behavior depending on whether they were boxed:

1L.asInstanceOf[Int]        // -> 1
(1L: Any).asInstanceOf[Int] // -> ClassCastException

This PR fixes this by adapting the unboxing helpers in BoxesRunTime. A couple of caveats:

• Unboxing requires an additional instanceof check. (Reason: Casting of Character, which is not subtype of Number).
• A wrong ClassCastException is raised on a failed cast to a primitive numeric value. It says "cannot cast to Number" instead of Integer etc.

Where the latter issue can easily be fixed by adding another instanceof check, the former is inherent to the fact that java.lang.Character does not extend java.lang.Number but conversion must be performed. It can only be fixed by integrating (part) of this behaviour in the compiler.

Maybe we should reconsider changing the behaviour the other way around. It is easy to emit a deprecation warning in 2.11 for casts that convert (implemented in PR #3295) and put the behaviour behind a flag in 2.12.

All partests except for the instrumentation tests succeed on this PR.

[xeno-by]

review @gkossakowski, I guess

[Ichoran]

I an uncomfortable with adding extra logic in very low-level and sometimes very hot routines without benchmarks demonstrating that it is okay.

[gkossakowski]

@Ichoran: I share you sentiment and that's why hesitated to decide between those two PRs. In retrospect, I believe that proper benchmarking is on the person proposing the change. I just don't have spare cycles to setup tests myself.

[adriaanm]

Based on the discussion at the twin PR, this is the way to go in principle, but the performance impact has to be studied carefully. Thus, I think this cannot be considered for M8 anymore.

[gkossakowski]

I changed the milestone so this doesn't hold 2.11.0-M8 release.

[gkossakowski]

We've got a nice race condition in last two comments. :)

[gzm0]

@odersky Continuing from your comment in PR #3295

It would be inacceptable to make boxing conversions more expensive than they are now.

I don't think we can incur the cost of an additional isInstanceOf check only when explicitly casting. Given the following example:

def foo[T](x: Int) = {
  val y = x.asInstanceOf[T]
  y
}

since T is erased, we cannot perform the cast on x when it is assigned to y, so currently the cast is performed when y is unboxed. If we want to avoid the additional isInstanceOf when unboxing, but still allow for the conversion to happen, we'd have to find such cases through some form static analysis.

One could imagine to propagate an tag-like of construct with the generic type to check, whether it needs normal or "converting" unboxing. However, this would greatly complicate some part in the compiler (erasure?) and I very much doubt it is worth the effort.

I really don't see how we can resolve this without checking on every unboxing :(

[soc]

I guess looking at benchmarks will be the only thing which enables a decision. I'd expect that VMs with good optimizers can get rid of it almost completely, but that's only an idea.

[xeno-by]

ping everyone

[Ichoran]

I'll write some tests.

[gkossakowski]

@lchoran: what's the ETA for the results of your benchmarking? I'm thinking about pushing this to 2.11.1.

[Ichoran]

@gkossakowski - This evening or Wednesday morning, SF time,

[Ichoran]

@gkossakowski - Looks like I'm liable to slip by a few hours. This is next on my to-do list, but list changes are proving surprisingly thorny, with lots of unexpected interactions. Wednesday afternoon is probable.

[gkossakowski]

No worries. Thanks for the update!

[Ichoran]

Testing with

import ichi.bench._

object Simple{ def main(args: Array[String]) {
  val th = ichi.bench.Thyme.warmed()
  val v = Vector.tabulate(1024)(identity)
  th.pbench{ v.sum }
  th.pbenchOff(){ v.sum }{ (0 /: v){ _ + _ } }
}}

for 20 replicates of each version of the library gives the following results:

Times of existing operations
  Sum alone:   8.1 us
  Sum (vs):   11.0 us
  Fold (vs):  12.7 us

Times with new unboxing
  Sum alone:  10.5 us   (29% slowdown, p < 0.03)
  Sum (vs):   11.8 us   ( 7% slowdown, p < 0.03)
  Fold (vs):  14.2 us   (12% slowdown, p < 0.03)

I doubt this is acceptable. Granted, boxed numbers aren't so fast anyway, but this is enough worse to notice (on an already bad problem). Maybe this can be revisited when optimization is better, but I'd be very wary merging it now.

(I could do more exhaustive tests, but these are sufficiently discouraging that I'm not sure it's worth the time.)

[Ichoran]

@gkossakowski - Tests are in. Forgot to at-you on the full post.

[adriaanm]

Ok, sadly we'll have to delay this then. Thanks @gzm0 and @Ichoran.