core.link: Streamline counters [DRAFT] #570
Conversation
Reduce the amount of counter-related code and data in `struct link`. The ring indexes (read, write) and now upgraded to 64-bit integers that do not wrap around [*]. This means that we don't need to use separate packet tx/rx counters because the ring indexes serve that purpose directly. (We do have to be careful to handle wrap-around when actually looking up elements in the ring based on the index.) The byte counters are simply removed. Do we really need them? Dropped packets are still counted directly because this is important information and I don't immediately see a way to derive it indirectly. [*] 64-bit packet counters really should not wrap around. Even at one billion packets per second it would take more than 500 years. However, if this is not a safe assumption then we could detect wrap around and account for it on the counters. (Google: "2^64/(1 billion per second" => "584.554531 years".)
|
I think But in all, is there a measurable difference? I remember doing some benchmarks and finding no difference between and even if the first form does a check on every iteration, while the second one does only at the start and it's an integer count from there on. I guess the index pointers become essentially free once they get cached in. |
|
I am always for simplicity but I wonder what is the benefit here? Did rx/txbytes actually cost us much? It seems like an interesting metric to me so I would consider keeping it. |
|
@eugeneia That is a fair point. I see no urgent need to make this change now. We could revisit this idea in the future if/when we are trying to squeeze more performance out of the basic app network and see if it really helps. I close this PR for now. |
Reduce the amount of counter-related code and data in
struct link.The ring indexes (read, write) and now upgraded to 64-bit integers that do not wrap around [*]. This means that we don't need to use separate packet tx/rx counters because the ring indexes serve that purpose directly. (We do have to be careful to handle wrap-around when actually looking up elements in the ring based on the index.)
The byte counters are simply removed. Do we really need them? (How about if we tracked only packets-per-second plus perhaps a system-wide average packet size?)
Dropped packets are still counted directly because this is important information and I don't immediately see a way to derive it indirectly.
[*] 64-bit packet counters really should not wrap around. Even at one billion packets per second it would take more than 500 years. However, if this is not a safe assumption then we could detect wrap around and account for it on the counters.
(Google: "2^64/(1 billion per second)" => "584.554531 years".)
@eugeneia @javierguerragiraldez @alexandergall what do you guys thing about this idea? I mean in terms of information loss, impact on the code, and conflicts with other open PRs?