Skip to content

Avoid unnecessary wrapping and array copy when using Array.apply (and potentially List.apply, ...) #502

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
smarter opened this issue Apr 26, 2015 · 6 comments

Comments

@smarter
Copy link
Member

smarter commented Apr 26, 2015

Currently, the following code:

val x = Array(1,2,3)

Is transformed to:

val x: Int[] = Array.apply(1, Predef.wrapIntArray([2,3]))

Which is as inefficient as it looks. scalac has rewriting rules in Cleanup to deal with this: https://github.com/scala/scala/blob/d030172d7ef807d85391d32c5456b1b97b15a402/src/compiler/scala/tools/nsc/transform/CleanUp.scala#L536-L545
We could do something similar, keeping in mind the following:

  • We have to be careful to not break semantics, see SI-6611
  • We'll need more (or better) rewriting rules than scalac to deal with all possible cases, for example scalac optimizes Array(1,2,3) and Array[String]("foo", "bar") but does nothing with Array[Int](1,2,3).
  • I'd like it if the rewriting was done before Erasure, so that it does not need to take into account whatever crazy encoding we'll come up with for arrays of value classes.

The alternative would be to do this and other optimizations like List(1,2,3) using macros, see https://groups.google.com/forum/#!topic/scala-internals/5oRYBYBUqMY/discussion and SI-6247, is this the direction we want to go in?

@DarkDimius
Copy link
Contributor

"early inlining" is one of the things that I want to do in optimizer, to have smaller trees for methods(and less methods, due to cheap DCL). I'm not sure if we want to cope with all this in the compiler itself. But I will indeed lock into this in optimizer.

@smarter
Copy link
Member Author

smarter commented Apr 26, 2015

When you say optimizer do you mean the backend optimizer or a hypothetical link-time optimizer?

@DarkDimius
Copy link
Contributor

Link-time optimizer.
It's not hypothetical. I'll opensource it around ScalaDays.
Though I'm pretty sure I wont get to implement anything from this by than, as currently I'm not doing anything domain-specific.

@DarkDimius
Copy link
Contributor

Also, @inline could be of help here. But for a new standard library.

@retronym
Copy link
Member

class Test {
  val a = Array(1, 2, 3)
  val b = Array[Int](1, 2, 3)
}

Before cleanup:

      Test.this.a = scala.Array.apply(1, scala.this.Predef.wrapIntArray(Array[Int]{2, 3}));
      Test.this.b = scala.Array.apply(scala.this.Predef.wrapIntArray(Array[Int]{1, 2, 3}), (ClassTag.Int(): scala.reflect.ClassTag)).$asInstanceOf[Array[Int]]();

Strictly speaking, cleanup is not be free to rewrite the second case without extra knowledge that the ClassTag argument came from implicit materialization (rather than from implicit search or as an explicitly passed argument). Concretely, these should not be rewritten:

val b1 = Array[Int](1, 2, 3)(null)
val b2 = Array[Int](1, 2, 3)({???; implicitly[reflect.ClassTag[Int]]})

@retronym
Copy link
Member

Augmenting class tag materialization to attach a marker tree annotation to its result might be a robust way to distinguish the cases.

nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Jul 8, 2019
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2019
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2019
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2019
nicolasstucki added a commit to dotty-staging/dotty that referenced this issue Jul 9, 2019
nicolasstucki added a commit that referenced this issue Jul 10, 2019
Fix #502: Optimize `Array.apply([...])` to `[...]`
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