-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Major improvements to wasm-bindgen-futures #1760
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
Conversation
3dd58d6
to
52fbaca
Compare
(The build failures don't seem related to this PR, except for the |
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.
Very nice! Does indeed read quite well and seems simple enough!
1da3472
to
7d95f60
Compare
7d95f60
to
2e348b1
Compare
(@alexcrichton I fixed all your feedback btw) |
I pushed up some stylistic feedback and tweaks that I figured would be easiest to go ahead and do myself. I debugged the issue with anyref and I think we'll need to fix that somehow before this lands. Looks like
We have to call |
Yeah, that's tricky. Can we somehow special-case |
Yeah that seems reasonable to me! |
@Pauan have you had a chance to take a look at this again? Do you want some help in fixing importing |
@alexcrichton I did take a look, but the anyref optimizations are over my head, so some help would be appreciated. |
This came up during rustwasm#1760 where `Promise.resolve` must be invoked with `this` as the `Promise` object, but we were erroneously importing it in such a way that it didn't have a shim and `this` was `undefined`.
Sure thing, I've posted #1795 to fix imports of |
This came up during #1760 where `Promise.resolve` must be invoked with `this` as the `Promise` object, but we were erroneously importing it in such a way that it didn't have a shim and `this` was `undefined`.
👍 All green now! |
Sorry I'm late, I struggled with the multi-threading atomic stuff.
This PR contains a few major improvements:
Code duplication has been removed.
Everything has been refactored so that the implementation is much easier to understand.
future_to_promise
is now implemented withspawn_local
rather than the other way around (this meansspawn_local
is faster since it doesn't need to create an unneededPromise
).Both the single threaded and multi threaded executors have been rewritten from scratch:
They only create 1-2 allocations in Rust per Task, and all of the allocations happen when the Task is created.
The singlethreaded executor creates 1 Promise per tick, rather than 1 Promise per tick per Task.
Both executors do not create
Closure
s during polling, instead all neededClosure
s are created ahead of time.Both executors now have correct behavior with regard to spurious wakeups and waking up during the call to
poll
.Both executors cache the
Waker
so it doesn't need to be recreated all the time.I believe both executors are now optimal in terms of both Rust and JS performance.