Skip to content
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

Class specialization with specialized method does not generate all specialized possibilities. #5281

Closed
scabug opened this issue Dec 5, 2011 · 22 comments

Comments

@scabug
Copy link

scabug commented Dec 5, 2011

Given a class that is specialized on a type parameter, T and a method specialized on a type parameter
W, the compiler does not generate the cross-product of methods required to avoid boxing and unboxing. When the type parameter is moved from the class to the method, all of the variations are created.

The is demonstrated in the following two gists:
Boxing occurs on case Function1[T,W] but not Function1[T,T]
Type parameter in class definition AND method.
https://gist.github.com/1425438

No boxing, type parameter moved to method from class.
https://gist.github.com/1434304

In the later case, we see from byte code that we will be calling the specialized Function1 apply methods,
so no boxing/unboxing will occur. I would expect that the same should happen in the first example.

@scabug
Copy link
Author

scabug commented Dec 5, 2011

Imported From: https://issues.scala-lang.org/browse/SI-5281?orig=1
Reporter: Marc Millstone (splittingfield)
Affected Versions: 2.9.1

@scabug
Copy link
Author

scabug commented Dec 5, 2011

@paulp said:
Please don't link to gists; please don't supply test code which is not self-contained; please don't include a bunch of extraneous detail.

After that, it looks like this.

class ClassMethod[@specialized T](x: T) {
  def m1[@specialized W](f: T => W): W = f(x)
  def m2(f: T => T): T = f(x)
}

class ClassClass[@specialized T, @specialized W](x: T) {
  def m3(f: T => W): W = f(x)
  def m4(f: T => T): T = f(x)
}
// ClassClass$mcII$sp
public int m3(scala.Function1);
   0:	aload_0
   1:	aload_1
   2:	invokevirtual	#13; //Method m3$mcII$sp:(Lscala/Function1;)I
   5:	ireturn

public int m3$mcII$sp(scala.Function1);
   0:	aload_1
   1:	aload_0
   2:	getfield	#22; //Field x$mcI$sp:I
   5:	invokeinterface	#28,  2; //InterfaceMethod scala/Function1.apply$mcII$sp:(I)I
   10:	ireturn

// ClassMethod$mcI$sp
public java.lang.Object m1(scala.Function1);
   0:	aload_0
   1:	aload_1
   2:	invokevirtual	#13; //Method m1$mcI$sp:(Lscala/Function1;)Ljava/lang/Object;
   5:	areturn

public java.lang.Object m1$mcI$sp(scala.Function1);
   0:	aload_1
   1:	aload_0
   2:	getfield	#22; //Field x$mcI$sp:I
   5:	invokestatic	#28; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
   8:	invokeinterface	#34,  2; //InterfaceMethod scala/Function1.apply:(Ljava/lang/Object;)Ljava/lang/Object;
   13:	areturn

@scabug
Copy link
Author

scabug commented Dec 5, 2011

Marc Millstone (splittingfield) said:
I apologize, as this is my first posting after being prompted from twitter.

This example still does not specialize on the return type

class Blah[@specialized T] {
  def blah[@specialized W](f:T=>W,x:T):W  = {
    f(x)
}
}

@scabug
Copy link
Author

scabug commented Dec 5, 2011

Marc Millstone (splittingfield) said (edited on Dec 5, 2011 10:32:39 PM UTC):
Nor does this

class Blah[@specialized T](a:Array[T]) {
  def blah[@specialized W](f:T=>W):W  = {
    f(a(0))
}
}

@scabug
Copy link
Author

scabug commented Dec 5, 2011

Marc Millstone (splittingfield) said:
This does generate the unboxed specialized methods

class Blah2[@specialized T,@specialized W](a:Array[T]) {
  def blah(f:T=>W):W  = {
    f(a(0))
}
}

@scabug
Copy link
Author

scabug commented Dec 6, 2011

Marc Millstone (splittingfield) said:
I have been looking into this further.

Simplifying to only specialize on Int, this code does not generate a blah method that is to int, only to java.lang.Object.

And as I said above, if the variable to act on is in the method definition, we see the same thing.

class Blah[@specialized (Int) T](a:T) {
  def blah[@specialized (Int) W](f:T=>W):W  = f(a) 
}
public java.lang.Object blah(scala.Function1, scala.reflect.Manifest);
  Code:
   Stack=3, Locals=3, Args_size=3
   0:	aload_0
   1:	aload_1
   2:	aload_2
   3:	invokevirtual	#17; //Method blah$mcI$sp:(Lscala/Function1;Lscala/reflect/Manifest;)Ljava/lang/Object;
   6:	areturn
  LineNumberTable: 
   line 65: 0

  LocalVariableTable: 
   Start  Length  Slot  Name   Signature
   0      7      0    this       Lmarc/junk/Blah$mcI$sp;
   0      7      1    f       Lscala/Function1;
   0      7      2    evidence$16       Lscala/reflect/Manifest;

  Signature: length = 0x2
   00 18 

