Wasm-friendly implementation of varargs. #5215
Conversation
|
@tanishiking Could you check that this PR achieves the same performance improvements on Wasm that you have measured before? For JS, the resulting code is basically not affected. (there are more intermediate variables in non-minify, but they are removed in minify mode) |
|
Sure! It looks this makes Wasm varargs faster without de-optimizing JS 👍
|
For JavaScript, using Scala arrays wrapped in `WrappedArray` (2.12) or `ArraySeq` (2.13) is not ideal for performance. Instead, since about forever, we have re-emitted them as JS arrays wrapped in our own `js.WrappedArray`/`WrappedVarArgs`. This, however, has poor performance on Wasm. We now call generic methods to choose the best implementation of a varargs Seq, depending on the platform. They take Scala arrays, and "convert" them to JS arrays if necessary. The conversions are intrinsified so that they are zero-cost. For this to work, we add an `InlineArrayReplacement` in the optimizer, similar to the existing `InlineJSArrayReplacement`. These changes revert the `toString()` of varargs seqs to what it is on the JVM, but only on Wasm. This breaks three partests, for which we had checkfiles with the Scala.js-specific strings. We extend our `partest` fork to recognize platform-specific checkfiles in order to fix this issue.
8c8cb84 to
e7fb94b
Compare
| result | ||
| } | ||
|
|
||
| @noinline def genericArrayToJSArray[T](array: Array[T]): js.Array[T] = |
There was a problem hiding this comment.
[note]
These scala.Array to js.Array conversions are optimized to JSArrayConstr, so there shouldn't be runtime js.Array construct in optimized pass 👍
| case ArrayToJSArray => | ||
| val tarray = targs.head | ||
| tarray match { | ||
| case PreTransLocalDef(LocalDef(_, /* mutable = */ false, replacement: InlineArrayReplacement)) => |
There was a problem hiding this comment.
Just out of curiosity, does't it works to just replace xxxArrayToJSArray(ArrayValue(...)) with JSArrayConstr(...), in this specific case?
It won't work with the following case without Replacement, and can't optimize other Array related operations though.
val vararg = Array(1,2,3)
foo(varar: _*)
def foo(x: Int*) = ...Anyway, InlineArrayReplacement looks good 👍
There was a problem hiding this comment.
No, that wouldn't really work in our optimizer architecture. We would have to identify a method target before pretransforming its arguments, and that goes against some other decisions that we make (like deciding to inline because of properties of the pretransformed arguments).
For JavaScript, using Scala arrays wrapped in
WrappedArray(2.12) orArraySeq(2.13) is not ideal for performance. Instead, since about forever, we have re-emitted them as JS arrays wrapped in our ownjs.WrappedArray/WrappedVarArgs. This, however, has poor performance on Wasm.We now call generic methods to choose the best implementation of a varargs Seq, depending on the platform. They take Scala arrays, and "convert" them to JS arrays if necessary. The conversions are intrinsified so that they are zero-cost.
For this to work, we add an
InlineArrayReplacementin the optimizer, similar to the existingInlineJSArrayReplacement.