Skip to content

timers: clean up, async integration for unref, simplify clearTimeout

Assorted, small changes to timers:

  1. A recent commit removed the usage of the second argument of tryOnTimeout but left the definition in place. Remove it.

  2. 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 default start value within rearm.

  3. 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.

  4. Remove dead code from timeout & interval clearing — timer[kOnTimeout] is not something that can exist, as kOnTimeout only exists on TimerWrap.

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), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

timers, test

Merge request reports

Loading