module: use optional chaining in cjs/loader.js - #37238
Conversation
Sorry, something went wrong.
Sorry, something went wrong.
|
Benchmark CI for module: 20:33:02 module/module-loader-deep.jscache='false' files=1000 ext='.js' * -3.51 % ±3.25% ±4.32% ±5.63% 20:33:02 module/module-loader.jscache='false' n=1 files=500 dir='rel' name='/index.js' * -3.80 % ±2.97% ±3.95% ±5.14% 20:33:02 module/module-loader.jscache='true' n=1000 files=500 dir='abs' name='/' * -1.81 % ±1.39% ±1.86% ±2.44% |
Sorry, something went wrong.
aduh95
left a comment
There was a problem hiding this comment.
LGTM if benchmark results don't show perf regression
Sorry, something went wrong.
|
Non-blocking from me, but I think whether the benchmarks look good in this case may be a matter of opinion. It shows three statistically-significant (but also, yes, small) regressions. Might be interesting to run again to see if they are persistent or not. Benchmark CI re-run for comparison: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/937/ |
Sorry, something went wrong.
|
Benchmark re-run confirms no significant changes in benchmark results. |
Sorry, something went wrong.
PR-URL: nodejs#37238 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Zijian Liu <lxxyxzj@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #37238 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Zijian Liu <lxxyxzj@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
No description provided.