Remove duplicated code#27985
Remove duplicated code#27985jahirfiquitiva wants to merge 2 commits intonodejs:masterfrom jahirfiquitiva:buffer
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.jsfile, 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