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

perf(http): make heap allocation for path conditional #26289

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

dsherret
Copy link
Member

Code:

Deno.serve({ port: 8085 }, request => {
  return new Response(request.url);
});

Before:

% wrk -d60s http://localhost:8085/path/testing\?testing=5
Running 1m test @ http://localhost:8085/path/testing?testing=5
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    56.01us   18.34us   3.28ms   93.84%
    Req/Sec    81.80k     3.13k   88.26k    90.77%
  9783713 requests in 1.00m, 1.67GB read
Requests/sec: 162789.89
Transfer/sec:     28.41MB

After:

% wrk -d60s http://localhost:8085/path/testing\?testing=5
Running 1m test @ http://localhost:8085/path/testing?testing=5
  2 threads and 10 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    55.44us   15.20us   2.42ms   90.41%
    Req/Sec    82.71k     2.92k   88.10k    89.93%
  9892916 requests in 1.00m, 1.69GB read
Requests/sec: 164607.06
Transfer/sec:     28.73MB

Cow::Borrowed(path)
} else {
Cow::Owned(format!("/{}", path))
}
Copy link
Member Author

Choose a reason for hiding this comment

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

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

@dsherret dsherret merged commit 661882f into denoland:main Oct 16, 2024
17 checks passed
@dsherret dsherret deleted the perf_heap_allocation_path branch October 16, 2024 04:43
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