Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple worker backends on the web (to fix the Chrome crash) #28

Merged
merged 11 commits into from
Oct 11, 2022

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Oct 11, 2022

What problem does this PR solve?

Workaround for #1

Google Chrome tends to crash when WASM PHP is initialized in a WebWorker. It is stable when WASM is initialized in an iframe, though. This PR swaps a WebWorker for an iframe until the Google Chrome bug is resolved.

Important! The iframe must be loaded from another domain to spin a new browser thread. If it's loaded from the same domain, WordPress "server" will run in the same thread that paints the user interface and dramatically slow down all user interactions.

How does it solve it?

Technically, this PR:

  1. Enables defering the WASM workload to any configurable backend
  2. Implements two backends: iframeWorkerBackend and webWorkerBackend
  3. Makes the iframeWorkerBackend the default one
const wasmWorker = createWordPressWorker(
	{
		backend: iframeWorkerBackend("http://127.0.0.1:8778/iframe-worker.html"),
		wordPressSiteURL: location.href
	}
);

@adamziel adamziel changed the title Stabilize in Chrome by swapping a web worker for an iframe Fix Chrome crashes by replacing a web worker with an iframe Oct 11, 2022
@eliot-akira
Copy link
Collaborator

eliot-akira commented Oct 11, 2022

What a debugging session that was in #1, the amount of technical tenacity to dig deep, chasing through decompiled WASM text format, to narrow down the cause of the crash in Chrome.

That's great how this workaround is implemented with an "adaptor" pattern, to support both iframe and web worker backends. I imagine this will be useful for (maybe) conditionally loading the iframe backend based on browser user agent; or better, when the Chrome team hopefully solves the "out of memory" error, the web worker can be the default (or only) backend.

The requirement for a second domain is a bit painful, but I think it shows how all this is a brave new frontier.


An observation: I noticed in package.json, the script build:php:web was changed to run a shell script at wasm-build/php/wasm-worker-build-wasm.sh, but the file itself still has prefix webworker-.

To be thorough, I searched the codebase (in this branch iframe-worker) for all occurrences of webworker.

./package.json:26:    "publish:php:web": "cp wasm-build/php/docker-output/webworker-* dist-web/ && cp src/shared/etc/php.ini dist-web/etc/",

./src/web/iframe-worker.html:6:         <script src="webworker-php.js"></script>
./src/web/wasm-worker.js:28:importScripts( '/webworker-php.js' );

./wasm-build/php/webworker-build-wasm.sh:24:        -o /output/webworker-php.js \
./wasm-build/php/webworker-build-wasm.sh:38:        --pre-js /preload/webworker-pre-php.js \

Well, perhaps this prefix change is better suited for the other PR (#25) about build scripts.

Edit: Oh but the PHP WASM is a web worker on its own (if I understand right), so maybe the above prefixes are all correct as intended.

@adamziel
Copy link
Collaborator Author

adamziel commented Oct 11, 2022

An observation: I noticed in package.json, the script build:php:web was changed to run a shell script at wasm-build/php/wasm-worker-build-wasm.sh, but the file itself still has prefix webworker-.

@eliot-akira you're right – it is confusing to retain the webworker terminology in all these places. PHP is not specific to workers, it's simply loaded in any context you load the script in. I initially intended this cleanup for a follow-up PR, but I ended up updating this one. It now uses the term "PHP web" in the build pipeline and then outputs two JS loaders: php-web.js and php-webworker.js. I hope it makes the repo more legible!

@adamziel adamziel changed the title Fix Chrome crashes by replacing a web worker with an iframe Multiple worker backends on the web (to fix the Chrome crash) Oct 11, 2022
@adamziel adamziel merged commit eb7b6b9 into trunk Oct 11, 2022
@adamziel adamziel mentioned this pull request Oct 11, 2022
@gziolo gziolo deleted the iframe-worker branch October 11, 2022 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants