Re: [dpdk-announce] important design choices - statistics - ABI

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Wednesday, June 17, 2015 12:17 PM
> To: Matthew Hall
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-announce] important design choices -
> statistics - ABI
> 
> On Tue, Jun 16, 2015 at 09:36:54PM -0700, Matthew Hall wrote:
> > On Wed, Jun 17, 2015 at 01:29:47AM +0200, Thomas Monjalon wrote:
> > > There were some debates about software statistics disabling.
> > > Should they be always on or possibly disabled when compiled?
> > > We need to take a decision shortly and discuss (or agree) this proposal:
> > > 	http://dpdk.org/ml/archives/dev/2015-June/019461.html
> >
> > This goes against the idea I have seen before that we should be moving
> toward
> > a distro-friendly approach where one copy of DPDK can be used by multiple
> apps
> > without having to rebuild it. It seems like it is also a bit ABI hostile
> > according to the below goals / discussions.
> >
> > Jemalloc is also very high-performance code and still manages to allow
> > enabling and disabling statistics at runtime. Are we sure it's impossible for
> > DPDK or just theorizing?
> >
> 
> +1 to this. I think that any compile-time option to disable stats should only
> be used when we have a proven performance issue with just disabling them
> at runtime.
> I would assume that apps do not switch on or off stats multiple times per
> second,
> so any code branches to track stats or not would be entirely predictable in
> the
> code - since they always go one way. Therefore, when disabledi, we should
> be looking
> at a very minimal overhead per stat. If there are lots of checks for the same
> value in the one path, i.e. lots of stats in a hot path, hopefully the compiler
> will be smart enough to make the check just once. If not, we can always do
> that in the C code by duplicating the hotpath code for with or without stats
> cases -
> again selectable at runtime.
> 

I see where you're coming from, but reality is you cannot guarantee that the few conditional branches in the library are going to be predicted correctly simply because the application and the other libraries used by the app have an unknown number of conditional branches themselves. For complex apps, the total number of conditional branches is large and the more there are, the lesser probability of such a branch being predicted correctly is. I agree that for test apps like l3fwd with just a few branches the probability to have library branches being predicted correctly is high, but for me is an incorrect proof point. The cost of ~14 cycles per branch misprediction is important for DPDK packet budgets.

Since we don't control the application, I am feeling very uncomfortable with generic statements about how application is likely to execute and the impact of library conditional branches over the application should be. To me, they sound like we are willing to take chances, and to me this is not the right decision. In my opinion, the right decision for a significant framework like DPDK is to keep all options open for the apps: keep counters always enabled, keep counters always disabled, keep counters enabled first and disabled later. I suggest we should move the focus from WHY arguments like: "I don't think anybody will ever need this this" to the HOW part of making sure that technical solution we pick is correct and keeps all options open.

I think stats are not always equivalent to incrementing a counter. In the guideline proposal, I am providing several examples of more complex statistics logic that is more than just inc one counter. As an extreme case, think about the case where the metric to compute requires complex math like prediction of next packet arrival time based on recent history, etc. When defining a policy, we should consider a broad spectrum of metrics, not just n_pkts_in.


> Also, there is also the case where the stats tracking itself is such low
> overhead
> that its not worth disabling. In that case, neither runtime nor compile-time
> disabling
> should need to be provided. For example, any library that is keeping a track
> of
> bursts of packets should not need to have that stat disable option - one
> increment
> or addition per burst of (32) packets is not going to seriously affect any app. :-
> )
> 
> Regards,
> /Bruce