RE: [PATCH v5] graph: add optional profiling stats

Jerin, Saeed,

> From: Jerin Jacob [mailto:[email protected]]
> Sent: Tuesday, 23 June 2026 11.09
> 
> 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.

Please review [v9], and I will submit a rebased version for DPDK 26.11.

[v9]: https://patchwork.dpdk.org/project/dpdk/patch/[email protected]/

> 
> 
> >
> > So sometimes #ifdef is required in the code too.
> >
> > >
> > >
> > >
> > > >
> > > > >
> > > > > >                 alignas(RTE_CACHE_LINE_MIN_SIZE) struct
> rte_node
> > > > > *nodes[]; /**< Next nodes. */
> > > > > >         };
> > > > > >  };
> > > > > >