public java.lang.Object blah$mcI$sp(scala.Function1, scala.reflect.Manifest);
  Code:
   Stack=2, Locals=3, Args_size=3
   0:	aload_1
   1:	aload_0
   2:	getfield	#26; //Field a$mcI$sp:I
   5:	invokestatic	#32; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
   8:	invokeinterface	#38,  2; //InterfaceMethod scala/Function1.apply:(Ljava/lang/Object;)Ljava/lang/Object;
   13:	areturn
  LineNumberTable: 
   line 66: 0

  LocalVariableTable: 
   Start  Length  Slot  Name   Signature
   0      14      0    this       Lmarc/junk/Blah$mcI$sp;
   0      14      1    f       Lscala/Function1;
   0      14      2    evidence$16       Lscala/reflect/Manifest;

  Signature: length = 0x2
   00 18 

public marc.junk.Blah$mcI$sp(int, scala.reflect.Manifest);
  Code:
   Stack=3, Locals=3, Args_size=3
   0:	aload_0
   1:	iload_1
   2:	putfield	#26; //Field a$mcI$sp:I
   5:	aload_0
   6:	aload_2
   7:	putfield	#42; //Field evidence$15:Lscala/reflect/Manifest;
   10:	aload_0
   11:	iload_1
   12:	invokestatic	#32; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
   15:	aload_2
   16:	invokespecial	#47; //Method marc/junk/Blah."<init>":(Ljava/lang/Object;Lscala/reflect/Manifest;)V
   19:	return

@scabug
Copy link
Author

scabug commented Dec 6, 2011

Marc Millstone (splittingfield) said:
Adding something like this though, will generate the necessary specialization.

class Blah[@specialized (Int) T:Manifest](a:T) {
  def blah[@specialized (Int) S>:T, @specialized (Int)  W:Manifest](f:S=>W):W  =f(a) 
}
public int blah$mIIc$sp(scala.Function1, scala.reflect.Manifest);
  Code:
   Stack=2, Locals=3, Args_size=3
   0:	aload_1
   1:	aload_0
   2:	getfield	#26; //Field a$mcI$sp:I
   5:	invokeinterface	#44,  2; //InterfaceMethod scala/Function1.apply$mcII$sp:(I)I
   10:	ireturn

@scabug
Copy link
Author

scabug commented Feb 10, 2012

