Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.Sign up
timers: Avoid linear scan in _unrefActive. #8751
Before this change, _unrefActive would keep the unrefList sorted when
Because _unrefActive is called extremely frequently, this linear scan
This commit changes _unrefActive so that it doesn't try to keep the
However, when a timer expires, unrefTimeout has to go through the whole
It is usually not large enough to have a significant impact on
With this change, _unrefActive does not show as a significant
I propose this for v0.10.34 inclusion, the heap seems like a large change for a stable and timer firing is mostly a concern when things are already going haywire, the problem (such as it is) is that we shouldn't penalize a well behaved system for the pessimistic case -- thoughts?
@orangemocha there's no doubt we want to use a heap eventually, the question is how can we improve performance while minimizing surface area change for a stable release, while also not further delaying anything around v0.12.
It's uncontroversial to want to use a heap here, and I strongly advocate it for v0.13 [and if it becomes critical bringing it back down for v0.12].
In my opinion the current
This PR also makes it possible to experience performance issues, in the sense that on every unref timer timeout, a node process will have to iterate through the whole list all the time. However, that happens only if connections time out, whereas the current implementation penalizes any node process with a high number of concurrent connections, whether they time out or not.
Either way, I think one could mitigate these performance issues by for instance scaling horizontally.
Like @tjfontaine said, there is no question that the heap implementation is superior in terms of algorithmic complexity, but I think this change brings some significant performance improvement while minimizing changes.