search

go-ping / ping

ICMP Ping library for Go
MIT License
1.33k stars 345 forks source link

Compute all statistics as packets are received #150

Closed jraby closed 3 years ago

jraby commented 3 years ago

This PR implements Welford's online algorithm to compute the standard deviation of the round trip times as packets are received without storing the full list. (the average is taken from this algorithm too) As for min and max, they are also computed as we go.

I didn't touch the rtts array since it is part of the public interface of the Statistics, but with this PR it is not needed to get the stats.

I wasn't sure what to do about the tests which set p.PacketsRecv and then filled p.rtts with test values. Since the packet count is an inherent part of the algorithm, I opted to move the increment into the updateStatistic method and repeatedly call it with test data.

One other thing: I think calling Statistics() from another goroutine could yield incorrect results since updateStatistics() could be running at the same time. Adding a RWLock should fix that. (I haven't done it yet since I do not use Pinger in this fashion and this particular problem was present in the original code where rtts can be modified while Statistics() is called)

There's also a commit that renames outPkt to inPkt in processPacket since that function is dealing with received packets.

(And this is all branched off b92053f because otherwise go-ping doesn't work on linux at the moment)

It might be easier to review commit by commit since they are self contained.

SuperQ commented 3 years ago

Thanks, this is great.

Yes, we do want to keep Pinger.rtts, as some people use this library to get all of them for later processing (for example, calculating quantiles). But adding the live stats update makes it so we don't need the full rtts list to calculate simple stats.