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

Reduce SSR Buffers in VList #2826

Merged
merged 1 commit into from
Aug 14, 2022
Merged

Conversation

futursolo
Copy link
Member

Description

This pull request updates the VList rendering logic that a new buffer is created only when a child VNode does not resolve immediately.

Checklist

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

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

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

https://yew-rs-api--pr2826-reduced-buffers-dpb963n6.web.app

(expires Sat, 20 Aug 2022 13:53:05 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 414.035 420.608 415.211 1.986
Hello World 10 1321.769 1340.888 1328.334 5.512
Function Router 10 3933.803 3961.567 3946.938 8.579
Concurrent Task 10 1011.628 1013.520 1012.600 0.696

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 409.141 474.372 426.398 19.300
Hello World 10 1315.391 1364.253 1333.935 15.156
Function Router 10 3053.501 3086.041 3065.595 8.840
Concurrent Task 10 1011.311 1013.019 1012.290 0.544

@github-actions
Copy link

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
boids 168.569 168.563 -0.006 -0.003%
contexts 106.698 106.701 +0.003 +0.003%
counter 84.322 84.322 0 0.000%
counter_functional 84.965 84.962 -0.003 -0.003%
dyn_create_destroy_apps 87.277 87.276 -0.001 -0.001%
file_upload 100.852 100.852 0 0.000%
function_memory_game 162.231 162.224 -0.008 -0.005%
function_router 343.247 343.233 -0.014 -0.004%
function_todomvc 157.198 157.193 -0.005 -0.003%
futures 221.177 221.178 +0.001 +0.000%
game_of_life 104.509 104.510 +0.001 +0.001%
immutable 180.214 180.221 +0.007 +0.004%
inner_html 81.438 81.437 -0.002 -0.002%
js_callback 110.025 110.025 0 0.000%
keyed_list 192.827 192.824 -0.003 -0.002%
mount_point 84.124 84.123 -0.001 -0.001%
nested_list 111.504 111.507 +0.003 +0.003%
node_refs 91.229 91.232 +0.003 +0.003%
password_strength 1547.431 1547.432 +0.001 +0.000%
portals 94.747 94.745 -0.002 -0.002%
router 312.695 312.693 -0.002 -0.001%
simple_ssr 150.671 150.674 +0.003 +0.002%
ssr_router 388.759 388.740 -0.019 -0.005%
suspense 107.726 107.728 +0.002 +0.002%
timer 87.071 87.070 -0.001 -0.001%
todomvc 138.570 138.570 0 0.000%
two_apps 84.983 84.982 -0.001 -0.001%
web_worker_fib 149.966 149.951 -0.015 -0.010%
webgl 85.115 85.117 +0.002 +0.002%

✅ None of the examples has changed their size significantly.

Copy link
Member

@ranile ranile 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 🎉

Comment on lines +218 to +219
// boxing to avoid recursion
.boxed_local();
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, what happens this isn't boxed? If the issue is stack overflow, is this something tail recursion can fix?

Copy link
Member Author

Choose a reason for hiding this comment

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

With Rust’s async/await design, it’s not possible to recursively call async functions without boxing.

https://rust-lang.github.io/async-book/07_workarounds/04_recursion.html

@ranile ranile merged commit 9e602b9 into yewstack:master Aug 14, 2022
@futursolo futursolo deleted the reduced-buffers branch December 15, 2022 10:13
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.

2 participants