RE: [PATCH dpdk 1/2] graph: always count objects and calls

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Robin Jarry [mailto:rjarry@redhat.com]
> Sent: Tuesday, 9 December 2025 13.46
> 
> Morten Brørup, Dec 09, 2025 at 13:11:
> >> My issue is that I would like the total_objs stat but without the
> >> overhead of rte_rdtsc() being called twice for every node visit.
> >
> > I get that. My proposal provides that.
> 
> I didn't get that from your previous message:
> 
> Morten Brørup, Dec 09, 2025 at 12:13:
> >>> Looking at patch 2/2, I disagree with the approach.
> >>>
> >>> RTE_LIBRTE_GRAPH_STATS should control all stats, incl. total_calls
> and total_objs.
> >>> Then, if enabled, the total_cycles stats can be controlled by
> rte_graph_cycle_stats_enable().
> >>>
> >>> Your v1 series introduces unnecessary overhead for applications not
> caring about total_calls/total_objs stats and thus built without
> RTE_LIBRTE_GRAPH_STATS.
> 
> Where do you propose a new flag to determine whether total_calls and
> total_objs are updated and total_cycles are not?

Sorry about not being more clear. I meant:
"Then, if [RTE_LIBRTE_GRAPH_STATS is] enabled [at build time], the total_cycles stats can be controlled by rte_graph_cycle_stats_enable()."

I proposed that you add your rte_graph_cycle_stats_enable() to control the total_cycles stats, and leave RTE_LIBRTE_GRAPH_STATS as it is now (i.e. to be able to disable all stats at build time).

> 
> >> And also, I would like to be able to enable/disable these stats *at
> >> runtime*.
> >
> > Why would you enable/disable total_calls/total_objs at runtime?
> 
> I don't want to disable/enable total_calls/total_objs. I want to
> disable/enable total_cycles at runtime.

OK. That makes sense.

> 
> >> Having it behind a compile time constant makes it very not flexible.
> >
> > Agree, but instrumentation has a performance cost, and should be
> > possible to disable at compile time.
> >
> >> I could have two booleans to control whether total_calls/total_objs
> are
> >> updated *and* whether total_cycles are computed. But that seems a
> bit
> >> overkill and it would mean two fields to check (two branches)
> instead
> >> of
> >> one per node.
> >>
> >> Is it really that bad to update two uint64_t counters?
> >
> > The cost is unnecessary. It's instrumentation.
> 
> In that instance, I consider total_objs not to be "instrumentation" but
> more like regular counters (e.g. ethdev stats, xstats, etc.). Unless
> I am mistaken, there is no way to disable statistics for interfaces for
> example.

Ethdev counters are updated by the NIC silicon.
They have zero software performance cost in the data plane.

> 
> For example, in grout, we use total_objs in the main loop to determine
> if we can save power by willingly putting the CPU to sleep for very
> short periods of time. If total_objs is no longer updated, grout cannot
> save power.

Makes sense. We do something similar in our application. Look at some instrumentation counters to estimate the workload and determine power saving actions.

If Grout is built with RTE_LIBRTE_GRAPH_STATS, it can use the graph stats; otherwise, it will have to look at something else.

We want to keep DPDK performant, so I'm opposed to making optional instrumentation mandatory.

> 
> total_cycles however, *is* instrumentation. It is only useful to
> identify hotspots and debug issues. But it can be interesting to enable
> it *at runtime* without recompiling.

The word "instrumentation" might not be 100 % clearly defined, and we seem to have slightly different interpretations of it.
But that is not important. My point is: These library counters are irrelevant for the data plane, so they should remain optional.
They are also irrelevant for the control plane.
They might be relevant for the management plane.