Skip to content

perf(request): cache method key#319

Open
yusukebe wants to merge 1 commit intov2from
perf/request-cache-methodkey
Open

perf(request): cache method key#319
yusukebe wants to merge 1 commit intov2from
perf/request-cache-methodkey

Conversation

@yusukebe
Copy link
Member

@yusukebe yusukebe commented Mar 12, 2026

Caching the method-key to improving performance.

@yusukebe
Copy link
Member Author

Hey @usualoma

I realized that the current v2 branch is not fast on Ping and Query benchmarks. The following is the result of comparing the v2 and the current npm:

============================================================
BENCHMARK RESULTS
============================================================

| Benchmark         | npm            | dev            | Difference  |
| ----------------- | -------------- | -------------- | ----------- |
| Average           | 76,276.42      | 83,202.77      | +9.08%      |
| Ping (GET /)      | 97,463.59      | 97,195.56      | -0.28%      |
| Query (GET /id)   | 94,839.04      | 94,642.80      | -0.21%      |
| Body (POST /json) | 36,526.62      | 57,769.94      | +58.16%     |

I think the changes like #310 decrease the performance. If so, it's a waste to use the changes.

But there are still some points for performance improvement.

The first one is this PR. Plus, another one is #320

npm vs PR #319 (this PR)

============================================================
BENCHMARK RESULTS
============================================================

| Benchmark         | npm            | dev            | Difference  |
| ----------------- | -------------- | -------------- | ----------- |
| Average           | 76,151.65      | 84,026.50      | +10.34%     |
| Ping (GET /)      | 97,012.20      | 98,769.61      | +1.81%      |
| Query (GET /id)   | 94,844.98      | 95,605.70      | +0.80%      |
| Body (POST /json) | 36,597.76      | 57,704.20      | +57.67%     |

npm vs PR #320

============================================================
BENCHMARK RESULTS
============================================================

| Benchmark         | npm            | dev            | Difference  |
| ----------------- | -------------- | -------------- | ----------- |
| Average           | 76,659.70      | 88,721.47      | +15.73%     |
| Ping (GET /)      | 97,443.60      | 101,423.13     | +4.08%      |
| Query (GET /id)   | 95,915.76      | 102,000.37     | +6.34%      |
| Body (POST /json) | 36,619.74      | 62,740.92      | +71.33%     |

Combining both PR will make it faster. I think it's enough as an "appeal point" for the major release.

What do you think of it? If it makes sense for you, please review the PRs.

@yusukebe yusukebe added the v2 label Mar 12, 2026
@usualoma
Copy link
Member

Hi @yusukebe!

Thanks for the follow-up!
You're right, my code wasn't great.

Given this approach, since method() gets called at least once in the request, I think it's better to generate it upfront rather than using ||=.

diff --git i/src/request.ts w/src/request.ts
index 67c29f7..f88b98e 100644
--- i/src/request.ts
+++ w/src/request.ts
@@ -367,7 +367,7 @@ const readBodyDirect = (request: Record<string | symbol, any>): Promise<Buffer>
 
 const requestPrototype: Record<string | symbol, any> = {
   get method() {
-    return (this[methodKey] ||= normalizeIncomingMethod(this[incomingKey].method))
+    return this[methodKey]
   },
 
   get url() {
@@ -536,6 +536,7 @@ export const newRequest = (
 ) => {
   const req = Object.create(requestPrototype)
   req[incomingKey] = incoming
+  req[methodKey] = normalizeIncomingMethod(incoming.method)
 
   const incomingUrl = incoming.url || ''
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants