wasmtime / PR #6091 Initial implementation of wasi-http s... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #6091 Initial implementation of wasi-http s...

Wasmtime GitHub notifications bot (Mar 23 2023 at 03:35):

brendandburns opened PR #6091 from http_serve to main:

This builds on top of #5929 to add an implementation of http serving.

This is a rough draft, but is ready for review, I think.

A server.c example is also added.

cc @pchickey

Wasmtime GitHub notifications bot (Mar 23 2023 at 03:38):

brendandburns edited PR #6091 from http_serve to main:

This builds on top of #5929 to add an implementation of http serving.

This is a rough draft, but is ready for review, I think.

A server.c example is also added.

The only thing that really needs review is the final commit

cc @pchickey

Wasmtime GitHub notifications bot (Mar 23 2023 at 15:59):

brendandburns updated PR #6091 from http_serve to main.

Wasmtime GitHub notifications bot (Mar 24 2023 at 20:41):

brendandburns updated PR #6091 from http_serve to main.

Wasmtime GitHub notifications bot (Mar 28 2023 at 20:04):

brendandburns requested alexcrichton for a review on PR #6091.

Wasmtime GitHub notifications bot (Mar 28 2023 at 20:04):

brendandburns requested fitzgen for a review on PR #6091.

Wasmtime GitHub notifications bot (Mar 28 2023 at 20:04):

brendandburns requested jameysharp for a review on PR #6091.

Wasmtime GitHub notifications bot (Mar 28 2023 at 20:04):

brendandburns requested pchickey for a review on PR #6091.

Wasmtime GitHub notifications bot (Mar 28 2023 at 20:04):

brendandburns requested cfallin for a review on PR #6091.

Wasmtime GitHub notifications bot (Mar 28 2023 at 20:04):

brendandburns requested elliottt for a review on PR #6091.

Wasmtime GitHub notifications bot (Mar 28 2023 at 20:04):

brendandburns updated PR #6091 from http_serve to main.

Wasmtime GitHub notifications bot (Mar 28 2023 at 20:13):

brendandburns updated PR #6091 from http_serve to main.

Wasmtime GitHub notifications bot (Mar 29 2023 at 05:07):

brendandburns requested wasmtime-core-reviewers for a review on PR #6091.

Wasmtime GitHub notifications bot (Mar 29 2023 at 05:07):

brendandburns requested wasmtime-default-reviewers for a review on PR #6091.

Wasmtime GitHub notifications bot (Mar 29 2023 at 05:07):

brendandburns updated PR #6091 from http_serve to main.

Wasmtime GitHub notifications bot (Mar 30 2023 at 15:30):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Mar 31 2023 at 03:41):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 06 2023 at 00:14):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 06 2023 at 04:37):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 06 2023 at 04:38):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 06 2023 at 04:40):

brendandburns edited PR #6091:

This builds on top of #5929 to add an implementation of http serving.

This is a rough draft, but is ready for review, I think.

~A server.c example is also added.~

The only thing that really needs review is the final commit

cc @pchickey

Wasmtime GitHub notifications bot (Apr 06 2023 at 16:47):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 06 2023 at 16:48):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 06 2023 at 17:04):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 06 2023 at 17:47):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 12 2023 at 02:42):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 13 2023 at 02:37):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 13 2023 at 03:00):

brendandburns requested alexcrichton for a review on PR #6091.

Wasmtime GitHub notifications bot (Apr 13 2023 at 03:00):

brendandburns requested wasmtime-fuzz-reviewers for a review on PR #6091.

Wasmtime GitHub notifications bot (Apr 13 2023 at 03:00):

brendandburns requested abrown for a review on PR #6091.

Wasmtime GitHub notifications bot (Apr 13 2023 at 03:00):

brendandburns requested wasmtime-compiler-reviewers for a review on PR #6091.

Wasmtime GitHub notifications bot (Apr 13 2023 at 03:00):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 13 2023 at 03:01):

brendandburns edited PR #6091:

This builds on top of #5929 to add an implementation of http serving.

~A server.c example is also added.~

This is a basic implementation, but ready for feedback/review.

cc @pchickey

Wasmtime GitHub notifications bot (Apr 15 2023 at 15:17):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 15 2023 at 15:27):

brendandburns edited PR #6091:

This builds on top of #5929 to add an implementation of http serving.

~A server.c example is also added.~

This is a basic implementation, but ready for feedback/review.

Known limitations:

Single-threaded for now. It can only handle one request at a time

Server is only on localhost:8080 and is not currently configurable