@dragos said:
It seems you're looking at the wrong place. I'll take the example reduced by Paul, and show that all specialization are performed correctly (unless I'm missing something). I added client call to test if specialization is correctly performed all across the board.

class ClassMethod[@specialized(Int) T](x: T) {
  def m1[@specialized(Int) W](f: T => W): W = f(x)
  def m2(f: T => T): T = f(x)
}

class Test {
  def test() {
    val x = new ClassMethod(1)
    x.m1(x => x + 1)
  }
}

Printing trees after specialization, Test.test looks like this:

def test(): Unit = {
      val x: ClassMethod[Int] = new ClassMethod$mcI$sp(1);
      {
        x.m1$mIcI$sp({
          @SerialVersionUID(0) final <synthetic> class $anonfun extends scala.runtime.AbstractFunction1$mcII$sp with Serializable {
            def this(): anonymous class $anonfun = {
              $anonfun.super.this();
              ()
            };
            final def apply(x: Int): Int = $anonfun.this.apply$mcII$sp(x);
            <specialized> def apply$mcII$sp(v1: Int): Int = v1.+(1)
          };
          (new anonymous class $anonfun(): (Int) => Int)
        });
        ()
      }
    }

Looks good to me!

You are looking at m1$mcI$sp, which is a specialization based only on class parameters. In order to hit the sweet spot, you need to be in a specialized case for both the method and class type parameters. That variant is called m1$mIcI$sp (the encoding of specialized methods is 'm' followed by instantiations for method type parameters, followed by 'c' and instantiations of the class type parameters). You were looking at m1$mcI, but that one can not call the int variant of m1, since it still has the type parameter W (you don't see it in bytecode, that's why I suggest you use -Xprint:spec):

class ClassMethod[@specialized(scala.Int) T >: Nothing <: Any] extends java.lang.Object with ScalaObject {
    <paramaccessor> protected[this] val x: T = _;
    def this(x: T): ClassMethod[T] = {
      ClassMethod.super.this();
      ()
    };
    def m1[@specialized(scala.Int) W >: Nothing <: Any](f: (T) => W): W = f.apply(ClassMethod.this.x);
    def m2(f: (T) => T): T = f.apply(ClassMethod.this.x);
    <specialized> def m1$mcI$sp[@specialized(scala.Int) W >: Nothing <: Any](f: (Int) => W): W = ClassMethod.this.m1[W](f.asInstanceOf[(T) => W]()).asInstanceOf[W]();
    <specialized> def m1$mIc$sp(f: (T) => Int): Int = f.apply(ClassMethod.this.x);
    <specialized> def m1$mIcI$sp(f: (Int) => Int): Int = ClassMethod.this.m1$mIc$sp(f.asInstanceOf[(T) => Int]());
    <specialized> def m2$mcI$sp(f: (Int) => Int): Int = ClassMethod.this.m2(f.asInstanceOf[(T) => T]()).asInstanceOf[Int]()
  };

@scabug
Copy link
Author

scabug commented Feb 20, 2012

@non said (edited on Feb 20, 2012 4:54:41 PM UTC):
Hi Iulian,

If you look at the bytecode of what you pasted, you'll see that it almost works but not quite.

The problem is that only ClassMethod implements the m1$mIcI$sp method, which if you read its implementation in bytecode immediately calls m1$mIc$sp. This method access "x" via the Object field, gets an Object back from Function1.apply, and then unboxes it to an int to return. So... you're right that this bug's title is wrong: all specialized possibilities are generated (in some sense) but not overridden/used correctly.

I think if ClassMethod$mcI$sp actually overrode m1$mIcI$sp then things might work. In fact, this is an issue I've been talking with Vlad about.

@scabug
Copy link
Author

scabug commented Feb 21, 2012

@adriaanm said:
Tentatively assigned to you, Erik. Feel free to reassign to Vlad or to Scala Reviewer.

@scabug
Copy link
Author

scabug commented Feb 21, 2012

@non said:
This is fine, thanks Adriaan!

@scabug
Copy link
Author

scabug commented Feb 27, 2012

@non said:
Good news! It seems like this is now working properly.

The weird part is that when I commented on the 20th it was not working. It's possible that since then other work has "fixed" this, or maybe my analysis was wrong. I'm going to try to build an earlier version to see which it is, as submit a test case to "prove" that this is working.

@scabug
Copy link
Author

scabug commented Feb 27, 2012

@non said:
So, I was a bit optimistic. Here's a pretty robust test case that proves that it is working, except in the case of AnyRef specialization:

import scala.{specialized => spec}

class D[@spec T, @spec W](t:T, w:W)

object D {
  def get[@spec T, @spec W](t:T, w:W) = {
    println(new D[T, W](t, w).getClass.getName)
    w
  }
}

class C[@spec T](x: T) {
  def m[@spec W](f: T => W): W = f(x)
}

object Test {
  // annotated all the types to work around SI-5526
  def main(args:Array[String]) {
    println(new D[Int, Int](1, 1).getClass.getName)
    println(new D[Int, String](1, "a").getClass.getName)
    println(new D[String, Int]("a", 1).getClass.getName)
    println(new D[String, String]("a", "a").getClass.getName)
    println(new D[Int, Double](1, 2.0).getClass.getName)

    // this output should be the same as above
    new C[Int](1).m[Int]((x:Int) => D.get[Int, Int](x, 1))
    new C[Int](1).m[String]((x:Int) => D.get[Int, String](x, "a"))
    new C[String]("a").m[Int]((x:String) => D.get[String, Int](x, 1))
    new C[String]("a").m[String]((x:String) => D.get[String, String](x, "a"))
    new C[Int](1).m[Double]((x:Int) => D.get[Int, Double](x, 2.0))
  }
}

The output I get is:

D$mcII$sp
D$mcIL$sp
D$mcLI$sp
D
D$mcID$sp
D$mcII$sp
D
D
D
D$mcID$sp

The top 5 lines are correct, but the bottom 5 should be the same as the top, which they are except in the IL and LI cases. It seems that AnyRef specialization is working when invoked directly (in cases 2 and 3) but not when invoked indirectly (cases 7 and 8).

I think I may close this bug (since it seems to be working in the case that Marc originally posted) and open a new one dealing with this AnyRef behavior.

@scabug
Copy link
Author

scabug commented Feb 27, 2012

@non said:
Actually, please ignore that whole last series of comments. The code I posted isn't measuring boxing at all, and I was right to say boxing is happening.

Argh! Back to the bytecode, I guess.

@scabug
Copy link
Author

scabug commented Mar 5, 2012

@non said:
Because my last comments are so confused, I will restate the problem:

C$mcI$sp doesn't have an implmentation of m$mIcI$sp, so it inherits C's implmentation, which uses boxing on T and W.

The basic problem seems to be that needsSpecialization doesn't ever get called with an env containing T and W: the envs are either Map(T -> Int) or Map(W -> Int) but not both. This means that all needsSpecialization calls involving m$mIcI$sp return false.

I think (hope?) if we asked about the env Map(T -> Int, W -> Int) it would return true.

@scabug
Copy link
Author

scabug commented Aug 14, 2012

@non said:
This is fixed in 2.10.x, I believe by 97ae6f6.

I tested a version of Marc's original example:

class M[@specialized(Int) T](x: T) {
  def m1[@specialized(Int) W](f: T => W): W = f(x)
  def m2(f: T => T): T = f(x)
}

class C[@specialized(Int) T, @specialized(Int) W](x: T) {
  def m3(f: T => W): W = f(x)
  def m4(f: T => T): T = f(x)
}

object Test {
  def aaa = new M[Int](33).m1((z:Int) => z + 1)
  def bbb = new M[Int](34).m2((z:Int) => z + 2)
  def ccc = new C[Int, Int](35).m3((z:Int) => z + 3)
  def ddd = new C[Int, Int](36).m4((z:Int) => z + 4)
}

For method invocations, I saw:

aaa: M$mcI$sp.m1$mIcI$sp // class specialized on T=I, method on W=I as well
bbb: M$mcI$sp.m2$mcI$sp // class specialized on I only, method on same T
ccc: C$mcII$sp.m3$mcII$sp // class specialized on T=I, W=I, method on same
ddd: C$mcII$sp.m4$mcI$sp // class specialized on T=I, W=I, method only on T

This looks like what is expected.

If someone else wants to doublecheck that would be great. Otherwise I'll close since I'm the assignee.

@scabug
Copy link
Author

scabug commented Aug 14, 2012

Adam Klein (aklein-at-novus.com) said:
I also now get expected specialization behavior for my use case, using 2.10.x branch. Thanks!

@scabug
Copy link
Author

scabug commented Aug 14, 2012

@non said:
This is fixed in 2.10.x, I believe by 97ae6f6.

@scabug scabug closed this as completed Aug 14, 2012
@scabug
Copy link
Author

scabug commented Aug 14, 2012

Adam Klein (aklein-at-novus.com) said (edited on Aug 14, 2012 3:45:52 PM UTC):
This might need to go under a new issue (existing issue?), but the compiler blows up using a combination of specialization & tailrec on 2.10.x:

trait Iter[@specialized(Int) A] {
  def hasNext: Boolean
  def next(): A
  def foldLeft[@specialized(Int) Y](init: Y)(f: (Y, A) => Y): Y = {
    @tailrec
    def fold(init: Y): Y =
      if (!hasNext)
        init
      else
        fold(f(init, next()))
    fold(init)
  }

//  // Works:
//  def foldLeft[@specialized(Int) Y](init: Y)(f: (Y, A) => Y): Y = {
//    var acc = init
//    while(hasNext) {
//      val a = next()
//      acc = f(acc, a)
//    }
//    acc
//  }
}

@scabug
Copy link
Author

scabug commented Aug 14, 2012

@non said:
You didn't post your crash. Does it seem to relate to this bug? #5788

I saw this awhile ago but it was marked fixed.

I'll try to look at this soon. In the meantime, if it isn't already in the bug system please open a new bug.

@scabug
Copy link
Author

scabug commented Aug 14, 2012

@non said:
Also, please try to most minimized code examples of bugs (i.e. the smallest piece of code that demonstrates it).

It's much easier for someone else to understand, and also to be sure the issue is where you think it is. For instance, it looks like you're posting profiling code, but it's easier for me to verify proper tail call optimization just looking at (simple) bytecode.

@scabug
Copy link
Author

scabug commented Aug 14, 2012

Adam Klein (aklein-at-novus.com) said (edited on Aug 14, 2012 3:45:08 PM UTC):
Ok, no problem. It does look like a duplicate of #5788. In fact,

trait Foo[@specialized(Int) A] {
  final def bar(a:A):A = bar(a)
}

Does seem to result in the same issue.

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

No branches or pull requests

2 participants