-
-
Notifications
You must be signed in to change notification settings - Fork 8
Description
About the fast path
In the code of process.emitWarning, deprecations are ignored when --no-deprecation flag is passed (reference).
Instead of creating the warning object and then discarding when the flag is true, we could just check earlier and just skip the rest of the method when type is DeprecationWarning.
There are some places that could benefit from this tiny change.
Going further with improvements
What if we add guards to those places to avoid any computation at all when --no-deprecation is true?
I did some experiments with esm internal resolve and by putting an if guard for process.noDeprecation on line 106, and the results is not that bad:
confidence improvement accuracy (*) (**) (***)
esm/cjs-parse.js n=100 -0.69 % ±2.91% ±3.91% ±5.16%
esm/esm-loader-defaultResolve.js specifier='./relative-existing.js' n=1000 0.76 % ±4.52% ±6.12% ±8.20%
esm/esm-loader-defaultResolve.js specifier='./relative-nonexistent.js' n=1000 0.26 % ±4.80% ±6.47% ±8.62%
esm/esm-loader-defaultResolve.js specifier='node:os' n=1000 * 7.18 % ±6.08% ±8.14% ±10.71%
esm/esm-loader-defaultResolve.js specifier='node:prefixed-nonexistent' n=1000 -2.21 % ±6.18% ±8.30% ±10.96%
esm/esm-loader-defaultResolve.js specifier='unprefixed-existing' n=1000 * 9.70 % ±7.34% ±9.84% ±12.96%
esm/esm-loader-defaultResolve.js specifier='unprefixed-nonexistent' n=1000 1.14 % ±2.07% ±2.77% ±3.64%
Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 7 comparisons, you can thus
expect the following amount of false-positive results:
0.35 false positives, when considering a 5% risk acceptance (*, **, ***),
0.07 false positives, when considering a 1% risk acceptance (**, ***),
0.01 false positives, when considering a 0.1% risk acceptance (***)
node benchmark/run.js esm
I think the results could be even better, this benchmark is a little bit flaky as you can see by the accuracy.
What about --no-warnings?
Well, at first I thought we could do the same for --no-warnings, add an if statement and avoid calling process.emitWarning, but NodeJS just unsubscribe the listener when --no-warnings is set , but they won't stop the event from being emitted, also, there are MANY places where process.on('warning is used, so I don't think we could do anything here.
Conclusion
So, in the end, we have two questions:
- Does it worth moving the
noDeprecationbefore the object creation? - Does it worth adding fast path for every location that emits deprecations?