developit / greenlet

🦎 Move an async function into its own thread.
https://npm.im/greenlet
4.67k stars 100 forks source link

Optimize Transferables #16

Closed johnsonjo4531 closed 6 years ago

johnsonjo4531 commented 6 years ago

This pull-request will allow transferable data-types (such as ArrayBuffer, MessagePort and ImageBitmap) to be transferred optimally (zero-copy) to and from the greenlet worker without any change to the api. The only problem with this is that it then makes the Transferable unusable in the main thread (once it is sent), so maybe we should make it an option to transfer it zero-copy to the worker. I don't think any issues will be had transferring zero-copy from the worker back to the main thread, because the worker is done once it returns and wouldn't need the data anymore. If you think you'll accept this pull-request we might want to add tests for transferable types. I could possibly add some, but I also can't get tests to pass on my local machine for some reason.

Sorry this is my first pull-request in open source so I might have done something wrong. I couldn't get tests to pass on my machine even when I had just cloned it and had no changes to the repo, but I don't believe this should break them.

Here is a html5rocks link that explains transferrables more in depth.

johnsonjo4531 commented 6 years ago

Let me know if there is anything I can do to make this a more acceptable pull-request or even if the pull-request itself is in some way unacceptable to the vision of greenlet. I'm fine making changes and working out the idea if you think it could possibly work. I thought about making the the idea for this pull-request an issue first, but I thought it was a small enough feature I could just create a pull-request.

developit commented 6 years ago

I love the idea of automatically handling transferrables. I'd like to see if there are any ways to golf the size down a bit since I think the increase is fairly major right now.

johnsonjo4531 commented 6 years ago

Yeah one part I can see that's might be able to be golfed is I repeated myself by using the same function twice (isTransferrable) I wasn't sure how to decrease that at the time, but maybe since they are being used once in both places (in and out of the worker) we can just copy the code inline in both places.

developit commented 6 years ago

Yup, that was my first thought too!

johnsonjo4531 commented 6 years ago

Okay I think I golfed it enough. Here's a script you can try to check out timing and such for transferables with zero copy https://jsfiddle.net/johnsonjo4531/emeu3xu3/1/. It seems marshalling doesn't take a significant amount of time unless your transferring large buffers between the worker and main UI thread.

zaygraveyard commented 6 years ago

Any chance this will get merged soon?

johnsonjo4531 commented 6 years ago

@developit feel free to let me know if you would like me to make any further adjustments or if there is something you would want me to look into further.

developit commented 6 years ago

I'm going to merge and then optimize - the filter() can be dropped since there is only ever 1 arg returned.