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

web worker #1091

Merged
merged 2 commits into from
May 6, 2024
Merged

web worker #1091

merged 2 commits into from
May 6, 2024

Conversation

futurepaul
Copy link
Collaborator

@futurepaul futurepaul commented Apr 23, 2024

closes #601 (kind of)

I've confused the concept of ServiceWorker and SharedWorker a lot of times. What we have here is a SharedWorker... in the future this will make it easier for a ServiceWorker to connect to the one canonical instance of Mutiny, whether or not the app is open.

For now, the main win is getting all the mutiny work off the main UI thread. Everything should be more responsive (no blocking) and in some cases faster (no contention, so faster, though there is a serialization cost going across threads).

EDIT:
it's a web worker now, not a shared worker. thx chrome android

TODO:

  • add the doc strings
  • test everything

Copy link

cloudflare-workers-and-pages bot commented Apr 23, 2024

Deploying mutiny-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8f94770
Status: ✅  Deploy successful!
Preview URL: https://6597b4b5.mutiny-web.pages.dev
Branch Preview URL: https://sw.mutiny-web.pages.dev

View logs

@benthecarman
Copy link
Collaborator

benthecarman commented Apr 23, 2024

in some places you do sw.function() and others you do sw?.function() what is the reasoning there?

edit: I think I only actually saw one

Copy link
Collaborator

@benthecarman benthecarman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everywhere where you do // PAIN in the worker is where we return a wasm_bindgen type instead of a JsValue this was supposed to be nicer for you but seems like it is worse now. Maybe in mutiny-wasm we should just try to do JsValue everywhere now

src/router.tsx Outdated
Comment on lines 181 to 182
<Route path="/swap" component={Swap} />
<Route path="/swaplightning" component={SwapLightning} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we're only temporarily disabling these for now, should bring them back eventually

src/routes/Chat.tsx Outdated Show resolved Hide resolved
src/routes/Send.tsx Outdated Show resolved Hide resolved
@benthecarman
Copy link
Collaborator

benthecarman commented Apr 23, 2024

What we have here is a SharedWorker.

this site says it doesn't work for chrome android 😢, will need to test

probably should add to the screen where we detect if js/wasm is enabled, that shared workers are available

@futurepaul
Copy link
Collaborator Author

What we have here is a SharedWorker.

this site says it doesn't work for chrome android 😢, will need to test

probably should add to the screen where we detect if js/wasm is enabled, that shared workers are available

THIS IS SO SAD :(. It's a one line change to make it a non-shared worker, but I might look into detecting Chrome Android specifically so we can use sharedworker other places. But that means I need to bring back all the double init defense stuff :(

I'm also having trouble with capacitor right now. It's not working iOS or Android, for what might be different reasons. One problem is this import.meta.url which doesn't seem to exist in the Capacitor environment, trying to figure it out.

@futurepaul
Copy link
Collaborator Author

Got android working...

Screenshot 2024-04-24 at 11 35 12 AM

But iOS is mad about something. Works in desktop safari...

Screenshot 2024-04-24 at 11 37 07 AM

@futurepaul
Copy link
Collaborator Author

Everywhere where you do // PAIN in the worker is where we return a wasm_bindgen type instead of a JsValue this was supposed to be nicer for you but seems like it is worse now. Maybe in mutiny-wasm we should just try to do JsValue everywhere now

yeah it sucks because the JsValue erases the type so I have to go digging to figure out what it's actually returning. do you remember why this was rejected btw? MutinyWallet/mutiny-node#882

@benthecarman
Copy link
Collaborator

Wasm bindgen does the same thing, it just doesn't work for async functions yet. I think they recently merged support for it though

@futurepaul futurepaul changed the title shared worker web worker May 1, 2024
@futurepaul futurepaul marked this pull request as ready for review May 1, 2024 22:50
@TonyGiorgio
Copy link
Collaborator

My mainnet tests on my android phone are great. No more flicker. Page switches are much faster now. Signet will need a full end to end test but this is really smooth. Nice work!

@TonyGiorgio
Copy link
Collaborator

Screenshot 2024-05-02 at 4 34 57 PM

I don't think it's intended to be at the bottom?

@futurepaul futurepaul force-pushed the sw branch 2 times, most recently from 7ec6a7e to 903e20b Compare May 6, 2024 19:48
futurepaul added 2 commits May 6, 2024 14:53
check if already initialized

more progress, zap feed not loading?

request send receive

fix setup

profile editing and show zaps

wallet connections

kitchen sink

mutiny plus and misc

get rid of swap

backup / restore, nostr stuff

get rid of gifts

channels stuff

manage federations and profile fixes

cleanup

fix build

fix chrome android

update to cap 6

bump to actual 6.0.0

update xcode version

fix interpolation again (regression)

move all static methods to the worker

add doc strings

get rid of window.nostr, make parse params async

fight load flicker

use a "-test" bundle for debug builds so they don't clobber

add back swaps and do some cleanup

fix activity flicker
package.json Show resolved Hide resolved
@TonyGiorgio TonyGiorgio merged commit dab53ac into master May 6, 2024
5 checks passed
@TonyGiorgio TonyGiorgio deleted the sw branch May 6, 2024 20:01
This was referenced May 6, 2024
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.

Service worker
3 participants