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

feat: split WebSocket #48

Merged
merged 8 commits into from
Oct 30, 2023
Merged

feat: split WebSocket #48

merged 8 commits into from
Oct 30, 2023

Conversation

mmastrac
Copy link
Contributor

@mmastrac mmastrac commented Sep 20, 2023

Fixes #40. Working on this in concert with denoland/deno#20579 and #49

This API is not final

To create a split WebSocket, you can either call:

From the WebSocketRead and WebSocketWrite halves, you'll need to ensure that obligated writes from WebSocketRead::read_frame are passed to write_frame. This is done by passing a closure that returns a future.

Note that a FragmentCollectorRead may be created from a WebSocketRead in the same way that FragmentCollector may be created from a WebSocket.

In the case of Deno's split socket, the read process looks like this:

  let mut ws = RcRef::map(&resource, |r| &r.ws_read).borrow_mut().await;
  let writer = RcRef::map(&resource, |r| &r.ws_write);
  let mut sender = move |frame| {
    let writer = writer.clone();
    async move { writer.borrow_mut().await.write_frame(frame).await }
  };
  loop {
    let res = ws.read_frame(&mut sender).await;

@mmastrac mmastrac changed the title [WIP] feat: split WebSocket feat: split WebSocket Oct 28, 2023
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for putting a cargo feature to gate it 👍 good idea

@littledivy
Copy link
Member

littledivy commented Oct 30, 2023

@mmastrac Code looks good but this example server is not working:

use fastwebsockets::upgrade;
use fastwebsockets::OpCode;
use fastwebsockets::WebSocketError;
use hyper::server::conn::Http;
use hyper::service::service_fn;
use hyper::Body;
use hyper::Request;
use hyper::Response;
use tokio::net::TcpListener;

async fn handle_client(fut: upgrade::UpgradeFut) -> Result<(), WebSocketError> {
  let ws = fut.await?;
  let (mut rx, mut tx) = ws.split(|ws| tokio::io::split(ws));

  loop {
    // Empty send_fn is fine because the benchmark does not create obligated writes.
    let frame = rx.read_frame(&mut move |_| async { Ok::<_, WebSocketError>(()) }).await?;
    match frame.opcode {
      OpCode::Close => break,
      OpCode::Text | OpCode::Binary => {
        tx.write_frame(frame).await?;
      }
      _ => {}
    }
  }

  Ok(())
}
async fn server_upgrade(
  mut req: Request<Body>,
) -> Result<Response<Body>, WebSocketError> {
  let (response, fut) = upgrade::upgrade(&mut req)?;

  tokio::task::spawn(async move {
    if let Err(e) = tokio::task::unconstrained(handle_client(fut)).await {
      eprintln!("Error in websocket connection: {}", e);
    }
  });

  Ok(response)
}

fn main() -> Result<(), WebSocketError> {
  let rt = tokio::runtime::Builder::new_current_thread()
    .enable_io()
    .build()
    .unwrap();

  rt.block_on(async move {
    let listener = TcpListener::bind("127.0.0.1:8080").await?;
    println!("Server started, listening on {}", "127.0.0.1:8080");
    loop {
      let (stream, _) = listener.accept().await?;
      println!("Client connected");
      tokio::spawn(async move {
        let conn_fut = Http::new()
          .serve_connection(stream, service_fn(server_upgrade))
          .with_upgrades();
        if let Err(e) = conn_fut.await {
          println!("An error occurred: {:?}", e);
        }
      });
    }
  })
}

Running load_test:

divy@mini ~/g/f/benches (split) [SIGINT]> ./load_test 10 0.0.0.0 8080 0 0
Running benchmark now...
ERROR: outstanding bytes negative!⏎              
divy@mini ~/g/fastwebsockets (split)> target/release/examples/split_server
Server started, listening on 127.0.0.1:8080
Client connected
Client connected
Client connected
Client connected
Client connected
Client connected
Client connected
Client connected
Client connected
Client connected
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Connection reset by peer (os error 54)
Error in websocket connection: Unexpected EOF

@mmastrac
Copy link
Contributor Author

I can repro that problem here. Digging into it.

@mmastrac
Copy link
Contributor Author

@littledivy Ah. It's because there wasn't a fragment collector on the read half. I will add some documentation around this.

@mmastrac
Copy link
Contributor Author

Ran the two benchmarks and it looks pretty good:

Split:

✘-INT ~/Documents/github/deno/deno_third_party [master|✔] 
10:01 $ prebuilt/mac/load_test 10 0.0.0.0 8080 0 0
Running benchmark now...
Msg/sec: 112289.000000
Msg/sec: 114351.750000
Msg/sec: 115550.500000
Msg/sec: 115966.750000
Msg/sec: 114090.500000
Msg/sec: 116488.000000
^C

Unsplit

✘-INT ~/Documents/github/deno/deno_third_party [master|✔] 
10:07 $ prebuilt/mac/load_test 10 0.0.0.0 8080 0 0
Running benchmark now...
Msg/sec: 113805.250000
Msg/sec: 114671.000000
Msg/sec: 114437.250000
Msg/sec: 115879.000000
Msg/sec: 116060.000000
Msg/sec: 113821.500000
^C

Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM!

@littledivy littledivy merged commit 88a15b5 into denoland:main Oct 30, 2023
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.

Add function to split a WebSocket into a read half and a write half
3 participants