Move web-client into a webworker to keep the main thread responsive #1534
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sisou
force-pushed
the
soeren/wasm-worker
branch
from
f0ae4d2 to
03fa52f
Compare
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## albatross #1534 +/- ##
=============================================
+ Coverage 64.89% 64.99% +0.10%
=============================================
Files 429 431 +2
Lines 54490 54498 +8
=============================================
+ Hits 35359 35422 +63
+ Misses 19131 19076 -55
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
sisou
force-pushed
the
soeren/wasm-worker
branch
3 times, most recently
from
3837cee to
ba92889
Compare
jsdanielh
reviewed
Apr 24, 2023
sisou
force-pushed
the
soeren/wasm-worker
branch
2 times, most recently
from
232e5e7 to
67d08d4
Compare
jsdanielh
approved these changes
May 2, 2023
In Transaction, the method could not parse strings, because trying to cast the input into a PlainTransaction already returned early with an Err, preventing string parsing to run at all.
- The wasm-timer crate was incompatible, as it required access to window. I patched it and set the updated version to be used globally, as the libp2p-websocket-transport package, which is an indirect dependency, is also using it. - Use wasm_timer::Delay for timeouts, instead of a custom implementation with js_sys promise and window.setTimeout. - Chromium-based browsers do not emit offline/online events in workers, so I changed them to listen for postMessage messages from the main thread instead. The sender of those events will be in the main-thread index.js file in a later commit.
JS objects returned by wasm-bindgen'd structs are just pointers into WASM memory ({ptr: number}) with mocked class methods and properties. Since calls into in the same thread WASM are sync, users of these objects don't notice a difference.
However, proxying these structs (which **only live** in WASM memory) over to the main thread from the webworker makes all accesses async, even direct property access. That means getting e.g. `peerInfo.address` returns a `Promise<string>` instead of a `string`. Unfortunately, there is no way to represent this behavior in Typescript files, meaning users get no indication of it.
Passing plain values to JS instead copies the primitives over into JS memory, and then over to the main thread, where they are cloned and can be accessed like normal. If the struct has no methods anyway, it thus makes sense to just return a plain JS object.
For the other direction, the issue is that the object the user wants to pass from the main thread into the client in the webworker only exists in the main thread WASM memory. The client WASM in the webworker has no knowledge of it and cannot access it, especially not over the async worker barrier. This means data has to be serialized and copied over into the worker instead, which we here just simply make explicit, instead of implicit with unknown failure cases.
Similar to the last commit, we need to serialize the config to pass it from the main thread to the worker, which mean introducing a plain config version into which the config gets encoded in the main thread and decoded from in the worker. Furthermore, this change has the advantage that `Client.create()` now simply takes a plain object with all properties optional, making it possible to pass a config object with the right properties in directly, without _having_ to go through the ClientConfiguration builder.
We cannot use TransactionBuilder client-connected, as the client and the TransactionBuilder (or at least the transactions that it outputs) live in different thread WASM memories, and accessing them from each other between these contexts is not (easily) possible. Unfortunately, this means for now users have to specify the validity start height and network ID manually, for which I added accessor methods on the client.
Even if they aren't internally. This is because the communication from main thread to webworker can only be async. Typing it async in rust generates the correct types in Typescript.
…t APIs For the NPM package, we create two builds, one for bundlers (that can use import from package names), and one for plain web use, that needs to specify the exact file that is imported, both in the main thread and in the webworker. This also updates the demo (now in example/) module.js file to use the updated client API, using the TransactionBuilder statically and passing serialized (to hex in this case) transactions when sending them. No other changes are required in the demo file. The `Client` class/object defined in dist/client-proxy.js _shadows_ the `class Client` exported by the worker types and adheres to that one's static API. For the user it is transparent.
In preparation for the introduction of features in the next commit.
This way the primitives-only (offline) build is much smaller (unoptimized 7.0MB -> 810kB)
…d targets Making both feature builds smaller.
Fix the setting of the log level for the client. The `primitives` feature was missing the `camelCase` `serde` directive and the `Tsify` derive directive for the `ClientConfiguration` struct. Also made the `primitives` and `client` features the default features and changed the script to disable the default features. Also changes the script to use `set -e` to avoid the usage of `&&` in each line.
Add handler to trigger the check of peers on the `visible` event from the `visibilitychange` event list from the document.
By transparently serializing them to their plain format before being sent to the worker.
Member
|
Rebasing it to get it merged |
jsdanielh
force-pushed
the
soeren/wasm-worker
branch
from
9534d16 to
ce263a0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's in this pull request?
Various changes, but all relatively minor, with minimal effect on the public API.
If anything is unclear, please ask!
This fixes UI freezing in browsers while the client is using lots of CPU.
Pull request checklist
clippyandrustfmtwarnings.