feat: emnapi_create_memory_view

[toyobayashi]

#16

There is no way to reset detached TypedArray.prototype.buffer to newly growed wasm memory ArrayBuffer.

[RReverser]

Hm this looks a bit unfortunate.

I was thinking that instead you could store the original TypedArray constructor, pointer, and length associated with the view via WeakMap.

Then, you could change the signature of emnapi_sync_memory to something like:

function emnapi_sync_memory(env: napi_env, arraybuffer_or_view: napi_value, js_to_wasm: boolean): napi_value;

That is, it would 1) simplify the API somewhat - user only needs the JS view object and emnapi does the rest under the hood and 2) when it's possible to update data in-place, it would return the same napi_value, and, when not, it would simply return a new instance of the same view.

This would make updates more or less transparent to the user, even if not in-place in the same object.

[toyobayashi]

Another problem is that, if the view created from HEAPU8.buffer is not the unique object (can be re-create by emnapi_sync_memory), how to deal with finalizers associated with the view could be a bit complex.

Imagine we created a Uint8Array with finalizer by emnapi_create_external_uint8array, later the wasm memory grew, then use emnapi_sync_memory created a new Uint8Array, and the old view is garbage collected, but the new view is still being used, then it's a bug.

[RReverser]

Good point. That can be relatively easily worked around though. So I mentioned these couple of things:

the original TypedArray constructor, pointer, and length associated with the view via WeakMap.

If you store them as an object { ctor: ..., ptr: ..., length: ... } in said WeakMap, then you can

1. pin any finalizers to this object of properties instead of the view itself

and

2. associate the new view with this object of properties after the growth

That way, any current view will be always associated with the same object of properties, and any finalizers will be executed only when the last view together with this object goes away.

[toyobayashi]

Done. the signature of emnapi_sync_memory is

declare function emnapiSyncMemory<T extends ArrayBuffer | ArrayBufferView> (
  jsToWasm: boolean,
  arrayBufferOrView: T,
  byteOffset?: number,
  data?: number,
  length?: number
): T

napi_status emnapi_sync_memory(napi_env env,
                               bool js_to_wasm,
                               napi_value arraybuffer_or_view,
                               size_t byte_offset,
                               void* data,
                               size_t length,
                               napi_value* result);

I prefer to keep them optional.

Any other suggestions?

[RReverser]

Any other suggestions?

Couple of minor ones:

1. I understand wanting to have optional byteOffset and length in the function - I suppose for updating only part of the view instead of making the more expensive update-everything copy - but why data?
   I don't think pointer will ever change and it seems somewhat dangerous to allow updates from a view created at one memory location to a Wasm memory at a different memory location. If user really wants that for some reason, at that point it's just a generic HEAPU8.set(...) call rather than "sync".
2. In the C function, given that you already store the result via napi_value* result, I'd suggest merging it together with napi_value arraybuffer_or_view argument. That is, the function could accept just napi_value* arraybuffer_or_view that updates the handle to the new value. After resizing, the old handle is invalidated and not useful anymore anyway and this makes the API tiny bit cleaner.

[RReverser]

This looks a bit complicated. I think what I'd do instead is check if user requested data pointer.

If they didn't, then we can create regular JS Buffer object and not bother with malloc, finalizers and so on.

If they did request data, then 1) malloc + zero memory like you already did here and 2) delegate the rest to _napi_create_external_buffer(...).

That way you don't need custom handling or separate WeakMap.

[toyobayashi]

emnapi_get_memory_address needs the separate buffer weakmap

[RReverser]

Why can't it use the same info it uses for other TypedArrays? As in, Buffer is just a typed array (it's a subclass of Uint8Array), so it can store the same descriptor in the same WeakMap as for all other typed arrays, and the return address will be just byteOffset like for other typed arrays.

[toyobayashi]

In addition to telling the user the address of the pointer, emnapi_get_memory_address also needs to tell the user who manages the memory and whether it is allocated by emnapi, so that users can manually free memory in an environment without FinalizationRegistry.

Why can't it use the same info it uses for other TypedArrays?

This question make me aware of another problem.

napi_create_external_buffer and emnapi_create_memory_view do not know whether the data is allocated by user or emnapi, so the buffer created by napi_create_buffer need to the new WeakMap to save this info, currently views created by emnapi_create_memory_view always treated as managed by the user.

[toyobayashi]

I removed the buffer WeakMap, but I have no idea how to solve the problem mentioned before.

views created by emnapi_create_memory_view always treated as managed by the user.

user may create an ArrayBuffer by using napi_create_arraybuffer, then using emnapi_create_memory_view or napi_create_external_buffer to create a view from this arraybuffer, pass the view to emnapi_get_memory_address will return wrong ownership and runtime_allocated.

[RReverser]

Hm, isn't it enough to check if the ArrayBuffer / ArrayBufferView is in the WeakMap? If it is, then you know it's one created by emnapi, if it's not, then it's a user object and you can skip any memory sync & memory mapping.

[RReverser]

That is, I think it's reasonable to document that emnapi_get_memory_address and similar functions just won't work on JS-land ArrayBuffers, but all the napi_* functions should.

[toyobayashi]

Not all ArrayBuffer / ArrayBufferView in WeakMap is allocated by emnapi, for example, napi_create_external_arraybuffer is also don't know where the data is from, but the created arraybuffer should be in the WeakMap because it needs sync.

I will find my spare time to think about how to write document related with arraybuffer and views.

[RReverser]

One thing I'm trying to figure out: what if the user just wants to re-attach a view after the memory growth, but not copy any data in any direction, what would the call look like? Something like this?

view = emnapiSyncMemory(false, view, 0, 0);

[toyobayashi]

what would the call look like

emnapi/packages/test/emnapitest/emnapi.test.js

Lines 21 to 22 in 7c6ee1e

	test_typedarray.GrowMemory()
	externalResult = promise.Module.emnapiSyncMemory(false, externalResult)

Module.emnapiSyncMemory(false, view, 0, -1) // or omit offset and length
emnapi_sync_memory(false, view_napi_value, 0, NAPI_AUTO_LENGTH)

In fact it will do unnecessary copy...

[RReverser]

In fact it will do unnecessary copy...

Yeah that's why I thought that maybe passing 0 would be better? Not sure.