Add multithreading example #2087
Closed
Conversation
This file contains 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
SpanishPear
changed the title
|
I'm not sure if there's any more config that's needed, I did a grep for the OG Let me know! |
|
For context, I didn't look at the contents of the example, only ported it
to work with master + set it to work with the repo 😋
The comments made are valid and would make it easier for users to navigate
the example so I'll put through those changes!
Thanks for the review
…On Sun, 26 Sep 2021, 10:10 pm mc1098, ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Awesome stuff 😍
The "Thanks to" section is a nice way to give a shout out to @insou22
<https://github.com/insou22>, so I appreciate that ❤️.
Most of my comments below is just me being picky for the most part :)
------------------------------
In examples/multi_thread/README.md
<#2087 (comment)>:
> +
+The multi-thread example is a conceptual WIP and is currently blocked on [a future Trunk feature](trunk-rs/trunk#46)
+
Could even add a link to your new example as an alternative here :)
------------------------------
In examples/web_worker_fib/README.md
<#2087 (comment)>:
> @@ -0,0 +1,23 @@
+# Web Worker Demo
+
+Calculate fibbonnaci value of a number in the worker thread,
+without blocking the main thread.
+
+You can access a live version here:
+
+## notes
+This example is NOT built with [trunk](https://github.com/thedodd/trunk).
+Multi-threading in yew does not currently build with Trunk, due to issues described in the [multi_thread](/examples/multi_thread/README.md) example.
+
+Instead, the example is built with `wasm_bindgen`.
⬇️ Suggested change
-Instead, the example is built with `wasm_bindgen`.
+Instead, the example is built with [`wasm-pack`](https://rustwasm.github.io/wasm-pack/) directly.
I think this is more correct?
------------------------------
In examples/web_worker_fib/static/index.html
<#2087 (comment)>:
> +<!doctype html>
+<html lang="en">
+ <head>
+ <meta charset="utf-8">
+ <title>Yew web worker demo</title>
+
+ <script src="wasm.js"></script>
+ <script type="application/javascript">wasm_bindgen('wasm_bg.wasm');</script>
+ </head>
+
+ <body>
+
+ </body>
+</html>
space indent over tab - more for consistency then hate for tabs :)
------------------------------
In examples/web_worker_fib/src/lib.rs
<#2087 (comment)>:
> @@ -0,0 +1,19 @@
+#![recursion_limit = "1024"]
+#![allow(clippy::large_enum_variant)]
Is this needed?
------------------------------
In examples/web_worker_fib/src/app.rs
<#2087 (comment)>:
> + if let Ok(value) = input.value().parse::<u32>() {
+ // start the worker off!
+ self.worker.send(WorkerInput { n: value });
+ }
You can use HtmlInputElement::value_as_number here then cast from f64 to
u32 - doing this you may want to set max attribute on the input, which I
don't think is a bad idea in of itself so that people don't try and workout
the fib of too large a number 😅
------------------------------
In examples/web_worker_fib/src/app.rs
<#2087 (comment)>:
> + worker: Box<dyn Bridge<Worker>>,
+ fibonacci_output: String,
+}
+
+pub(crate) enum Message {
+ Click,
+ RunWorker,
+ WorkerMsg(WorkerOutput),
+}
+
+impl Component for Model {
+ type Message = Message;
+ type Properties = ();
+
+ fn create(ctx: &Context<Self>) -> Self {
+ let worker = Worker::bridge(ctx.link().callback(Self::Message::WorkerMsg));
⬇️ Suggested change
- let worker = Worker::bridge(ctx.link().callback(Self::Message::WorkerMsg));
+ let worker = Worker::bridge(ctx.link().callback(Message::WorkerMsg));
Purely a style suggestion - if you prefer the Self:: prefix then keep
them :)
If you change this one then there are others but I won't highlight them
until you've made a decision here.
------------------------------
In examples/web_worker_fib/src/app.rs
<#2087 (comment)>:
> @@ -0,0 +1,123 @@
+use serde::{Deserialize, Serialize};
+use yew::prelude::*;
+use yew::web_sys::HtmlInputElement;
+use yew_agent::{Agent, AgentLink, Bridge, Bridged, HandlerId, Public};
+
+pub(crate) struct Model {
+ clicker_value: u32,
+ n_ref: NodeRef,
Very picky nit:
I don't mind input_ref or fib_input_ref but I think it should be bit more
expressive a name in this example.
------------------------------
In examples/web_worker_fib/src/app.rs
<#2087 (comment)>:
> + <>
+ <h1> { "Web worker demo" } </h1>
+ <h3> { "Output: " } { &self.fibonacci_output } </h3>
+ <br />
+ <input ref={self.n_ref.clone()} type="number" />
+ <button onclick={ctx.link().callback(|_| Message::RunWorker)}> { "submit" } </button>
+ <br /> <br />
+ <h3> { "Main thread value: " } { self.clicker_value } </h3>
+ <button onclick={ctx.link().callback(|_| Message::Click)}> { "click!" } </button>
+ </>
Nit:
All the examples, as far as I remember 🙃, have the same format when it
comes to tags and braces:
<h1>{ "Space around this string lit only" }</h1>
------------------------------
In examples/web_worker_fib/src/app.rs
<#2087 (comment)>:
> +pub(crate) struct Worker {
+ link: AgentLink<Self>,
+}
+
+#[derive(Serialize, Deserialize)]
+pub(crate) struct WorkerInput {
+ n: u32,
+}
+
+#[derive(Serialize, Deserialize)]
+pub(crate) struct WorkerOutput {
+ value: u32,
+}
+
+impl Agent for Worker {
+ type Reach = Public<Self>;
+ type Message = ();
+ type Input = WorkerInput;
+ type Output = WorkerOutput;
+
+ fn create(link: AgentLink<Self>) -> Self {
+ Self { link }
+ }
+
+ fn update(&mut self, _msg: Self::Message) {
+ // no messaging
+ }
+
+ fn handle_input(&mut self, msg: Self::Input, id: HandlerId) {
+ // this runs in a web worker
+ // and does not block the main
+ // browser thread!
+
+ let n = msg.n;
+
+ fn fib(n: u32) -> u32 {
+ if n <= 1 {
+ 1
+ } else {
+ fib(n - 1) + fib(n - 2)
+ }
+ }
+
+ let output = Self::Output { value: fib(n) };
+
+ self.link.respond(id, output);
+ }
+
+ fn name_of_resource() -> &'static str {
+ "wasm.js"
+ }
+}
I think the Agent stuff can go in it's own file
------------------------------
In examples/web_worker_fib/Cargo.toml
<#2087 (comment)>:
> @@ -0,0 +1,14 @@
+[package]
+name = "yew-worker-demo"
+version = "0.1.0"
+edition = "2018"
+
Feel free to add yourself as the author here 🎉
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2087 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGVFWMN27ZNH52E4CVK2YTLUD4EUVANCNFSM5EYUBBYQ>
.
|
NickHu
suggested changes
Oct 4, 2021
3 tasks
|
Aware I've left this for a while - will get a chance to polish it this weekend EDIT: That didn't end up happening, but I'll get to it eventually |
voidpumpkin
added
the
S-waiting-on-author
voidpumpkin
removed
the
S-waiting-on-author
|
... I think i did a whoopsy and force-undid my work, but have fixed it. Can someone re-open this PR? |
ranile
reviewed
Nov 21, 2021
voidpumpkin
added
the
S-waiting-on-author
3 tasks
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.
Description
Introduce a functioning mulithreading with web weorkers demo.
Fixes #2070
Checklist
cargo make pr-flow