Wasm-friendly implementation of varargs. #5215
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.