boa icon indicating copy to clipboard operation
boa copied to clipboard

More efficient conversion of Vec to JsArray

Open lastmjs opened this issue 2 years ago • 6 comments

I just want to bring this up and get some preliminary feedback. My project Azle is a TS/JS environment for the Internet Computer, a Wasm environment. It is relatively resource-constrained compared with other cloud solutions at the moment, and all computation is metered with a unit called cycles. Each RPC call into a canister has a cycle limit.

Users will need to deal with large arrays of Vec<u8> to deal with raw binary data. Right now it seems like the conversion between Vec<u8> and JsArray (to get the user's raw binary data into the canister in the RPC call) is extremely inefficient, insomuch that the cycles limit on the computation is reached and the call fails. I am reaching the limit on arrays of length between 10,000 and 25,000 (~10-20kb).

What can be done to make the conversion more efficient? And btw I am not 100% it's Boa's conversion that is causing the problem, I think my code can be optimized to some extent. But I fear the BigO of the JsArray code (like from_iter) will just be insufficient. What can be done if this is the case? Is there a possibility of just sharing a memory location with a boa context so that we do not need to iterate through entire arrays to convert them over?

lastmjs avatar May 03 '22 17:05 lastmjs

The thing is that a JS Array is an array of values of any type, which means that every value in the Rust array must be converted to a JS value. I don't think there is much room for improvement here (maybe pre-allocation, which we might be already doing)

But, the ideal JS type for a Vec<u8> is a TypedArray, which uses a Rust Vec backend to be stored. So, I would say that the next step should be to implement easy conversion from Rust Vec to TypedArray.

Razican avatar May 04 '22 05:05 Razican

Awesome, I will experiment with that next, thanks!

lastmjs avatar May 06 '22 16:05 lastmjs

I'm ready to tackle this, can you show me some code for what is currently possible converting a Vec<u8> into a JsUint8Array and vice versa? If it isn't currently possible to do this without iterating over every item, I can work on a pull request to do it efficiently.

lastmjs avatar Jun 18 '22 15:06 lastmjs

Most of this work would be done here: https://github.com/boa-dev/boa/blob/main/boa_engine/src/object/jstypedarray.rs

The idea is to add another From implementation in the JsTypedArrayType! macro (which should probably be renamed for Rust standard naming convention, in snake_case).

This From would receive a Vec<$element>, and create a $name object. For this, it will need to first create an ArrayBuffer like this:

let my_vector: Vec<u8> = vec![];
let array_buffer = boa_engine::builtins::array_buffer::ArrayBuffer {
  array_buffer_data: Some(my_vector),
  array_buffer_byte_length: 8,
  array_buffer_detach_key: JsValue::undefined()
};
let array_buffer_js_value: JsValue = array_buffer.into();

It might be useful to have a JsArrayBuffer API too, and create the From constructor from that API object.

Then, you can create the internal JsObject with JsObject::from_proto_and_data. This should solve issues of some of the information here being private to boa_engine.

The last part, of converting a JsUint8Array (or any other sub-type) to a Rust Vec can be trickier. The issue here is that the Vec is referenced from multiple places (that's why we need the garbage collector). But this also means that you cannot just "move" out of it.

You can use Option::take() to take the vector, but this would then make the array unusable in JavaScript. You can also clone it out, but honestly, I would leave that to the user.

I think the best approach here is to have a 3-method interface: take(), borrow() and borrow_mut(). The first would take the vector, moving it, without any performance penalty, but would actually change the the JS structure (and therefore break it). It's still useful in case the context won't run anything else, which is a valid use case. The borrow() and borrow_mut() could just return a reference to the vector, making it possible to clone() it if needed, or they can actually modify it from Rust, if they select to do that, and make the changes available in JS. This would call to the Gc APIs, and could panic if it's borrowed elsewhere.

Razican avatar Jun 20 '22 09:06 Razican

@Razican Can you really implement a borrow() and borrow_mut() for array_buffer_data? I had been playing around at how an API wrapper might work for ArrayBuffer for fun when I saw the above comment. I thought I'd try to add those methods but struggled to get it to work. Ultimately, I ended up with the below.

https://github.com/nekevss/boa/blob/feature/impl-dataview-arraybuffer-wrappers/boa_engine/src/object/jsarraybuffer.rs

nekevss avatar Jun 21 '22 19:06 nekevss

I've attempted to follow your guidance here but run into a major problem early on. Here's what I'm trying to add to https://github.com/boa-dev/boa/blob/main/boa_engine/src/object/jstypedarray.rs:

impl From<Vec<$element>> for $name {
    #[inline]
    fn from(o: Vec<$element>) -> Self {
        let o_len = o.len();

        let array_buffer = crate::builtins::array_buffer::ArrayBuffer {
            array_buffer_data: Some(o),
            array_buffer_byte_length: o_len,
            array_buffer_detach_key: JsValue::undefined()
        };            

        let uint8_array_js_object = crate::object::JsObject::from_proto_and_data(
            Context::default().intrinsics().constructors().$constructor_object().prototype(),
            crate::object::ObjectData {
                kind: crate::object::ObjectKind::ArrayBuffer(array_buffer),
                internal_methods: &crate::object::internal_methods::ORDINARY_INTERNAL_METHODS
            }
        );

        // TODO not finished here
    }
}

The problem is that array_buffer_data must be a Vec<u8>, and I'm not sure how to proceed with converting all of the different types of Vec<numberType> to a Vec<u8>, and based on our conversations in the Discord this might not be feasible.

Here's some of the conversation from Discord:

Person 1:

Do you have suggestions for how the best way to convert a Vec<u16>, Vec<u32> etc into an ArrayBuffer?

Person 2:

Hm, that's a bit more complicated
Because JS only knows about u8 buffer arrays, the other typed arrays are just bit reinterpretations of the same buffer
Could be possible if you can transmute from Vec<u8> to Vec<numType> safely
Hm, doesn't seem like a transmute is possible
The alternative would be to change the definition of ArrayBuffer to take an Option<NumVec>, where NumVec is an enum containing every possible Vec<numType>
But that's a lot more work which involves rewriting a lot of the internals of ArrayBuffer

lastmjs avatar Jun 21 '22 22:06 lastmjs

I think this was solved in #2170, right? Can you confirm @lastmjs ?

Razican avatar Oct 10 '22 12:10 Razican

I haven't tested it yet, our solution is still a bit hacky. But this looks like it would probably solve the problem. I plan to incorporate this eventually.

lastmjs avatar Oct 11 '22 18:10 lastmjs

I will close this, since the solution seems to be there. If anyone finds that this is still a problem, feel free to comment here, and we can re-open it.

Razican avatar Oct 18 '22 14:10 Razican