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

Implemented an asyncify based implementation of asynchronous threading #3710

Merged
merged 72 commits into from
May 15, 2023

Conversation

john-sharratt
Copy link
Contributor

@john-sharratt john-sharratt commented Mar 26, 2023

This pull request implements asynchronous threading

In essence what this does is allow the runtime to exit a WASM thread and resume it where it left off after an async condition passes - when coupled with the WASIX syscalls this allows for all waits and IO operations to unload their WASM threads when going idle.

In future this will allow a designer to move threads from one machine to another

Performed some benchmarks and asynchronous threading gives approximately a 6% increase in performance thus the benefits of parking idle threads appears to out way the overhead of managing them (tested with multiple runs).

before:

john@AlienWorld:/prog/deploy$ wrk -t 10 -c 600 -d 60 http://localhost:9080/
Running 1m test @ http://localhost:9080/
  10 threads and 600 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   203.43ms  335.09ms   1.87s    85.30%
    Req/Sec     1.38k   777.50     4.85k    71.05%
  750093 requests in 1.00m, 1.07GB read
Requests/sec:  12483.79
Transfer/sec:     18.24MB

after:

john@AlienWorld:/prog/deploy$ wrk -t 10 -c 600 -d 60 http://localhost:9080/
Running 1m test @ http://localhost:9080/
  10 threads and 600 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   125.93ms  209.33ms   1.41s    87.05%
    Req/Sec     1.38k   592.88     4.32k    73.30%
  798846 requests in 1.00m, 1.14GB read
Requests/sec:  13292.74
Transfer/sec:     19.42MB

Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

It looks good from me for the API side

@john-sharratt john-sharratt changed the title Implemented the draft asyncify implementation for asynchronous threading Implemented an asyncify based implementation for asynchronous threading Mar 27, 2023
@john-sharratt john-sharratt changed the title Implemented an asyncify based implementation for asynchronous threading Implemented an asyncify based implementation of asynchronous threading Mar 27, 2023
Copy link
Contributor

@theduke theduke left a comment

Choose a reason for hiding this comment

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

I'm not so happy about putting more logic into the logic in binfactory/exec.rs, I really wanted to move away from that to make the logic less dependent on it.

Calls like WasiEnvBuilder::run() do not go through that flow.

We should but this into some reusable function , that preferably also does not require spawning threads.

lib/api/src/ptr.rs Show resolved Hide resolved
lib/wasi/src/runtime/task_manager/mod.rs Outdated Show resolved Hide resolved
lib/wasi/src/runtime/task_manager/tokio.rs Outdated Show resolved Hide resolved
@theduke
Copy link
Contributor

theduke commented Mar 27, 2023

Only did a supferficial review, more in-depth after we discuss it.

Also:

  • needs some tests

@john-sharratt
Copy link
Contributor Author

Only did a supferficial review, more in-depth after we discuss it.

Also:

  • needs some tests

the snapshot tests already extensively test this (forking, http web server, etc...) - what we lack are tests in the JS area but that is not for this pull request

@john-sharratt
Copy link
Contributor Author

john-sharratt commented Mar 30, 2023

Quite some refactoring to get the snapshot tests working - there were a number of edge cases that were failing

@john-sharratt john-sharratt requested a review from theduke April 19, 2023 00:06
@john-sharratt john-sharratt dismissed theduke’s stale review April 19, 2023 00:07

Completed this round of review

@john-sharratt
Copy link
Contributor Author

Tests have been added in snapshots

Copy link
Member

@syrusakbary syrusakbary 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 from the API side

Copy link
Contributor

@theduke theduke left a comment

Choose a reason for hiding this comment

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

This is only partial, final review to come.

lib/api/src/externals/memory.rs Show resolved Hide resolved
lib/api/src/externals/memory.rs Show resolved Hide resolved
lib/api/src/externals/memory.rs Show resolved Hide resolved
lib/cli/src/commands/run/wasi.rs Show resolved Hide resolved
lib/wasi-web/src/runtime.rs Show resolved Hide resolved
lib/wasi/src/runtime/task_manager/mod.rs Show resolved Hide resolved
lib/wasi/src/syscalls/mod.rs Show resolved Hide resolved
lib/wasi/src/syscalls/mod.rs Show resolved Hide resolved
Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

LGTM on the API side

@john-sharratt john-sharratt enabled auto-merge May 15, 2023 05:27
@john-sharratt john-sharratt merged commit 553b0fb into master May 15, 2023
@john-sharratt john-sharratt deleted the asynchronous-threading branch May 15, 2023 05:30
@john-sharratt john-sharratt restored the asynchronous-threading branch June 9, 2023 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants