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

Polled SSR Stream #2824

Merged
merged 32 commits into from Sep 10, 2022
Merged

Polled SSR Stream #2824

merged 32 commits into from Sep 10, 2022

Conversation

futursolo
Copy link
Member

@futursolo futursolo commented Aug 12, 2022

Description

This pull request introduces a polled SSR stream instead of using channels.

Compared to channels, Polled Streams:

  • yield content before a suspended component in a single chunk and does not wait until reaching a certain capacity.
  • have better performance than channel-based streams with much smaller reserved capacity (1024 vs 8192).
  • resolving future is polled alongside the stream.

Checklist

  • I have reviewed my own code
  • I have added tests

github-actions[bot]
github-actions bot previously approved these changes Aug 12, 2022
@github-actions
Copy link

github-actions bot commented Aug 12, 2022

Visit the preview URL for this PR (updated for commit a78f0b3):

https://yew-rs-api--pr2824-polled-stream-x9xginht.web.app

(expires Sun, 11 Sep 2022 02:17:22 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@futursolo futursolo added the A-yew Area: The main yew crate label Aug 12, 2022
@github-actions
Copy link

github-actions bot commented Aug 12, 2022

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 419.804 428.535 423.388 2.875
Hello World 10 1258.366 1308.084 1289.160 15.715
Function Router 10 3026.638 3176.004 3118.468 61.431
Concurrent Task 10 1011.819 1015.229 1013.623 1.109

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 417.519 433.946 424.418 4.352
Hello World 10 748.980 793.166 769.875 11.488
Function Router 10 2648.450 2834.495 2763.374 71.040
Concurrent Task 10 1008.841 1012.347 1011.034 1.133

@github-actions
Copy link

github-actions bot commented Aug 12, 2022

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
boids 169.308 169.311 +0.003 +0.002%
contexts 107.335 107.335 0 0.000%
counter 84.829 84.831 +0.002 +0.002%
counter_functional 85.453 85.453 0 0.000%
dyn_create_destroy_apps 87.789 87.790 +0.001 +0.001%
file_upload 99.493 99.494 +0.001 +0.001%
function_memory_game 162.753 162.754 +0.001 +0.001%
function_router 345.724 345.643 -0.081 -0.023%
function_todomvc 157.712 157.715 +0.003 +0.002%
futures 222.151 222.155 +0.004 +0.002%
game_of_life 105.023 105.023 0 0.000%
immutable 180.612 180.610 -0.002 -0.001%
inner_html 81.929 81.929 0 0.000%
js_callback 110.685 110.688 +0.003 +0.003%
keyed_list 193.486 193.489 +0.003 +0.002%
mount_point 84.614 84.615 +0.001 +0.001%
nested_list 111.951 111.955 +0.004 +0.003%
node_refs 91.812 91.808 -0.004 -0.004%
password_strength 1547.278 1547.283 +0.005 +0.000%
portals 95.384 95.385 +0.001 +0.001%
router 315.201 315.084 -0.117 -0.037%
simple_ssr 151.356 151.347 -0.010 -0.006%
ssr_router 391.231 391.232 +0.001 +0.000%
suspense 108.385 108.389 +0.004 +0.004%
timer 87.625 87.625 0 0.000%
todomvc 139.212 139.213 +0.001 +0.001%
two_apps 85.473 85.474 +0.001 +0.001%
web_worker_fib 150.535 150.536 +0.001 +0.001%
webgl 84.398 84.395 -0.004 -0.005%

✅ None of the examples has changed their size significantly.

github-actions[bot]
github-actions bot previously approved these changes Aug 12, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 12, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 13, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 16, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 16, 2022
@hamza1311 hamza1311 added this to the v0.20 milestone Aug 21, 2022
# Conflicts:
#	packages/yew/src/server_renderer.rs
github-actions[bot]
github-actions bot previously approved these changes Aug 29, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 29, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 29, 2022
Copy link
Member

@hamza1311 hamza1311 left a comment

Choose a reason for hiding this comment

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

Looks good, sorry it took me a while to get to it

tracing = "0.1.36"
pin-project = "1.0.11"
Copy link
Member

Choose a reason for hiding this comment

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

pin-project-lite maybe?

Copy link
Member Author

@futursolo futursolo Sep 4, 2022

Choose a reason for hiding this comment

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

However, if you already have proc-macro related dependencies in your crate’s dependency graph, there is no benefit from using this crate.

See: https://docs.rs/pin-project-lite/0.2.8/pin_project_lite/#different-no-proc-macro-related-dependencies

The primary benefit of #[pin_project] is that rustfmt will continue to work for the struct.

state: BufStreamState,

// This type is not send or sync.
_marker: PhantomData<Rc<()>>,
Copy link
Member

Choose a reason for hiding this comment

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

Why not do:

impl !Send for Inner {}
impl !Sync for Inner {}

Copy link
Member Author

Choose a reason for hiding this comment

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

This is due to that negative impls is not stable.

See: https://doc.rust-lang.org/unstable-book/language-features/negative-impls.html

packages/yew/src/platform/fmt/buffer.rs Outdated Show resolved Hide resolved
packages/yew/src/platform/fmt/buffer.rs Outdated Show resolved Hide resolved
Comment on lines +20 to +21
where
F: Future<Output = ()>,
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps remove the bound from here? That would mean that everywhere this type is named doesn't require the bound - only the usages do

Copy link
Member Author

Choose a reason for hiding this comment

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

We cannot remove this as this bound is declared on the MaybeDone type itself.

@@ -1,103 +0,0 @@
//! This module contains types for I/O functionality.
Copy link
Member

Choose a reason for hiding this comment

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

It seems that this file was moved, rather than deleted. git mv would be better in that case since that preserves the git history

Copy link
Member Author

Choose a reason for hiding this comment

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

fmt is a complete rewrite of the buffer and not connected to the previous implementation.

I tried to interactively rebase the changes and is having conflicts with every commits after the commit that removed io.rs.

packages/yew/src/platform/pinned/mpsc.rs Show resolved Hide resolved
packages/yew/src/platform/pinned/oneshot.rs Show resolved Hide resolved
github-actions[bot]
github-actions bot previously approved these changes Sep 4, 2022
Copy link
Member

@hamza1311 hamza1311 left a comment

Choose a reason for hiding this comment

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

Sorry, I didn't see the review notifications. Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew Area: The main yew crate breaking change
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants