[wasm] Migrate to ref/out param while reading data from typed array

[maraf]

• Migrate to ref/out param while reading data from typed array in mono_wasm_typed_array_to_array_ref
• The test case doesn't prove the fix, because it doesn't fail without the fix. Could it be related to the publish requirement?

[ghost]

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

---

• Migrate to ref/out param while reading data from typed array in mono_wasm_typed_array_to_array_ref

Author:	maraf
Assignees:	maraf
Labels:	arch-wasm, area-System.Runtime.InteropServices.JavaScript
Milestone:	7.0.0

[kg]

Good catch

[pavelsavara]

[FAIL] System.Runtime.InteropServices.JavaScript.Tests.TypedArrayTests.Uint8ArrayToArray
[13:50:25] info: System.Runtime.InteropServices.JavaScript.JSException : RangeError: Array buffer allocation failed
[13:50:25] info:     at new ArrayBuffer (<anonymous>)
[13:50:25] info:     at eval (eval at t (http://127.0.0.1:33131/dotnet.js:3:44553), <anonymous>:3:23)
[13:50:25] info:     at Object.mono_wasm_invoke_js_with_args_ref (http://127.0.0.1:33131/dotnet.js:3:59772)

[pavelsavara]

Are there other obsolete js functions used internally, which could be replaced with better one ?

[kg]

[FAIL] System.Runtime.InteropServices.JavaScript.Tests.TypedArrayTests.Uint8ArrayToArray
[13:50:25] info: System.Runtime.InteropServices.JavaScript.JSException : RangeError: Array buffer allocation failed
[13:50:25] info:     at new ArrayBuffer (<anonymous>)
[13:50:25] info:     at eval (eval at t (http://127.0.0.1:33131/dotnet.js:3:44553), <anonymous>:3:23)
[13:50:25] info:     at Object.mono_wasm_invoke_js_with_args_ref (http://127.0.0.1:33131/dotnet.js:3:59772)

Interesting, that usually means Node ran out of memory.

[kg]

Are there other obsolete js functions used internally, which could be replaced with better one ?

There are a bunch of them still in the codebase, we should consider deleting them all and fixing whatever touches them. I wouldn't be surprised if it broke some stuff though.

[pavelsavara]

Could we also rename mono_obj_array_new_ref, mono_obj_array_set_ref, conv_string_rooted to *_root to be more consistent ?

TBH: if we are able to land the new interop, I would much rather not extend the scope of this private API and moved all of it to undocumented INTERNAL.

[maraf]

Interesting, that usually means Node ran out of memory.

Yea, I should allocate less in the test. I put there some high numbers, because I wasn't able to make the test fail before the fix. But @pavelsavara suggested some changes.

[kg]

Could we also rename mono_obj_array_new_ref, mono_obj_array_set_ref, conv_string_rooted to *_root to be more consistent ?

TBH: if we are able to land the new interop, I would much rather not extend the scope of this private API and moved all of it to undocumented INTERNAL.

I don't consider it expanding the scope, since we're just replacing broken APIs with fixed ones. If you're proposing that we delete the API I understand the reasoning but I think we would need to do that in 8

[maraf]

Could we also rename mono_obj_array_new_ref, mono_obj_array_set_ref, conv_string_rooted to *_root to be more consistent ?

I think the names are correct

• conv_string_rooted is already remapped to conv_string_root and marked as obsolete,
• mono_obj_array_new_ref and mono_obj_array_set_ref takes MonoObjectRef, not WasmRoot

[pavelsavara]

the caller has to get WasmRoot instance first by calling mono_wasm_new_root_buffer and then call get_address on it and only then they could call mono_obj_array_new_ref with it. I think we could do that on behalf of the user and return WasmRoot instance from mono_obj_array_new_root for him.

[maraf]

the caller has to get WasmRoot instance first by calling mono_wasm_new_root_buffer and then call get_address on it and only then they could call mono_obj_array_new_ref with it. I think we could do that on behalf of the user and return WasmRoot instance from mono_obj_array_new_root for him.

I'm not opposed refactoring the API in a new PR.

[maraf]

/azp run runtime-wasm

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[maraf]

/azp run runtime-wasm

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).