Allow users to configure asyncify data location

[kkoenig]

Export the 'Asyncify' object to allow users to further
customize behavior

[RReverser]

Thanks for working on this! cc @xtuc who also wanted something like this

One idea I had was to allow the compiled code itself specify different location by declaring public static data array like static uint8_t asyncify_data[1024] + size_t asyncify_size = sizeof(asyncify_data);.

Those would guarantee that space is reserved just for Asyncify stack, doesn't overlap with anything else, and they'd show up as exported numbers in Wasm exports.

Would that work for your use case too?

[kkoenig]

Hi! Longer term that approach makes sense to me and I like the idea of having an option to explicitly reserve stack space (either in code directly or in wasm-ld itself). Its unclear to me what the right direction is there and I don't have a strong opinion.

For my use case I'd like to be able to use wasm-opt to asyncify an existing binary without needing to recompile. Unfortunately this requires a bit of trial and error to pick free space and I think we may as well make it configurable for now

Exporting the Asyncify class itself allows me to build a library that operates on an instantiated WASM module, I'd imagine other implementers will find useful as well

[xtuc]

In addition to @RReverser idea, the asyncify JS code could auto-detect these symbols to work out the data location

[xtuc]

thanks

[RReverser]

Unfortunately this requires a bit of trial and error to pick free space and I think we may as well make it configurable for now

Yeah that's why I'm a bit concerned with configuring this from JS side instead of source code side - there's no way to guess the right address & size that won't corrupt some other memory, unless you reserved it explicitly for Asyncify, or unless you're doing malloc in runtime.

Can you show how you'd choose addresses with this JS approach?

[kkoenig]

Yeah that's why I'm a bit concerned with configuring this from JS side instead of source code side - there's no way to guess the right address & size that won't corrupt some other memory, unless you reserved it explicitly for Asyncify, or unless you're doing malloc in runtime.

Agreed.

Can you show how you'd choose addresses with this JS approach?

No special automatic method, just looking at wasm layout or configuring based on custom wasm-ld settings.

In the current state this library will only work if the hardcoded 16-1024 range is unused, in the short term I think its reasonable to make this configurable so people that know what they are doing (would need to use the Asyncify object directly to configure it) can provide a different range. Otherwise I can't use this library without maintaining a local patch

[RReverser]

Otherwise I can't use this library without maintaining a local patch

TBH I myself am in the same position but wanted to make sure that before I add something configurable, it is something that actually helps in long term; otherwise configuration is very hard to remove :)

I guess malloc usecase or configuring wasm-ld is reasonable enough so we can try to merge it.

I was hoping to avoid JS-based configuration for reasons mentioned earlier and leave definitions to the source code, but, if like in your case, people want to be able to define custom address without recompiling Wasm, I guess there's no other choice.

[RReverser]

I'm not sure how comfortable I am with exporting this class - its methods are definitely not for public consumption, they were meant to be an implementation detail whereas now they will become part of public API.

Instead, could you perhaps add 3rd optional parameter to Instance, instantiate and instantiateStreaming and pass that parameter directly to this constructor?

[kkoenig]

Happy to do that but will no longer be able to use for my use case. An example would be a library that operates on an already instantated WebAssembly module, that automatically asyncifies it before calling one of the exports:
eg. https://nodejs.org/api/wasi.html#wasistartinstance using Asyncify internally

[RReverser]

Hmm why not asyncify in advance? Doing it as part of instantiation makes a bit more sense semantically.

[RReverser]

Ah right, and another part of the reason is that you need to wrap all the imports too. Those can be provided only before the instantiation happened.

[kkoenig]

Not sure where you landed on this, but I'd prefer to expose the Asyncify class and add any "advanced" settings there vs adding additional arguments to the 3 instantiate APIs. My rationale:

• it works for my use case :)
• keeps change smaller
• more flexible for advanced users, in general I would argue anyone configuring the data region explicitly should be comfortable dealing with Asyncify internals

[RReverser]

I think there's a difference between dealing with Asyncify internals and committing to API stability of previously internal Asyncify class. If there's a specific function you need, I'd still lean towards extracting that specific API TBH so that the overall surface to maintain is smaller :)

[RReverser]

@kkoenig Are you planning to come back to this? Would be good to get it over the finish line.

[rkusa]

In case this helps anyone coming to this issue. While not pretty, I am increasing the stack size by manually writing the start and end locations to the Wasm memory. Pro: no change to Asyncify needed, and I have full control of how this memory is allocated (alloc is my own export).

I am doing the following after I've instantiated my Wasm instance:

const STACK_SIZE = 4096;
const DATA_ADDR = 16;
const ptr = await exports.alloc(STACK_SIZE);
new Int32Array(exports.memory.buffer, DATA_ADDR, 2).set([
  ptr,
  ptr + STACK_SIZE,
]);

[kkoenig]

@kkoenig Are you planning to come back to this? Would be good to get it over the finish line.

Yes sorry about the delay - will do another pass before the end of the week, is it just the outstanding comment re: dataBegin + 8? Are you comfortable with exporting the Asyncify class?