http: improve performance for incoming headers #26041
Conversation
lib/_http_incoming.js
Outdated
| @@ -137,130 +137,101 @@ function _addHeaderLines(headers, n) { | |||
|
|
|||
| // 'array' header list is taken from: | |||
| // https://mxr.mozilla.org/mozilla/source/netwerk/protocol/http/src/nsHttpHeaderArray.cpp | |||
There was a problem hiding this comment.
I think this comment might be outdated now? The link is dead at any rate.
There was a problem hiding this comment.
This link is redirected to here. But I think it might not be a good way to keep an unstable url in comment. Should I remove it?
There was a problem hiding this comment.
I'd be okay with removing it.
| if (lowercased) { | ||
| return '\u0000' + field; | ||
| } else { | ||
| return matchKnownFields(field.toLowerCase(), true); |
There was a problem hiding this comment.
It might be slightly faster to use iteration than recursion. it might also be a wash, however. :-)
There was a problem hiding this comment.
I've run the benchmark of iteration and found there was no difference between them.
lib/_http_incoming.js
Outdated
| return 'expires'; | ||
| case 'Set-Cookie': | ||
| case 'set-cookie': | ||
| if (field === 'If-Match' || field === 'if-Match') |
There was a problem hiding this comment.
Isn't this supposed to be if-match instead of if-Match?
There was a problem hiding this comment.
Yes, I'll change it.
|
CI before landing: https://ci.nodejs.org/job/node-test-pull-request/20749/ |
|
Landed in da0dc51 |
PR-URL: nodejs#26041 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
PR-URL: #26041 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
This improves the performance of
matchKnownFields. This function is heavily used when parsing incoming http headers.Here is the benchmark result:
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes