Skip to content

perf(ext/http): Reduce size of ResponseBytesInner #24840

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

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

nathanwhit
Copy link
Member

@nathanwhit nathanwhit commented Aug 1, 2024

I noticed set_response_body was unexpectedly hot in profiles, with most of the time being spent in memmove.

It turns out that ResponseBytesInner was massive (5624 bytes), so every time we moved a ResponseBytesInner (for instance in set_response_body) we were doing a >5kb memmove, which adds up pretty quickly.

This PR boxes the two larger variants (the compression streams), shrinking ResponseBytesInner to a reasonable 48 bytes.


Benchmarked with a simple hello world server:

// hello-server.ts
Deno.serve((_req) => {
  return new Response("Hello world");
});
// run with `deno run -A hello-server.ts`
// in separate terminal `wrk -d 10s http://127.0.0.1:8000`

Main:

Running 10s test @ http://127.0.0.1:8000/
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    53.39us    9.53us   0.98ms   92.78%
    Req/Sec    86.57k     3.56k   91.58k    91.09%
  1739319 requests in 10.10s, 248.81MB read
Requests/sec: 172220.92
Transfer/sec:     24.64MB

This PR:

Running 10s test @ http://127.0.0.1:8000/
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    45.44us    8.49us   0.91ms   90.04%
    Req/Sec   100.65k     2.26k  102.65k    96.53%
  2022296 requests in 10.10s, 289.29MB read
Requests/sec: 200226.20
Transfer/sec:     28.64MB

So a nice ~15% bump. (With response body compression, the gain is ~10% for gzip and neutral for brotli)

@nathanwhit nathanwhit changed the title perf(http): Reduce size of ResponseBytesInner perf(ext/http): Reduce size of ResponseBytesInner Aug 1, 2024
@bartlomieju
Copy link
Member

I see 8-9% speed up on M1 Max:
main

wrk -d 10s --latency http://localhost:8000
Running 10s test @ http://localhost:8000
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    75.96us   68.31us   3.40ms   99.36%
    Req/Sec    60.72k     2.53k   62.38k    97.03%
  Latency Distribution
     50%   73.00us
     75%   84.00us
     90%   97.00us
     99%  131.00us
  1220434 requests in 10.10s, 174.58MB read
Requests/sec: 120836.25
Transfer/sec:     17.29MB

This PR:

wrk -d 10s --latency http://localhost:8000
Running 10s test @ http://localhost:8000
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    67.71us   34.28us   3.12ms   92.86%
    Req/Sec    65.45k     1.55k   67.33k    93.07%
  Latency Distribution
     50%   68.00us
     75%   77.00us
     90%   89.00us
     99%  115.00us
  1315388 requests in 10.10s, 188.17MB read
Requests/sec: 130242.23
Transfer/sec:     18.63MB

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

@nathanwhit nathanwhit enabled auto-merge (squash) August 1, 2024 23:53
@nathanwhit nathanwhit merged commit 930ccf9 into denoland:main Aug 2, 2024
17 checks passed
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.

2 participants