timers: clean up, async integration for unref, simplify clearTimeout
Assorted, small changes to timers:
-
A recent commit removed the usage of the second argument of
tryOnTimeout
but left the definition in place. Remove it. -
It's possible for user-code to flip an existing timeout to be an interval during its execution, in which case the current code would crash due to
start
being undefined. Fix this by providing a defaultstart
value withinrearm
. -
When async hooks integration for Timers was introduced, it was not included in the code for unref'd or subsequently ref'd timers which means those timers only have TIMERWRAP hooks.
-
Remove dead code from timeout & interval clearing —
timer[kOnTimeout]
is not something that can exist, askOnTimeout
only exists onTimerWrap
.
The first two commits are semver-major as they rely on several prior semver-major PRs, the other two can both go into 8.x & 9.x and I will handle back-porting these sometime later this month.
Benchmark results:
timers/timers-breadth.js thousands=5000 -0.49 % ±3.13% ±4.13% ±5.31%
timers/timers-cancel-pooled.js millions=5 *** 1643.69 % ±84.28% ±111.55% ±144.05%
timers/timers-cancel-unpooled.js millions=1 *** 101.54 % ±9.13% ±12.06% ±15.51%
timers/timers-depth.js thousands=1 -0.01 % ±0.09% ±0.12% ±0.15%
timers/timers-insert-pooled.js millions=5 0.43 % ±2.05% ±2.71% ±3.48%
timers/timers-insert-unpooled.js millions=1 0.80 % ±3.13% ±4.13% ±5.30%
timers/timers-timeout-pooled.js millions=10 -2.38 % ±4.92% ±6.49% ±8.34%
Checklist
-
make -j4 test
(UNIX), orvcbuild test
(Windows) passes -
tests and/or benchmarks are included -
commit message follows commit guidelines
Affected core subsystem(s)
timers, test