Memory state is not preserved between calls to the server.

The main method of your HTTP serving WASM must exit before the server starts (no infinite loops)

There are some interesting questions about what we actually want wasi-http serving to mean revealed by this implementation.

Specifically, three interesting questions come to mind.

How do we want to configure the server that is started (listening host/port), currently there are no flags to wasmtime that are specific to a module. I think using an http_config.toml file similar to what the cache settings does makes the most sense, but interested in other's thoughts.

What is the relationship between serving and the main/_start method in the WASM module. If you request an HTTP server, should the main run? Should the server run in parallel to the main? Currently it does run the main, but it expects it to exit before the server starts up.

Currently there's no memory persistence between calls to the request handler, static variables are re-initialized with each call. I _believe_ that this is because the main has exited and so the memory is recreated fresh each time. What do we expect the memory lifespan inside the module to be.

It's quite possible that some of these questions are better asked in the wasi-http spec repo rather than this implementation, happy to take the discussion there. It's related to the discussion about requests here: https://github.com/WebAssembly/wasi-http/issues/24

cc @pchickey

Wasmtime GitHub notifications bot (Apr 25 2023 at 16:39):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Apr 25 2023 at 22:56):

pchickey submitted PR review:

I left some idiom comments, which are pretty minor overall.

There is, however, one much bigger ask I have from this PR: now that wasmtime is running inside a tokio executor, we need to change to running wasmtime with its async API: set Config::async_support(true), and then swap out instantiate for instantiate_async, call for call_async. Then, for all of wasi-http's calls into the host, change the bindgen to set async = true and then remove all the run-inside-tokio wrappers in the synchronous bodies of the import functions.

Also, we need some tests to show this works. An echo server like https://github.com/bytecodealliance/wasmtime/blob/main/crates/test-programs/tests/http_tests/runtime/wasi_http_tests.rs#L14-L22 would be one good test program, and then a simple proxy that exercises the client http inside the server context as well.

Wasmtime GitHub notifications bot (Apr 25 2023 at 22:56):

pchickey submitted PR review:

I left some idiom comments, which are pretty minor overall.

There is, however, one much bigger ask I have from this PR: now that wasmtime is running inside a tokio executor, we need to change to running wasmtime with its async API: set Config::async_support(true), and then swap out instantiate for instantiate_async, call for call_async. Then, for all of wasi-http's calls into the host, change the bindgen to set async = true and then remove all the run-inside-tokio wrappers in the synchronous bodies of the import functions.

Also, we need some tests to show this works. An echo server like https://github.com/bytecodealliance/wasmtime/blob/main/crates/test-programs/tests/http_tests/runtime/wasi_http_tests.rs#L14-L22 would be one good test program, and then a simple proxy that exercises the client http inside the server context as well.

Wasmtime GitHub notifications bot (Apr 25 2023 at 22:56):

pchickey created PR review comment:

Idiom: this constructor is better phrased as an impl From<hyper::Method for crate::types::Method

Wasmtime GitHub notifications bot (Apr 25 2023 at 22:56):

pchickey created PR review comment:

            _ => Method::Other(m.as_str().to_string()),

Wasmtime GitHub notifications bot (Apr 25 2023 at 22:56):

pchickey created PR review comment:

            s => crate::types::Scheme::Other(s.to_string()),

Wasmtime GitHub notifications bot (Apr 25 2023 at 22:56):

pchickey created PR review comment:

I don't like when clones are hidden inside unassuming impls like From, and From<&mut _> is unidiomatic as well. impl Stream { fn as_bytes(&self) -> Self { Bytes::from(self.data.clone()) }} would be a better way to write this one

Wasmtime GitHub notifications bot (Apr 25 2023 at 22:56):

pchickey created PR review comment:

Unfortunately this one is tedious to implement properly, and its probably OK to leave it out for a little bit until we switch over to using the component model runtime
            Method::Other(_) => unimplemented!("Method::Other"),

Wasmtime GitHub notifications bot (Apr 26 2023 at 20:58):

pchickey edited PR review comment.

Wasmtime GitHub notifications bot (May 12 2023 at 20:30):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (May 12 2023 at 20:40):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (May 12 2023 at 23:53):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (May 31 2023 at 16:47):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Jun 02 2023 at 02:24):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Jun 02 2023 at 02:24):

brendandburns updated PR #6091.

Wasmtime GitHub notifications bot (Sep 05 2023 at 23:23):

brendandburns closed without merge PR #6091.

Last updated: Nov 22 2024 at 17:03 UTC