-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: replace util._extend with object.assign #7208
Conversation
`Object.assign` is built-in and has slight performance advantage- over the polyfill `_extend`. One thing to keep an eye is that `._extend` used to silently fails- while `assign` throws an error.
Do you have some benchmarks to back that up? That wasn't the case, as of #4593. |
Also see this similar PR that was submitted just the other week. |
I will close this However, would like to hear if anyone has any thoughts on the benchmark findings using the attached script. |
I think it was the nature of the data you were operating on. I switched to
It might be good to have a benchmark in the repo for this, since it keeps coming up. |
@cjihrig this is interesting. With your |
@suryagh can I suggest opening a PR to create a benchmark for this? cc: @nodejs/benchmarking |
@cjihrig will do. |
Checklist
make -j4 test
(UNIX) orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
http, tls, child-process, cluster, domain, fs, https, tty
Description of change
Object.assign
is built-in and has some performance advantage-over the polyfill.
One thing to keep an eye is that
_extend
used to fail silently-while
assign
throws an error. Let me know if there are any concerns-or if the
pr
need to be split based on the components.