Skip to content
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

Support any iterable for incoming slices and vectors #4206

Open
RunDevelopment opened this issue Oct 16, 2024 · 2 comments
Open

Support any iterable for incoming slices and vectors #4206

RunDevelopment opened this issue Oct 16, 2024 · 2 comments

Comments

@RunDevelopment
Copy link
Contributor

Motivation

Right now, incoming slices and vectors are typed as T[] or UInt8Array and similar for primitives. This is unnecessarily restrictive.

Even today, all function taking slices and vectors already support array-like objects (anything with a length that can be indexed). So just by changing the type definition, exported Rust functions would be easier to use on the JS side, but we can go even further.

Proposed Solution

Support arbitrary iterables in the JS glue code.

The basic idea is that we can support any iterable by just converting the input argument with Array.from, UInt8Array.from, etc. to the desired array type. Obviously, we don't always have to do this conversion. If the argument is already in the desired shape, we can skip the conversion. If the argument is in a different but similar-ish shape, we can also define a custom code path for it.

However, this also allows us to be more efficient than users could be. The prime example here is Set. Sets are not array-like, but they do have a size property, which fulfills essentially the same function. This means that we can skip the conversion to an array by having custom code path for Set.

To illustrate what I mean, here is how I would imagine the glue code for taking a Vec<Foo> where Foo is user-defined Rust struct with #[wasm_bindgen].

#[wasm_bindgen]
pub fn drop_it(array: Vec<Foo>) {}
function passArrayJsValueToWasm0(array, malloc) {
    // a single check for non-array types to keep the happy path fast
    if (!Array.isArray(array)) {
        if (array instanceof Set) {
            // Set is common enough to warrant special path to avoid the conversion
            const ptr = malloc(array.size * 4, 4) >>> 0;
            const mem = getDataViewMemory0();
            let i = 0;
            for (const item of array) {
                mem.setUint32(ptr + 4 * i++, addHeapObject(item), true);
            }
            WASM_VECTOR_LEN = array.size;
            return ptr;
        }

        // just convert to an array
        array = Array.from(array);
    }

    const ptr = malloc(array.length * 4, 4) >>> 0;
    const mem = getDataViewMemory0();
    for (let i = 0; i < array.length; i++) {
        mem.setUint32(ptr + 4 * i, addHeapObject(array[i]), true);
    }
    WASM_VECTOR_LEN = array.length;
    return ptr;
}

/**
 * @param {ArrayLike<Foo> | Iterable<Foo>} array
 */
export function drop_it(array) {
    const ptr0 = passArrayJsValueToWasm0(array, wasm.__wbindgen_malloc);
    const len0 = WASM_VECTOR_LEN;
    wasm.drop_it(ptr0, len0);
}

Please keep in mind that this is just PoC code that I threw together in 5 minutes.

Alternatives

The main alternative would to be only adjust the generated type definition to reflect what is actually accepted. This is already an improvement, but falls short of being convenient to use on the JS side.

Additional Context

I'm willing to make a PR for this.

@daxpedda
Copy link
Collaborator

This sounds perfectly fine to me and I'm happy to review a PR.
I would appreciate if you could link all the necessary resources as well, e.g. how ArrayLike is defined and such.

@RunDevelopment
Copy link
Contributor Author

This sounds perfectly fine to me and I'm happy to review a PR.

Great, then I'll make one next week.

I would appreciate if you could link all the necessary resources as well, e.g. how ArrayLike is defined and such.

Sorry, I forgot about that.

ArrayLike<T> is essentially any object with a length property that can be indexed with numbers. So it's similar to readonly T[], but doesn't require any array methods (like e.g. map). I proposed ArrayLike<T>, because that's what the set method of typed arrays uses (please ignore the doc comment, the array doesn't just accept arrays and typed arrays; the type is exactly what the ECMAScript spec uses), and because our conversion for Rust structs doesn't need anything more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants