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

Missing generated AnyRef specialization for array access #12764

Open
bbrehm opened this issue Apr 4, 2023 · 3 comments
Open

Missing generated AnyRef specialization for array access #12764

bbrehm opened this issue Apr 4, 2023 · 3 comments

Comments

@bbrehm
Copy link

bbrehm commented Apr 4, 2023

Reproduction steps

Consider the following jmh benchmark (on scala 2.13.8 and hotspot openjdk-19, but I believe this applies to all versions except that graal is possibly better than C2 at loop hoisting the checks):

//effectively equivalent to scala standard library array iterator
class AnyIter[@specialized A](a: Array[A]) extends Iterator[A] {
  private[this] var idx = 0
  private[this] val len = a.length

  override def hasNext: Boolean = idx < len
  override def next(): A        = { val r = a(idx); idx += 1; r }
}

class AnyRefIter[A <: AnyRef](a: Array[A]) extends Iterator[A] {
  private[this] var idx = 0
  private[this] val len = a.length

  override def hasNext: Boolean = idx < len

  override def next(): A = {
    val r = a(idx); idx += 1; r
  }
}
@State(Scope.Benchmark)
@OperationsPerInvocation(1000)
class ArrayOps {
  val items = Range(1, 1000).iterator.map { idx => s"${idx}" }.toArray

  @Benchmark
  def scalaIter(bh: Blackhole): Unit = {
    val iter = items.iterator.filter { _ != "aaa" }
    bh.consume(iter)
    for (item <- iter) bh.consume(item)
  }

  @Benchmark
  def anyIter(bh: Blackhole): Unit = {
    val iter = new AnyIter(items).filter { _ != "aaa" }
    bh.consume(iter)
    for (item <- iter) bh.consume(item)
  }

  @Benchmark
  def anyRefIter(bh: Blackhole): Unit = {
    val iter = new AnyRefIter(items).filter { _ != "aaa" }
    bh.consume(iter)
    for (item <- iter) bh.consume(item)
  }
}

This gives with LinuxPerfNorm profiler enabled

Benchmark                                               Mode  Cnt    Score    Error  Units
joernBench.ArrayOps.anyIter                             avgt   21    5.307 ±  0.045  ns/op
joernBench.ArrayOps.anyIter:instructions:u              avgt    3  115.353 ± 10.216   #/op
joernBench.ArrayOps.anyRefIter                          avgt   21    4.689 ±  0.034  ns/op
joernBench.ArrayOps.anyRefIter:instructions:u           avgt    3  104.056 ± 25.334   #/op
joernBench.ArrayOps.scalaIter                           avgt   21    5.397 ±  0.119  ns/op
joernBench.ArrayOps.scalaIter:instructions:u            avgt    3  116.199 ± 21.201   #/op

(the confidence intervals reported by jmh are overly wide in many cases, because it assumes the quite conservative student t distribution. In other words, one needs to very carefully design test parameters with high iteration and fork counts in order to get good confidence intervals, and I did not do that here)

Problem

The problem is that specialization conflates AnyRef with Any. In many cases, there is not a relevant distinction in terms of performance whether we statically know that a type T is T <: AnyRef.

For accessing an Array[T] this makes a big difference, though: Accessing the array is aload if T <: AnyRef, and is a much slower scala.runtime.ScalaRunTime.array_apply(xs: AnyRef, idx: Int): Any.

Ideally, specialization would stop conflating Any and AnyRef.

If that is difficult for technical reasons, then we need to very prominently document this and warn people away from naive use of @specialized. Instead we need to point people to using custom specialization, as the standard library already does in some places like scala.collection.immutable.ArraySeq$ofRef.

Furthermore, we need to take our own bitter medicine and manually specialize performance critical parts. It is hard to imagine more critical code than iteration over arrays, so we definitely need a class scala.collection.ArrayOps$ArrayIteratorOfRef. But we need to look at all uses of @specialized and consider manually specializing the AnyRef case.

@SethTisue SethTisue added this to the Backlog milestone Aug 10, 2023
@SethTisue
Copy link
Member

then we need to very prominently document this and warn people away from naive use of @specialized

I'm not sure where that would go...? In the Scaladoc for @specialized? And/or, one could imagine a feature guide to specialization living at https://docs.scala-lang.org/overviews/index.html , perhaps using the original SID as a guide.

@SethTisue
Copy link
Member

If that is difficult for technical reasons

Nobody is working on Scala 2 specialization; there are many open bugs. Currently @specialized is a no-op in Scala 3, as the design is considered flawed. Threads about what a replacement might look like include https://contributors.scala-lang.org/t/status-of-specialization-in-scala-3/4628 and scala/scala3#17329

@som-snytt
Copy link

specialized means primitives. specialized(AnyRef) used to work in 2.9.2:

  <specialized> class AnyIter$mcT$sp extends AnyIter {
    <paramaccessor> <specialized> protected[this] val a$mcT$sp: Array[java.lang.Object] = _;
    override <specialized> def next(): java.lang.Object = AnyIter$mcT$sp.this.next$mcT$sp();
    override <specialized> def next$mcT$sp(): java.lang.Object = {
      val r: java.lang.Object = AnyIter$mcT$sp.this.a$mcT$sp.apply(AnyIter$mcT$sp.this.AnyIter$$idx);
      AnyIter$mcT$sp.this.AnyIter$$idx = AnyIter$mcT$sp.this.AnyIter$$idx.+(1);
      r
    };
    <specialized> def this(a$mcT$sp: Array[java.lang.Object]): AnyIter$mcT$sp = {
      AnyIter$mcT$sp.this.a$mcT$sp = a$mcT$sp;
      AnyIter$mcT$sp.super.this(a$mcT$sp);
      ()
    }
  };

Scaladoc is unclear.

https://github.com/scala/scala/blob/v2.13.13/src/library/scala/specialized.scala#L31

Oh, there is a test that suggests a workaround:

class Spek[@specialized(Int, AnyRef) A, @specialized(Unit) B](a: Array[A]) extends Test.It[A] {
  private[this] var idx = 0
  private[this] val len = a.length

  override def hasNext: Boolean = idx < len
  override def next(): A        = { val r = a(idx); idx += 1; r }

  //def bomb(): B = null.asInstanceOf[B]
}

where type It[A] = scala.collection.AbstractIterator[A] makes classes that are easier to work with;
and in which the second (unused) dummy specialized parameter forces Spek$mcLV$sp.class

  public A$sp next$mcL$sp();
    flags: ACC_PUBLIC
    Code:
      stack=3, locals=2, args_size=1
         0: aload_0
         1: getfield      #19;                // Field a$mcL$sp:[Ljava/lang/Object;
         4: aload_0
         5: getfield      #23;                // Field Spek$$idx:I
         8: aaload
         9: astore_1
        10: aload_0
        11: aload_0
        12: getfield      #23;                // Field Spek$$idx:I
        15: iconst_1
        16: iadd
        17: putfield      #23;                // Field Spek$$idx:I
        20: aload_1
        21: areturn

warn people away from naive use of @specialized

it could say "beware generalist use of specialized!"

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

3 participants