On Tue, Jun 23, 2026 at 12:15 PM Morten Brørup <[email protected]> wrote: > > > From: Jerin Jacob [mailto:[email protected]] > > Sent: Tuesday, 23 June 2026 07.13 > > > > On Mon, Jun 22, 2026 at 12:11 AM Morten Brørup > > <[email protected]> wrote: > > > > > > Added graph node profiling stats, build time configurable by enabling > > > RTE_GRAPH_PROFILE in rte_config.h. > > > > > > Signed-off-by: Morten Brørup <[email protected]> > > > > Please update app/test/test_graph.c to validate this featue. > > Ack. > > > > @@ -92,7 +92,62 @@ rte_graph_obj_dump(FILE *f, struct rte_graph *g, > > bool all) > > > fprintf(f, " total_sched_fail=%" PRId64 > > "\n", > > > n->dispatch.total_sched_fail); > > > } > > > - fprintf(f, " total_calls=%" PRId64 "\n", n- > > >total_calls); > > > + fprintf(f, " total_calls=%" PRIu64 "\n", n- > > >total_calls); > > > + if (rte_graph_has_stats_feature()) { > > > + fprintf(f, " total_cycles=%" PRIu64 ", > > avg cycles/call=%.1f\n", > > > + n->total_cycles, > > > + n->total_calls == 0 ? > > (double)0 : > > > + (double)n->total_cycles / > > (double)n->total_calls); > > > + } > > > +#ifdef RTE_GRAPH_PROFILE > > > > > > Please introduce rte_graph_has_profile_featue() similar to > > rte_graph_has_stats_feature() to reduce if def clutter as possible. > > Disagree, see below. > > > > > > + uint64_t calls = n->usage_stats[0].calls; > > > + fprintf(f, " objs[0]\n"); > > > + fprintf(f, " calls=%" PRIu64 ", cycles=%" > > PRIu64 ", avg cycles/call=%.1f\n", > > > + calls, > > > > > > > > diff --git a/lib/graph/rte_graph_worker_common.h > > b/lib/graph/rte_graph_worker_common.h > > > index 4ab53a533e..0d8039575d 100644 > > > --- a/lib/graph/rte_graph_worker_common.h > > > +++ b/lib/graph/rte_graph_worker_common.h > > > @@ -144,12 +144,26 @@ struct __rte_cache_aligned rte_node { > > > rte_node_process_t process; /**< Process > > function. */ > > > uint64_t process_u64; > > > }; > > > + /** Fast path area cache line 3. */ > > > +#ifdef RTE_GRAPH_PROFILE > > > + struct { > > > + uint64_t calls; /**< Calls processing > > resp. 0 or 1 objects. */ > > > + uint64_t cycles; /**< Cycles spent > > processing resp. 0 or 1 objects. */ > > > + } usage_stats[2]; /**< Usage when this node > > processed 0 or 1 objects. */ > > > + uint64_t full_burst_calls; /**< Calls processing a > > full burst of objects. */ > > > + uint64_t full_burst_cycles; /**< Cycles spent > > processing a full burst of objects. */ > > > + uint64_t half_burst_calls; /**< Calls processing a > > half burst of objects. */ > > > + uint64_t half_burst_cycles; /**< Cycles spent > > processing a half burst of objects. */ > > > + /** Fast path area cache line 4. */ > > > +#endif > > > > Is it an ABI breakage? > > No. The modifications are enclosed in #ifdef, and disabled by default. > It is generally required that when rte_config.h options are modified, both > the application and DPDK itself are built together; and then API/ABI breakage > becomes irrelevant.
Yes. I don't know the current policy for this. Adding @Thomas Monjalon @David Marchand > > IMO, we should keep our structures lean in release builds. This means that > fields used for detailed profiling, advanced debugging, cookie validation, > etc. should use the #ifdef pattern rather than the rte_lib_has_some_feature() > pattern; especially if they affect the size of a structure. And when those > fields are not present, any code accessing them cannot use the > rte_lib_has_some_feature() pattern. > The mbuf and mempool libraries also use #ifdef pattern for similar features. Yes for the structure inclusion we can use #ifdef. But inside the function we can use rte_lib_has_some_feature() scheme. Reasons are : 1)It will remove the ifdef cultter 2)Detect the compilation issue even if the feature is disabled. This will make sure reduce the build options to enable build sanity 3) Compiler is smart enough to understand to disable the block if the feature is not enabled.(Just like #ifdef) > > > > > > alignas(RTE_CACHE_LINE_MIN_SIZE) struct rte_node > > *nodes[]; /**< Next nodes. */ > > > }; > > > }; > > >
