-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Streamed SSR Response #2697
Streamed SSR Response #2697
Conversation
Visit the preview URL for this PR (updated for commit d4b15f6): https://yew-rs-api--pr2697-platform-6j632oka.web.app (expires Sun, 29 May 2022 16:43:19 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
Size Comparison
✅ None of the examples has changed their size significantly. |
@@ -14,7 +14,7 @@ keywords = ["web", "webasm", "javascript"] | |||
categories = ["gui", "wasm", "web-programming"] | |||
description = "A framework for making client-side single-page apps" | |||
readme = "../../README.md" | |||
rust-version = "1.56.0" | |||
rust-version = "1.60.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MSRV has been updated to 1.60 due to the requirement of namespaced dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! This will be a great improvement
# Conflicts: # examples/ssr_router/src/bin/ssr_router_server.rs
0d477f4
The benchmark workflow failure is not related. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things in the custom BufWriter
implementation that I didn't notice the first time around, other than that, looks mostly good to me (from an API perspective).
9c36639
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For build times it might still be best to get rid of the tokio dependency in the long run, but I don't view this as blocking (since I have yet to measure the difference anyway). We could also add build times to the performance benchmarking step, I think it'd be a good metric to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for all the work you put in
Description
This pull request consists of the following changes:
Yew Runtime
Introduces a single-threaded Runtime to bridge between Send and non-Send futures.
This runtime pins the rendering task to one of the thread in the internal rendering pool.
Pinning a future to a single thread is better than making them Send when it comes to processing web requests as they are usually shortly lived and io-bounded (e.g.: waiting for DB / API connection), resolving them on a single thread eliminates the overhead of sending the tasks between threads.
(See: https://docs.rs/actix-rt/latest/actix_rt/index.html)
yew::platform
(previouslyyew::io_coop
) public.This exposes a
spawn_local
that always works with the current runtime used by ServerRenderer.yew::platform::run_pinned
to run a future pinned on to a single thread.ServerRenderer
are nowSend
.ServerRenderer
renders withrun_pinned
by default.LocalServerRenderer
.tokio
orwasm-bindgen-futures
runtime is required.With this change:
async-std
runtime) as it brings its own runtime.Send
and can be polled from web servers that requires Send futures (warp
/axum
).Streamed Server-side Rendering
Other
Checklist