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

Fix VList Stream in SSR #2801

Merged
merged 12 commits into from Aug 7, 2022
Merged

Conversation

futursolo
Copy link
Member

@futursolo futursolo commented Aug 2, 2022

Description

This pull request fixes the rendering stream in VList.

Previously, the VList would block until a child future has finished before the child stream was returned.
This pull request allows items to be yielded to the parent stream as soon as an item is written to the child stream.

As a result, this pull request increases function router benchmark performance for ~20%.

Checklist

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

pull request:

╭─────────────────┬───────┬──────────┬──────────┬───────────┬────────────────────╮
│    Benchmark    │ Round │ Min (ms) │ Max (ms) │ Mean (ms) │ Standard Deviation │
├─────────────────┼───────┼──────────┼──────────┼───────────┼────────────────────┤
│    Baseline     │  10   │ 328.295  │ 350.396  │  343.984  │       6.029        │
│   Hello World   │  10   │ 476.104  │ 493.962  │  481.288  │       5.170        │
│ Function Router │  10   │ 1532.462 │ 1553.854 │ 1540.794  │       5.968        │
│ Concurrent Task │  10   │ 1011.433 │ 1017.915 │ 1015.075  │       2.250        │
╰─────────────────┴───────┴──────────┴──────────┴───────────┴────────────────────╯

master:

╭─────────────────┬───────┬──────────┬──────────┬───────────┬────────────────────╮
│    Benchmark    │ Round │ Min (ms) │ Max (ms) │ Mean (ms) │ Standard Deviation │
├─────────────────┼───────┼──────────┼──────────┼───────────┼────────────────────┤
│    Baseline     │  10   │ 372.443  │ 384.248  │  376.874  │       3.182        │
│   Hello World   │  10   │ 472.603  │ 498.872  │  478.182  │       7.599        │
│ Function Router │  10   │ 1816.955 │ 1861.456 │ 1834.111  │       14.948       │
│ Concurrent Task │  10   │ 1009.515 │ 1015.743 │ 1012.627  │       2.061        │
╰─────────────────┴───────┴──────────┴──────────┴───────────┴────────────────────╯

@futursolo futursolo changed the title Fix VList Stream Fix VList Stream in SSR Aug 2, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 2, 2022
@futursolo futursolo added the A-yew Area: The main yew crate label Aug 2, 2022
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

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

https://yew-rs-api--pr2801-fix-rendering-stream-vs03l8f3.web.app

(expires Sat, 13 Aug 2022 16:00:53 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

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

github-actions bot commented Aug 2, 2022

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
boids 172.722 172.720 -0.002 -0.001%
contexts 109.580 109.579 -0.001 -0.001%
counter 86.526 86.526 0 0.000%
counter_functional 87.179 87.180 +0.001 +0.001%
dyn_create_destroy_apps 89.619 89.623 +0.004 +0.004%
file_upload 102.758 102.760 +0.002 +0.002%
function_memory_game 166.896 166.904 +0.008 +0.005%
function_router 351.738 351.736 -0.002 -0.001%
function_todomvc 161.595 161.585 -0.010 -0.006%
futures 225.433 225.390 -0.043 -0.019%
game_of_life 107.137 107.137 0 0.000%
immutable 208.692 208.695 +0.003 +0.001%
inner_html 83.687 83.685 -0.002 -0.002%
js_callback 112.752 112.750 -0.002 -0.002%
keyed_list 195.249 195.247 -0.002 -0.001%
mount_point 86.170 86.172 +0.002 +0.002%
nested_list 115.645 115.641 -0.004 -0.003%
node_refs 93.464 93.463 -0.001 -0.001%
password_strength 1546.080 1546.080 0 0.000%
portals 97.259 97.255 -0.004 -0.004%
router 321.074 321.076 +0.002 +0.001%
simple_ssr 154.248 154.251 +0.003 +0.002%
ssr_router 397.798 397.812 +0.015 +0.004%
suspense 110.486 110.486 0 0.000%
timer 89.259 89.260 +0.001 +0.001%
todomvc 142.623 142.621 -0.002 -0.001%
two_apps 87.190 87.189 -0.001 -0.001%
web_worker_fib 153.608 153.615 +0.007 +0.004%
webgl 87.539 87.538 -0.001 -0.001%

✅ None of the examples has changed their size significantly.

github-actions[bot]
github-actions bot previously approved these changes Aug 2, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 3, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 3, 2022
# Conflicts:
#	.github/workflows/benchmark-ssr.yml
@github-actions
Copy link

github-actions bot commented Aug 3, 2022

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 352.126 368.988 360.855 5.780
Hello World 10 1210.873 1261.104 1234.832 16.158
Function Router 10 4653.621 4805.525 4719.233 46.171
Concurrent Task 10 1012.881 1016.538 1014.642 0.996

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 404.882 414.560 409.781 3.187
Hello World 10 1202.682 1236.553 1223.266 12.067
Function Router 10 3733.080 3819.429 3784.534 30.588
Concurrent Task 10 1011.468 1014.475 1013.286 1.009

github-actions[bot]
github-actions bot previously approved these changes Aug 3, 2022
@futursolo
Copy link
Member Author

futursolo commented Aug 4, 2022

There are some additional optimisations that can be landed to improve SSR performance.
I have chosen to land them individually to make sure that each PR would improve performance when compared with master.

Most optimisations can be found in #2772.

Final Result

╭─────────────────┬───────┬──────────┬──────────┬───────────┬────────────────────╮
│    Benchmark    │ Round │ Min (ms) │ Max (ms) │ Mean (ms) │ Standard Deviation │
├─────────────────┼───────┼──────────┼──────────┼───────────┼────────────────────┤
│    Baseline     │  10   │ 339.746  │ 346.630  │  343.666  │       2.751        │
│   Hello World   │  10   │ 334.349  │ 342.953  │  338.006  │       3.302        │
│ Function Router │  10   │ 1317.307 │ 1335.086 │ 1323.627  │       6.098        │
│ Concurrent Task │  10   │ 1012.617 │ 1018.859 │ 1015.107  │       2.391        │
╰─────────────────┴───────┴──────────┴──────────┴───────────┴────────────────────╯

github-actions[bot]
github-actions bot previously approved these changes Aug 6, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 6, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 6, 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.

Implementation looks good. The performance improvements are great!

Copy link
Member

@WorldSEnder WorldSEnder left a comment

Choose a reason for hiding this comment

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

Is there any reason to use 30 as the magical cut off, or was that determined by an educated guess?

@futursolo
Copy link
Member Author

Is there any reason to use 30 as the magical cut off, or was that determined by an educated guess?

This is picked by futures::future::join_all.

When the number of futures is small, it will use a select like strategy which all futures are polled when one is waken up, this is faster than FuturesUnordered which assigns wakers and operates like kqueue by only waking up futures that are ready. FuturesUnordered is used when the number of futures is big and join_all will switch to FuturesOrdered. The results don't really need to be collected into a vector, so the switching to FuturesUnordered allows to discard the result as they are ready.

@futursolo futursolo merged commit f0b0df3 into yewstack:master Aug 7, 2022
@futursolo futursolo deleted the fix-rendering-stream branch December 15, 2022 10:16
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
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants