Closed josephrocca closed 3 years ago
Published as 1.0.3.
@josephrocca Now that I think about it a bit more... does this actually work for you? I'd expect it to fail because SAB requires COOP/COEP, and those still enforce same-origin restrictions.
@RReverser Hmm, I don't know the detauls of the COOP/COEP headers, but it seems to work fine for me 🤷
Contents of index.html:
<script type="module">
let decode = (await import("https://deno.land/x/wasm_image_decoder@v0.0.4/mod.js")).default;
let buf = await fetch("https://i.imgur.com/LYVUrUf.jpg", {referrer:""}).then(r => r.arrayBuffer());
let result = decode(buf);
console.log(result);
</script>
then serve with COOP/COEP headers:
deno run --allow-net --allow-read=. https://raw.githubusercontent.com/josephrocca/denoSimpleStatic/master/main.ts --port=3001
and it logs the image data to the console with no warnings/errors in Chrome. You can see in the source that v0.0.4 is using the fetch/blob approach.
If I run the above code without the COOP/COEP headers I get a warning in the console:
[Deprecation] SharedArrayBuffer will require cross-origin isolation as of M91, around May 2021.
On this explainer page for COOP/COEP it says:
These headers instruct the browser to block loading of resources or iframes which haven't opted into being loaded by cross-origin documents, and prevent cross-origin windows from directly interacting with your document. This also means those resources being loaded cross-origin require opt-ins.
What do they mean by "resources which haven't opted into being loaded by cross-origin documents"? deno.land doesn't serve the COOP/COEP headers, but it does serve with access-control-allow-origin: *
. Maybe they're talking about this header when they say "opt-in"? My first counter-thought was: "Well, you always need access-control-allow-origin: *
, so they can't have meant that." But of course there are some resources (e.g. images/scripts) which can usually be loaded cross-origin without that header, and so I'm guessing that the above-quoted paragraph is talking about those ones?
I mean... if you don't have those warnings when serving main HTML with COOP/COEP, then I guess it's fine, but I'm not entirely sure how those rules work or why it's accepted 😅
Just to be on the safe side, I'd also check in Firefox which is already stricter regarding COOP/COEP headers and implements the restrictions mentioned in the warning in Chrome.
I did try to test with Firefox, but it turned out to be a bit of a pain, since it doesn't support top-level await yet, which is used in the module. I downloaded nightly and enabled it, but then it complains about the export
in workerHelpers.no-bundler.js
- saying that export
can only be used at the top-level (even though it is already at the top-level) :/
I downloaded nightly and enabled it, but then it complains about the export in workerHelpers.no-bundler.js - saying that export can only be used at the top-level (even though it is already at the top-level) :/
Oh yeah, it's because it doesn't support module workers yet. See https://twitter.com/RReverser/status/1384837796159008770.
Or TL;DR - include https://www.npmjs.com/package/module-workers-polyfill too for polyfilling those (note that you'll have to download it locally, since it suffers from the same issue as you fixed in this PR due to Worker being hosted on a different domain).
Okay, I've done that and now I get this error:
Some other feature missing in Firefox?
Oh, I thought you said you've already fixed it, but, apparently, that was someone else. See https://github.com/GoogleChromeLabs/wasm-bindgen-rayon/issues/1 for details (it's also mentioned in the README).
Most notably, even when you're using threads, the main thread still can't be blocked while waiting for the Rayon pool to get ready or for all the calculations to finish. You need to instantiate the main JS+Wasm in a dedicated Worker to avoid blocking the UI altogether.
Oh wow, so you need to init everything in a web worker and create your own glue to talk to the main thread? If that understanding is correct, wouldn't it make sense for this glue to be automatically generated? Or maybe throw a more helpful error message and/or put that note in a more prominent place in the readme? I might have misunderstood something here. This is certaintly surprising to me and I'd expect it to surprise other users too. I guess ideally it would detect whether it's being initialised on the main thread and create a worker for itself if so.
If my understanding is correct and you'd like to keep the init code as-is, then I think it would be a good idea to put all this critical setup stuff in a single "setting up" section of the readme.
Interestingly firefox has the "waiting is now allowed on this thread" when using a local webserver, but silently fails when serving from glitch:
This is certaintly surprising to me and I'd expect it to surprise other users too.
It's definitely surprising and definitely something Chrome needs to fix for consistent and clear error message across the board, I've pinged the relevant internal bug again.
As for the glue - in principle, it's possible to do something here with wasm-bindgen-futures
, but it won't be as simple as "detect whether it's being initialised on the main thread and create a worker for itself if so" because all your exports etc. still need to be proxied to the main thread, and now become asynchronous too and so on. Some manual changes will still be required.
More generally, the problem is that any mutex or similar blocking mechanism will attempt to block the current thread (by definition) but on the Web it was decided not to allow that, since blocking UI even for relatively short period of time degrades user experience significantly.
I'm working on something that might improve situation with glue code further in the future here, but for now, yeah, you just need to put Wasm in a Worker and send final results to the main thread. In most usecases it's not too hard to do that - e.g. see the demo code https://github.com/GoogleChromeLabs/wasm-bindgen-rayon/blob/main/demo/wasm-worker.js.
it won't be as simple as "detect whether it's being initialised on the main thread and create a worker for itself if so" because all your exports etc. still need to be proxied to the main thread, and now become asynchronous too and so on
Ah, I see!
In most usecases it's not too hard to do that
I think in a lot of cases it'd be a bit more complicated, because you may want to expose the full API surface of the wasm module to the main thread, and so you'd have to write a bunch of plumbing code to deal with that - probably involving tracking task IDs so that you know which main-thread promises to resolve when you recieve messages back from the worker. But maybe most users will tend to do everything in the worker and only send data back to the main thread as necessary.
Looks like there are some libraries to help deal with this sort of problem to some extent. It would be cool if there were a JS library like this except that it neatly exposes a module from the worker to the main thread without any plumbing required by the developer. Then main-thread use could perhaps be as simple as:
let module = await workerify("./wasm/wasm_image_decoder.js");
await module.default(); // init
await module.initThreadPool(4);
I'm working on something that might improve situation with glue code further in the future here
Looking forward to hearing more about this!
You might want to take a look at https://github.com/GoogleChromeLabs/comlink meanwhile - it sounds very close to what you want.
Oh neat, thank you!
Closes #6