-
Notifications
You must be signed in to change notification settings - Fork 396
Wasm: Implement ArrayDeque without js.Array in Wasm backend #5164
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
base: main
Are you sure you want to change the base?
Conversation
The original implementation of ju.ArrayDeque uses js.Array for its internal data structure. When compiling to Wasm, operations on ju.ArrayDeque require JS interop calls to access the underlying js.Array, which causes a slow performance. This commit introduces an implementation of ju.ArrayDeque for the Wasm backend. This version uses Scala's Array instead of js.Array to avoid JS-interop.
val innerJS = this.innerJS // local copy | ||
val innerWasm = this.innerWasm // local copy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand correctly that, these local copies are for better performance (accessing local copy is faster than class field), right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note]
This is also a problem, let's wait until we decide to use (or not to use) Array[AnyRef]
anyway.
If we don't, we can do something like val innerJS = if (isWebAssembly) null else this.innerJS
, or maybe we can do the same as PriorityQueue
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For ArrayDeque
, it does not seem like we rely all that much on the growability of js.Array
s. The only place that grows is ensureCapacityForAdd()
. It always doubles the size of the internal array. It's unlikely that the underlying VM could do a better job than just reallocating at about the same moments.
Perhaps it would be better to just always use Array[AnyRef]
, even on JS. That would avoid complicating the logic, which in this class is particularly dense already.
@gzm0 WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For comparison, I wrote a Array[AnyRef]
(even on JS) version b73e8a1
And, I wrote a micro-benchmark for ArrayDeque
, and compare the two implementation on JS (scala.Array vs js.Array), each benchmark basically add/poll integer 10000 times.
tanishiking/scalajs-benchmarks@13cd090
The result shows that, js.Array
is still slightly faster than scala.Array
in most cases. However, considering the complexity of the program, the difference may be negligible.
https://colab.research.google.com/drive/17eevbKILkGiT21uqWCDuD3KAkj-de6BT?usp=sharing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were those benchmarks done in fullLink?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was in fastLink
, in fullLink
scala.Array and js.Array has a bit smaller difference in addLast bench
https://colab.research.google.com/drive/17eevbKILkGiT21uqWCDuD3KAkj-de6BT?usp=sharing
it's interesting that, with scala.Array impl, FIFO and LIFO cases (last two benchmarks) are consistently faster than js.Array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the overall sentiment, that just using scala.Array
for JS as well might be worthwhile.
It seems that the investigation here as already gone beyond gut feeling, so I guess we can let the numbers speak.
The current benchmarks do look like there is some overhead not related to resizing. For example, I'd expect peekFirst_Repeatedly
to not cause any mutations. Do we have an understanding of where these come from?
Perhaps it would be better to split it into two PRs for |
249ba0b
to
1ba7624
Compare
I took a liberty of splitting a PR for PriorityQueue #5168 |
Simliar to ju.ArrayList #5162, add a Wasm version of implementation of
ArrayDeque
that doesn't have JS-interop for internal data structure.I wrote a micro-benchmark for
ArrayDeque
, and compare the two implementation on JS (scala.Array vs js.Array), each benchmark basically add/poll integer 10000 times.tanishiking/scalajs-benchmarks@13cd090
In WebAssembly, it works around 6~7x- faster than js.Array impl.
https://colab.research.google.com/drive/17eevbKILkGiT21uqWCDuD3KAkj-de6BT?usp=sharing