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

Tracking issue for WebSocket performance #16315

Closed
8 of 20 tasks
littledivy opened this issue Oct 17, 2022 · 5 comments
Closed
8 of 20 tasks

Tracking issue for WebSocket performance #16315

littledivy opened this issue Oct 17, 2022 · 5 comments
Assignees
Labels
ext/websocket related to the ext/websocket crate perf performance related

Comments

@littledivy
Copy link
Member

littledivy commented Oct 17, 2022

Let's look at a simple ping/pong benchmark.

Deno.serve(req => {
  const { socket, response } = Deno.upgradeWebSocket(req);
  socket.onmessage = (e) => {
    socket.send(e.data);
  }
  return response;
}, { port: 8000 });
# Deno 1.26.1
Msg/sec: 74114.500000

# uWebSocket.js
Msg/sec: 259207.750000

The goal should be to have as little overhead as possible and be on par with uWebSocket.js and tokio-tungstenite. We should also integrate this test using uWebSockets/benchmarks/load_test.c in our CI workflow.

After applying the below changes, I see a ~3.5x improvement in the above benchmark:

# Deno (with below changes)
Msg/sec: 258291.750000

A very sloppy implementation of the above can be found here: main...littledivy:deno:websocket_perf

@littledivy littledivy added perf performance related ext/websocket related to the ext/websocket crate labels Oct 17, 2022
@littledivy littledivy self-assigned this Oct 17, 2022
littledivy added a commit that referenced this issue Oct 17, 2022
littledivy added a commit that referenced this issue Oct 17, 2022
Towards #16315 

It created a bunch of Error objects and rejected the promise. This patch
changes `op_sleep` to resolve with `true` if it was cancelled.
@uNetworkingAB
Copy link

Always good when perf. is getting better across the industry but I find it a bit underwhelming when I've been pointing this out long before Deno was even conceived, yet received not the slightest ack. or response until someone else (in this case Bun) decided to actually listen to the input given. Anyways - I hope Deno will improve here because it will make us all waste less resources doing common things.

@uNetworkingAB
Copy link

Btw, if you like load_test.c there is a similar one for http that performs much better than wrk (and hence gives more accurate numbers) under uSockets/examples/ it's called http_load_test.c.

That benchmark could sure see some improvements, but is pretty fine as is for finding perf. regressions or differences.

bartlomieju pushed a commit that referenced this issue Oct 17, 2022
bartlomieju pushed a commit that referenced this issue Oct 17, 2022
Towards #16315 

It created a bunch of Error objects and rejected the promise. This patch
changes `op_sleep` to resolve with `true` if it was cancelled.
littledivy added a commit that referenced this issue Oct 20, 2022
@littledivy
Copy link
Member Author

@bartlomieju
Copy link
Member

@littledivy should we open a new issue for tracking WebSocket performance? It appears most (all?) things listed here were already addressed

@littledivy
Copy link
Member Author

Yeah, I'll open a new one soon.

hardfist pushed a commit to hardfist/deno that referenced this issue Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext/websocket related to the ext/websocket crate perf performance related
Projects
None yet
Development

No branches or pull requests

3 participants