perf: optimize validate http token#4608
Conversation
|
This implementation has already been added to node. |
mcollina
left a comment
mcollina
left a comment
There was a problem hiding this comment.
Great work! How much do we gain?
|
|
||
| for (let i = 0; i < characters.length; i++) { | ||
| const c = characters.charCodeAt(i) | ||
| if (!isTokenCharCode(c)) { |
There was a problem hiding this comment.
Can you please remove the isTokeCharCode function and just inline the change?
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4608 +/- ##
=========================================
+ Coverage 0 92.93% +92.93%
=========================================
Files 0 106 +106
Lines 0 32971 +32971
=========================================
+ Hits 0 30640 +30640
- Misses 0 2331 +2331 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I didn't just take the number 12 for validation through a regular expression, it's after 12 characters that the problems start in the fast version, it's probably something to do with v8 optimization for short strings. Here is a benchmarking of the validation of strings from 8 to 20 characters 
|
metcoder95
left a comment
metcoder95
left a comment
There was a problem hiding this comment.
lgtm with Matteo's comment
Co-authored-by: Panda <panda@mail.ru>
4 participants
This relates to...
Header name validation performance
Changes
Status