-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[Bridge] Array Buffers / Typed Arrays for WebSockets, Ajax #1424
Comments
There are two roadblocks here. One is the API limitation you described and one solution would be to work with the WebKit team to patch the API for the next-next release of JSC. The second issue is that the RN bridge uses JSON to communicate between JS and native. There are no shared references so you currently can't access a TypedArray from Obj-C that JS can also access. This could change at the cost of losing the Chrome debugger but if shared memory enables another class of apps that do photo processing, etc, that might be the right trade off. |
We can have a separate |
Even without shared memory, the RN bridge could benefit from binary serialization and transferring ownership of memory. However, given the direction of the language, it might as well just be a shared reference. |
This is also an issue for any kind of Bluetooth related functionality. Perhaps we should document current work-arounds for this? |
FYI: I recently put a lot of time into a workaround for reading/writing Typed Arrays in native code and it seems to perform reasonably well. About 7ms/MB for reading, 20ms/MB for writing. The overhead of the read/write function call itself (regardless of array size) is ~0.2ms. The whole journey is documented here: Final implementation: EJConvertTypedArray.h, EJConvertTypedArray.m |
it looks like this API is finally being added to JSC |
Wow really? You have a link for this? |
Looks like this is resolved and merged? #4483 |
Not quite, it is just a fix for WebSocket. There's more being requested in this ticket. |
I'm writing a react-native-thrift lib, but find out that binary protocol can't be sent over xhr, Typed Arrays will make it work as MDN says, can you continue to work for this feature? |
maybe a relative pr: #6327 |
@springuper I tried to modify RCTNetworking and XMLHttpRequest.js to add basic binary support, it might help. UPDATE: |
Looks like Typed Array support will indeed be in iOS 10: https://developer.apple.com/library/prerelease/content/releasenotes/General/iOS10APIDiffs/Objective-C/JavaScriptCore.html |
@cvermilion Will this let it work on Android as well? |
@saulshanabrook I'm not sure, but I think the Android version uses JavaScriptCore directly, so I would think so. But looking at the ReactAndroid build files it looks like it's using a pretty old commit of JSC (r174650, from 2014), so having TypedArray access might require upgrading JSC on that side? |
I think it does, I was checking it out. Will verify that. |
fetching .gz file in react-native android 0.30 fails with response.arrayBuffer(): fetch('http://127.0.0.1:8125/bonjour.txt.gz', {
mode: 'no-cors'
})
.then(function(response) {
console.log('response %o: ', response);
return response.arrayBuffer();
})
.then(function(buffer) {
console.log('buffer %o: ', buffer);
}) Here is the result from react-native running on my android device (chrome debugger): You can see _blobBlob and _blobInit in the response, as well as arrayBuffer size of 54. It should be 42 as you can see from the correct result from firefox, running on the same android device: firefox android and react-native android run the same javascript code. Why Firefox can properly use arrayBuffer on a fetch() response object, but not react-native? note: I'm trying to download a .gz file as an arrayBuffer to then decompress it using pako. note2: A workaround to my problem is to use react-native-fetch-blob. I would prefer use a simple fetch however :) |
So TypedArray support is now in iOS 10. For earlier versions we could either use the code @phoboslab wrote, or show an unsupported error. On Android we'd need to rebuild android-jsc with a newer version of WebKit, and add it to maven. That would be TypedArray support on the JSC side done. |
We are trying to keep issues focused on bugs and this is more of a feature request, albeit a very cool and valuable feature request. I am going to close this issue but if people are interested in working to improve this then that would be pretty great! |
Is there at least a Product Pain for this? |
I don't see one, you could create a new one! Maybe we should flush out Product Pains every once in a while to keep it fresh.... |
@sebmarkbage how would a shared reference get implemented? seems too low level for JS. @ALL can we take care of the bridge--is-the-bottleneck issue? seems that the JS<->Native interop is the "weakest link" in the RN performance story. |
We are discussing and looking for a way to implement a I wonder if it could be generalized in RN? and if so, what would it mean when in ObjC iOS/Java Android (and not just in C++ support) – would it also mean RN to provide an abstract new type on top of this? |
It's currently not possible to send/received ArrayBuffers with React Native.
Am I right to assume that this is largely due to the missing TypedArray implementation in JSC? I.e. we're not able to transfer raw memory in a sane and performant way between JavaScript and native.
I've opened an issue over at webkit.org (https://bugs.webkit.org/show_bug.cgi?id=120112), asking for a TypedArray API a few years ago and have written a working implementation for this API (https://github.com/phoboslab/JavaScriptCore-iOS/blob/master/JavaScriptCore/JSTypedArray.h) in my fork of JSC. Forking JSC has worked okay-ish for my own library, but now I'm stumbling over this issue again with React Native.
It would be really useful to be able to load and handle binary data for a number of use cases. So I'm wondering if you guys would be interested to weigh in on a TypedArray API or if you have any ideas how this would be workaroundable without the huge performance penalty that we currently face.
The text was updated successfully, but these errors were encountered: