asyncify icon indicating copy to clipboard operation
asyncify copied to clipboard

Allow users to configure asyncify data location

Open kkoenig opened this issue 2 years ago • 9 comments

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

kkoenig avatar Nov 16 '21 03:11 kkoenig

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?

RReverser avatar Nov 16 '21 10:11 RReverser

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

kkoenig avatar Nov 16 '21 16:11 kkoenig

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

xtuc avatar Nov 16 '21 16:11 xtuc

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?

RReverser avatar Nov 16 '21 18:11 RReverser

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

kkoenig avatar Nov 16 '21 19:11 kkoenig

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 avatar Nov 16 '21 20:11 RReverser

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

RReverser avatar Jan 24 '22 00:01 RReverser

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,
]);

rkusa avatar Jan 24 '22 22:01 rkusa

@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?

kkoenig avatar Jan 28 '22 01:01 kkoenig