-
Notifications
You must be signed in to change notification settings - Fork 31k
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
Remove duplicated code #27985
Remove duplicated code #27985
Conversation
Hello @jahirfiquitiva and thank you for the contribution! I've gone ahead and started an automated benchmark compare job, I'm interested to see what it comes up with... Good luck 🤞 |
Seems like that duplicate code still has a performance benefit. Summary of only significant results.
All results FTR - https://gist.github.com/refack/abe23c6941cf9599f46146296e331cf0 |
Interesting ... Thanks for sharing @refack |
Yes, this seems to noticeably slow down small buffer writes with all specified encodings, except for UCS-2, and to improve the speed for UCS-2 writes somehow. Probably because it's the only one that was not normalized to |
Understood... I'll let you take the final decision 🙂 |
Hi, @jahirfiquitiva! Thanks again for the pull request. I'm going to close this one out, but I hope this doesn't discourage you from improving the code base in other ways! |
Description
I was checking at the source code of the
buffer.js
file, and noticed that certain parts of the code could be simplified.Basically, you were checking for some lowercase expected text and then doing "
something
". If the text was not lowercase, you would make it lowercase and then do the exact same check and the exact same "something
".So, I thought, why not ensuring the text is already lowercase and then do the checks only once?
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesNotes
Unfortunately I didn't read or noticed the Pull Request contribution guidelines before creating it, so I hope there's no issue into getting the changes merged (if they deserve to be merged).
Thanks a ton