-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
lib: micro-optimize url.resolve() #184
lib: micro-optimize url.resolve() #184
Conversation
Wow, neat! |
for (var i = index, k = i + 1, n = list.length; k < n; i += 1, k += 1) | ||
list[i] = list[k]; | ||
list.pop(); | ||
} |
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.
This is a nice trick to know about. Especially since splice is quite slow. :o
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.
The indenting is a bit off here.
Wait until you see the numbers for EventEmitter#removeListener(). :-) |
d3d8577
to
e12303f
Compare
@chrisdickinson Fixed the indentation and renamed the function to spliceOne(). PTAL. |
👍 |
LGTM |
Rename the url.parse() benchmark from url.js to url-parse.js. A follow-up commit is going to add another one for url.resolve(). PR-URL: nodejs#184 Reviewed-By: Chris Dickinson <[email protected]>
Replace the call to Array#splice() with a faster open-coded version that creates less garbage. Add a new benchmark to prove it. With the change applied, it scores about 5% higher and that is nothing to sneeze at. PR-URL: nodejs#184 Reviewed-By: Chris Dickinson <[email protected]>
e12303f
to
aff56cd
Compare
Replace the call to Array#splice() with a faster open-coded version
that creates less garbage.
Add a new benchmark to prove it. With the change applied, it scores
about 5% higher and that is nothing to sneeze at.
R=@chrisdickinson