On Tue, Jun 23, 2026 at 12:40 PM Morten Brørup <[email protected]> wrote: > > +Thomas Monjalon & +David Marchand, as intended by Jerin > > > From: Jerin Jacob [mailto:[email protected]] > > Sent: Tuesday, 23 June 2026 08.57 > > > > 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) > > I agree with these advantages. > But a function using rte_lib_has_some_feature() cannot access non-existing > fields: > https://godbolt.org/z/s3nKx45Ms
I missed that. You can add these new struct updates in the slowpath area of rte_node. Above offsetof(struct rte_node, ctx) Use RTE_NEXT_ABI, Get around off, ABI breakge issue. > > So sometimes #ifdef is required in the code too. > > > > > > > > > > > > > > > > > > > alignas(RTE_CACHE_LINE_MIN_SIZE) struct rte_node > > > > *nodes[]; /**< Next nodes. */ > > > > > }; > > > > > }; > > > > >
