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

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

> From: Robin Jarry [mailto:rjarry@redhat.com]
> Sent: Saturday, 22 August 2026 11.53
> 
> Hey Morten,
> 
> I have some concerns with the "histogram" implementation. There are
> holes in the data. You will only capture specific batch sizes.
> 
> NB: did you notice we already have a burst size histogram exported in
> the grout metrics:
> 
> https://github.com/DPDK/grout/blob/v0.17.1/modules/infra/api/stats.c#L3
> 13-L336

Thanks for the pointer.
The existing histogram only covers packet burst size.
I want to capture performance data for development (optimization) purposes.

I'll take a look at it, to see if something similar could be relevant for the data I'm aiming to collect.

> 
> Morten Brørup, Jul 03, 2026 at 17:43:

[...]

> >  	if (rte_graph_has_stats_feature()) {
> > -		start = rte_rdtsc();
> > +		cycles = -rte_rdtsc();
> 
> I presume this works but it feels confusing taking a "negative" value
> of an unsigned integer.

Maybe this is more readable:
cycles = rte_rdtsc();
rc = node->process(graph, node, objs, node->idx);
cycles = rte_rdtsc() - cycles;

> 
> >  		rc = node->process(graph, node, objs, node->idx);
> > -		node->total_cycles += rte_rdtsc() - start;
> > +		cycles += rte_rdtsc();
> > +		node->total_cycles += cycles;
> >  		node->total_calls++;
> >  		node->total_objs += rc;
> > +#ifdef RTE_GRAPH_PROFILE
> > +		if (rc <= 1) {
> > +			node->usage_stats[rc].calls++;
> > +			node->usage_stats[rc].cycles += cycles;
> > +		} else if (rc == RTE_GRAPH_PROFILE_BURST_SIZE) {
> > +			node->usage_stats[2].calls++;
> > +			node->usage_stats[2].cycles += cycles;
> > +		} else if (rc == RTE_GRAPH_BURST_SIZE) {
> > +			node->usage_stats[3].calls++;
> > +			node->usage_stats[3].cycles += cycles;
> 
> If you want a reliable histogram, you would need to change these tests
> to the following:
> 
> 		id (rc >= RTE_GRAPH_BURST_SIZE) {
> 			node->usage_stats[3].calls++;
> 			node->usage_stats[3].cycles += cycles;
> 		} else if (rc >= RTE_GRAPH_PROFILE_BURST_SIZE) {
> 			node->usage_stats[2].calls++;
> 			node->usage_stats[2].cycles += cycles;
> 		} else if (rc != 0) {
> 			node->usage_stats[1].calls++;
> 			node->usage_stats[1].cycles += cycles;
> 		} else {
> 			node->usage_stats[0].calls++;
> 			node->usage_stats[0].cycles += cycles;
> 		}
> 
> Otherwise, you will miss lots of odd-sized batches in your histogram
> data.

Correct.
However, I only need a few representative snapshots to help identify what to optimize.
Zero and one object processed are quite frequently, so I want to know how frequent they occur.
BTW, the cycles/object for processing one object is substantially higher than when processing a burst.

For bursts, I want to be able to calculate the cycles/object.
I also want to be able to see how frequent they occur.

I wanted the data to stay within one cache line, so I compromised.
Since I don't need to stay within one cache line, I can improve it with better coverage.

> 
> 
> > +		}
> > +#endif
> >  	} else {
> >  		node->process(graph, node, objs, node->idx);
> >  	}
> 
> 
> --
> Robin
> 
> # Not a flying toy.