fix: performance bottleneck in stat.js

[betamos]

Array.shift seems to be very slow, perhaps linear, on some engines, resulting in _update consuming a lot of CPU.

I transferred a large payload of 2.4GB between two machines running Node 12 on a local network and attached a Chrome profiler which showed a hotspot on the target machine of about 15s in this single line of code (the shift operation). The total transfer time was about 45s.

The CPU consumption was so high that the target machine was not able to consume the stream as new data was received, resulting in the TCP window filling up and causing network congestion. This was at speeds around 60MB/s on a high end MBP.

After this change, the time for the loop was down to about 1s. This might sound like a big improvement but 1s is still a lot for stats that are not important to all clients; perhaps consider allowing an option for turning stats off completely.

[jacobheun]

This looks good. A side effect of disabling stats would be that the connection manager won't have any idea of what priority connections should be pruned if limits are exceeded. For use cases where connection limits aren't really going to be an issue (fixed connections) there shouldn't be any ill effect of disabling stats.

I would be open to a PR that allows disabling stats.

[betamos]

Thanks for the swift response and merge. I didn't realize Stats was load bearing, then perhaps it is wiser to make some of the heavier features configurable at least.

Btw, yesterday it looked like the Windows tests were failing due to this change, but now they look fine. Any idea what happened?

[jacobheun]

Thanks for the swift response and merge. I didn't realize Stats was load bearing, then perhaps it is wiser to make some of the heavier features configurable at least.

Yeah, Stats definitely needs some love. We're going to be working on the Connection Manager soon and will hopefully work out a minimum set of needed stats (if any) that are required for it to manage connections properly. From there we can shut everything else off by default I think.

Btw, yesterday it looked like the Windows tests were failing due to this change, but now they look fine. Any idea what happened?

I should have mentioned I reran them. Due to some timing issues on the windows CI environment the tests can be flakey for stats with when they are checked. I'm planning to get that resolved when I go through it as part of the async refactor in the next couple of weeks.