Emscripten: FETCH=1 + USE_PTHREADS=1 is not fully supported by wasm backend

[sbc100]

This currently generates and error message: FETCH+USE_PTHREADS not yet compatible with wasm (shared.make_fetch_worker is asm.js-specific)

I guess we need to port shared.make_fetch_worker to wasm?

[kripken]

I think so, yes.

It may make sense to just rewrite that code: right now it mixes handwritten JS with some code copied from the asm.js build (for malloc etc.), but I think it could just be written in C and compiled normally (using EM_ASM or EM_JS for the JS bits).

Alternatively, the current handwritten JS could be kept as-is, and instead of copying code from the asm.js build we could compile a tiny C program to provide the compiled stuff it needs. The current handwritten JS could turn into JS library code for that C program.

cc @juj

[joextodd]

A similar error is generated without USE_PTHREADS=1

FETCH not yet compatible with wasm (shared.make_fetch_worker is asm.js-specific)

[sbc100]

@joextodd I believe that issues was fixed in https://github.com/kripken/emscripten/pull/7010

[sbc100]

I've been looking into this a little. I don't think we can have malloc work correctly in the worker without having the exact same malloc function that the wasm module provides. This means that we will most likely need the entire wasm module on the worker... which makes me think maybe the fetch worker should be more like a pthread that the system starts.

For now, I will reduce the error to warning, since the worker is only needed for blocking calls, and not all programs require this.

[sbc100]

I imagine the blocker to making the fetch worker into the pthread is that it can be used when PROXY_TO_WORKER is enabled without necessarily having pthreads enabled. This leads me to a more general question which is, now that we have USE_PTHREADS and PROXY_TO_PTHREAD is there still much value in maintaining a separate PROXY_TO_WORKER? If we would remove PROZY_TO_WORKER then the fetch workers could become a pthread itself (I think).

Thoughts? @juj

[juj]

Personally from my perspective we could deprecate PROXY_TO_WORKER, but I do not know if there are current users of the feature that do care about that.

Though we could deprecate use of -s FETCH=1 in PROXY_TO_WORKER mode, I do not know if that combination has any uses?

Fetch worker could sensibly migrate to being a pthread for wasm.

[VirtualTim]

~This is fixed now, right?~
Thought this was about fastcomp WASM, but it is about the WASM backend. Never mind.

[sbc100]

Is it? I think the fetch worker still depended on the asmjs output

[VirtualTim]

Oh, I was basing my comment more on the title. I'm compiling with -s USE_PTHREADS=1 -s PTHREAD_POOL_SIZE=8 -s WASM=1 -s FETCH=1 without issue. Apart from streaming fetches not working on non-Firefox.

I'm setting up my fetch using attr.attributes = EMSCRIPTEN_FETCH_LOAD_TO_MEMORY | EMSCRIPTEN_FETCH_SYNCHRONOUS | EMSCRIPTEN_FETCH_REPLACE;, if that makes a difference.

[sbc100]

When you did you build you should have seen this warning:

"Bug/TODO: Blocking calls to the fetch API do not currently work under WASM backend (https://github.com/emscripten-core/emscripten/issues/7024)"

As far as I know that warning is still valid and so is this bug. We could re-title to be more clear if you like?

[VirtualTim]

Oh, the WASM backend. I though this was about the Fastcomp backend with -s WASM=1.
I thought this was the issue fixed by #8077.
Maybe this issue should be renamed?

[kripken]

After the last PR landed, sync xhrs work in pthreads with the wasm backend and fetch - it does it directly without the worker being involved.

We should still keep this bug open for the remaining things, like IDB fetches, that perhaps someone will want to implement.

I think this is enough to remove this from the list of blockers for the wasm backend by default, removing there.

[malytomas]

I would like to ask what is the status on this? Is it planned to support asynchronous/waitable fetches on wasm+pthreads?
I currently have 20 threads each handling one fetch at a time and it feels rather suboptimal. I would like to have just one thread managing all the fetches together.
Thanks

[femski]

Lack of async fetch on WASM pthreads is a showstopper for us too. For example when you are panning and zooming a map - a lot of tile requests/fetches get canceled. No async fetch means we can not cancel unnecessary fetches - results in a huge performance drop.

[kripken]

Async fetches should work, also in pthreads, unless I'm missing something? As discussed above, the last missing piece is sync fetches and IDB fetches.