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

Configurable Runtime #2772

Merged
merged 54 commits into from Aug 28, 2022
Merged

Configurable Runtime #2772

merged 54 commits into from Aug 28, 2022

Conversation

futursolo
Copy link
Member

@futursolo futursolo commented Jul 3, 2022

Description

This pull request adds a Runtime type to make the runtime configurable.

Checklist

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

@futursolo futursolo added the A-yew Area: The main yew crate label Jul 3, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2022
@github-actions
Copy link

github-actions bot commented Jul 3, 2022

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

https://yew-rs-api--pr2772-local-runtime-8kie5tce.web.app

(expires Sun, 04 Sep 2022 14:20:13 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

github-actions[bot]
github-actions bot previously approved these changes Jul 3, 2022
@github-actions
Copy link

github-actions bot commented Jul 3, 2022

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
boids 169.355 169.356 +0.001 +0.001%
contexts 107.389 107.380 -0.009 -0.008%
counter 84.911 84.911 0 0.000%
counter_functional 85.535 85.537 +0.002 +0.002%
dyn_create_destroy_apps 87.863 87.868 +0.005 +0.006%
file_upload 101.433 101.433 0 0.000%
function_memory_game 162.726 162.727 +0.001 +0.001%
function_router 345.443 345.459 +0.016 +0.005%
function_todomvc 157.710 157.702 -0.008 -0.005%
futures 221.990 221.991 +0.001 +0.000%
game_of_life 105.071 105.069 -0.002 -0.002%
immutable 180.744 180.736 -0.008 -0.004%
inner_html 82.010 82.011 +0.001 +0.001%
js_callback 110.732 110.722 -0.011 -0.010%
keyed_list 193.483 193.482 -0.001 -0.001%
mount_point 84.694 84.694 0 0.000%
nested_list 112.012 112.011 -0.001 -0.001%
node_refs 91.855 91.854 -0.001 -0.001%
password_strength 1546.345 1546.344 -0.001 -0.000%
portals 95.437 95.434 -0.003 -0.003%
router 314.906 314.905 -0.001 -0.000%
simple_ssr 151.382 151.391 +0.009 +0.006%
ssr_router 390.930 391.037 +0.107 +0.027%
suspense 108.431 108.426 -0.005 -0.005%
timer 87.706 87.707 +0.001 +0.001%
todomvc 139.212 139.213 +0.001 +0.001%
two_apps 85.557 85.557 0 0.000%
web_worker_fib 150.537 150.532 -0.005 -0.003%
webgl 85.688 85.691 +0.003 +0.003%

✅ None of the examples has changed their size significantly.

@futursolo
Copy link
Member Author

futursolo commented Jul 3, 2022

SSR Benchmark Example: https://github.com/yewstack/yew/runs/7166129506?check_suite_focus=true#step:7:386

This benchmark will continue to fail until master has a benchmark present.

@futursolo futursolo mentioned this pull request Jul 3, 2022
2 tasks
github-actions[bot]
github-actions bot previously approved these changes Aug 21, 2022
@futursolo
Copy link
Member Author

futursolo commented Aug 21, 2022

I have updated the examples to process requests within Yew Runtime instead of the default tokio runtime.
This allows the application to be rendered on the same thread as the request is processed.

This increases performance significantly for ServerRenderer under certain environment where switching between threads is expensive (e.g.: in VM).

Function Router Results in a 4 core Debian VM

Before (e117d3f)

Running 30s test @ http://127.0.0.1:8080
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    15.46ms   15.93ms  46.57ms   82.34%
    Req/Sec     2.32k   608.53     5.37k    68.77%
  276500 requests in 30.02s, 0.99GB read
Requests/sec:   9211.24
Transfer/sec:     33.82MB

After (9453ba7)

Running 30s test @ http://127.0.0.1:8080
  4 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    12.00ms   14.19ms  51.02ms   77.67%
    Req/Sec     5.72k     1.16k    7.77k    78.75%
  683108 requests in 30.04s, 2.45GB read
  Socket errors: connect 0, read 0, write 0, timeout 8
Requests/sec:  22738.57
Transfer/sec:     83.49MB

github-actions[bot]
github-actions bot previously approved these changes Aug 21, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 21, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 21, 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, it took so long to get to this PR

Can you also explain/document why this is needed? Is it to allow future support for other runtimes?

Also, how does it tie with #2839?

examples/simple_ssr/src/bin/simple_ssr_server.rs Outdated Show resolved Hide resolved
packages/yew/src/platform/rt_tokio/local_worker.rs Outdated Show resolved Hide resolved
type SpawnTask = Box<dyn Send + FnOnce()>;

thread_local! {
static TASK_COUNT: RefCell<Option<Arc<AtomicUsize>>> = RefCell::new(None);
Copy link
Member

Choose a reason for hiding this comment

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

Why use Arc<AtomicUsize> for a thread local? I think Cell (or Rc<Cell>) should do the job

Copy link
Member Author

@futursolo futursolo Aug 28, 2022

Choose a reason for hiding this comment

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

When a task is spawned with spawn_pinned, the task count is increased from the thread that spawns the task. This allows the task count to be increased before the task spawner receives the task.

packages/yew/src/platform/rt_tokio/local_worker.rs Outdated Show resolved Hide resolved
packages/yew/src/platform/rt_tokio/local_worker.rs Outdated Show resolved Hide resolved
packages/yew/src/platform/rt_tokio/mod.rs Outdated Show resolved Hide resolved
packages/yew/src/platform/rt_tokio/mod.rs Outdated Show resolved Hide resolved
github-actions[bot]
github-actions bot previously approved these changes Aug 28, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 28, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 28, 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 to me.

The reason why a configuration runtime is needed is still unclear to me. Can you explain that?

@futursolo
Copy link
Member Author

futursolo commented Aug 28, 2022

Can you also explain/document why this is needed? Is it to allow future support for other runtimes?

This allows users to set their own number of threads.
num_cpus usually detects the correct threads to spawn.
But sometimes users may want to control this themselves when this cannot be detected correctly or they wish to only use some but not all CPU cores.

Also, how does it tie with #2839?

This exposes Runtime and LocalHandle as a public type.
Which means that now ServerRenderer uses public API for creating default runtime and spawning tasks.

@futursolo futursolo merged commit cffb7c5 into yewstack:master Aug 28, 2022
@futursolo futursolo deleted the local-runtime branch December 15, 2022 10:14
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