-
-
Notifications
You must be signed in to change notification settings - Fork 22.3k
feat: do not modify the Content-Type twice when sending strings #6991
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
Conversation
ShubhamOulkar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Methodology
- Scenario:
res.send('<html>...</html>') - Iterations: 100,000
- Environment: Windows / Node.js
- Metric: Operations per second (ops/sec)
Results
| Metric | Before (Current) | After (Optimized) | Improvement |
|---|---|---|---|
| Total Time | ~501 ms | ~137 ms | ~3.6x Faster |
| Avg Op Time | 0.0050 ms | 0.0014 ms | -72% overhead |
| Throughput | ~200k ops/sec | ~728k ops/sec | +264% |
jonchurch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some nits
Phillip9587
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…set-twice-content-type
We were modifying the Content-Type twice when a manual Content-Type was not set (that is, without
res.set('Content-Type', 'text/plain').send('hey')), because it would enter here and then enter here again. This reduces how often we go into thecontent-typelibrary, adds a small performance improvement, and slightly improves CPU usage, building on all the validations thatexpress/lib/utils.js
Line 225 in 2cd372e
For those who do comparisons with a hello-world, this will improve Express’s results, we’re almost catching up to Fastify 😄
The following diffs are based on the simple hello-world example in this repo (https://github.com/expressjs/express/tree/master/examples/hello-world)
diff v4.22.1 vs this pr
diff v4.22.1 vs v5.2.1