facebook/hermes

Add TypedArray support to jsi

sercand opened this issue · 10 comments

JSI does not have the typed array interface. As a result, we have to use ArrayBuffer to pass UInt8Array data to native. Without a TypedArray interface, we can't allocate array buffer once and reuse again on typed arrays for better performance.

iOS 10+ Javascript Core and Hermes supports Typed Arrays and I believe it is plausible to add TypedArray to the JSI.

I am opening this issue here as suggested by @TheSavior in his comment.

With JSI you can create a TypedArray, access its elements and get a native pointer to its contents. What more functionality is needed that is missing?

@tmikov
First, the most important thing is TypedArray currently disabled in ios JSCRuntime. One has to use their own forked JSCRuntime.cpp to use TypedArrays at iOS.

What I want to do is basicly with an example:

// allocate buffer once
const buffer = new ArrayBuffer(1024 * 64); // 64 kb buffer
// reuse buffer n times for a task
for (let i = 0; i < n; i++) {
  const length = requiredSize(i);
  // if buffer don't have enough size reallocate
  let data = new Uint8Array(buffer, offset, length); 
  // write to data
  sendDataToNative(data);
}

but jsi only has very simple Object::getArrayBuffer, ArrayBuffer::length, ArrayBuffer::data
functions for Buffers. But javascript core has really straightforward typed array api.

We have to access required fields with Object::getProperty even for simple things like getting typed array storage type (which is done by checking its string "name" property, if I know correctly). It should be as easy as in JSCore because in some cases TypedArrays becomes hot path in workload.

  • About JSCRuntime missing TypedArray support. Can you submit a PR fixing it to ReactNative? I believe this is an oversight, since the old version of JSC isn't used anymore.
  • About checking if something is a TypedArray: can't you use jsi::Runtime::instanceof()?

Lastly, if I understand you correctly, the problem isn't that JSI doesn't support TypedArray, but that it doesn't support it efficiently enough, and using the JSI API with TypedArray is not very ergonomic.

Fair enough. For the performance part it would really help if we could look at some numbers. A piece of code where accessing TypedfArray via JSI is a performance bottleneck.

About the ergonomic part of the API. What would an improved JSI API look like?

Hi

I would also be interested in adding TypedArray support to jsi.

I believe this is an oversight, since the old version of JSC isn't used anymore.

JSC without TypedArray api was also present on iOS 9, and support for that was dropped recently facebook/react-native@829a223

Fair enough. For the performance part, it would really help if we could look at some numbers. A piece of code where accessing TypedfArray via JSI is a performance bottleneck.

I don't have actual numbers, but our implementation of webgl would be a good example (https://github.com/expo/expo/tree/master/packages/expo-gl-cpp). Performance problems will be most visible for calls that use small TypedArrays, in webgl there is a lot of that(usually called per animation frame) and some of those functions can accept various TypedArray types which will require a lot of instanceof checks.

What would an improved JSI API look like?

This is the implementation we are currently planning to use (for hermes@0.2.1) expo@48c16c7 if you would be interested I would gladly cleanup code and create PR

Hi,
@tmikov I would be grateful for any response to my last comment.

@wkozyra95 sorry, somehow I missed this message. It looks fine to me. @mhorowitz do you think they should move ahead with the PR?

I have commented on the PR.

In #248 we concluded that there wasn't a justification to modify JSI based on efficiency. But, it is still cumbersome to use typed arrays with JSI. We would still welcome a PR to make typed arrays easier to use.

I assumed that you meant creating totally separate library that depends on jsi.

We would still welcome a PR to make typed arrays easier to use.

Ok, I'll prepare new PR. Are you ok with the API (general structure) I proposed in original PR or do you have any suggestions?

I'm closing this since it's been open for several months.
There was a discussion of using JSI with typed arrays in #419, which we also decided not to move forward with.

In that thread, I included some discussion of what kind of API I think would make sense to add to support zero-copy typed array buffer sharing with native code. If someone is interesting in building this, please submit a new PR, or open a new issue to discuss your plans. Thanks!