|WARNING| [v13] graph: add optional profiling stats

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/168244

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-23

# DPDK Patch Review

## Errors

None identified.

## Warnings

### 1. Documentation: RST definition list preferred

**File:** `doc/guides/prog_guide/graph_lib.rst`

The documentation adds three configuration options as a bullet list. This would be better structured as a definition list for clarity.

**Current:**
```rst
- Enable the ``RTE_GRAPH_PROFILE`` config option for more profiling details.
  Set the ``RTE_GRAPH_PROFILE_BURST_SMALL``, ``RTE_GRAPH_PROFILE_BURST_MEDIUM``,
  and ``RTE_GRAPH_PROFILE_BURST_LARGE`` config options to sample specific
  burst size intervals.
```

**Suggested:**
```rst
``RTE_GRAPH_PROFILE``
   Enable for detailed profiling statistics (disabled by default).

``RTE_GRAPH_PROFILE_BURST_SMALL``, ``RTE_GRAPH_PROFILE_BURST_MEDIUM``, ``RTE_GRAPH_PROFILE_BURST_LARGE``
   Configure burst size interval boundaries for profiling samples.
```

### 2. Unsigned loop counter preferred

**File:** `lib/graph/graph_debug.c:119`

The loop uses `unsigned int idx` which is acceptable, but the more common DPDK pattern for array iteration is to use `uint32_t` or match the type returned by `RTE_DIM()` (which is `size_t`).

**Current:**
```c
for (unsigned int idx = 0; idx < RTE_DIM(sample_sizes); idx++) {
```

**Suggested (minor preference):**
```c
for (uint32_t idx = 0; idx < RTE_DIM(sample_sizes); idx++) {
```

### 3. Formatting: Double-indent continuation lines

**File:** `lib/graph/graph_debug.c:105-109`

The multi-line `fprintf` uses single indent for continuations. DPDK style requires double-indent for continuation lines.

**Current:**
```c
fprintf(f, "       total_cycles=%" PRIu64 " (%.1f%% of all nodes), avg cycles/call=%.1f\n",
    n->total_cycles,
    all_total_cycles == 0 ? 0.0 :
    (double)n->total_cycles / (double)all_total_cycles * 100.0,
    n->total_calls == 0 ? 0.0 :
    (double)n->total_cycles / (double)n->total_calls);
```

**Suggested:**
```c
fprintf(f, "       total_cycles=%" PRIu64 " (%.1f%% of all nodes), avg cycles/call=%.1f\n",
		n->total_cycles,
		all_total_cycles == 0 ? 0.0 :
		(double)n->total_cycles / (double)all_total_cycles * 100.0,
		n->total_calls == 0 ? 0.0 :
		(double)n->total_cycles / (double)n->total_calls);
```

This also applies to the other multi-line `fprintf` calls at lines 136, 150, 154.

## Info

### 1. Cache line alignment rationale

The `alignas(RTE_CACHE_LINE_MIN_SIZE)` on the `usage_stats_01` array (line 126) is well-placed. This ensures the profiling data starts on a fresh cache line, avoiding false sharing with the frequently-accessed node fields above it. The v13 changelog indicates this was a fix from v12, which is correct.

### 2. Static assert placement

The static asserts verifying the configuration constants (lines 238-243) are appropriately placed in the hot path function where the values are used, ensuring they're checked even if profiling code is compiled out in some translation units.

### 3. Burst size interval design

The interval-based approach (small/medium/large) is a good design that avoids the overhead of per-object-count buckets while still providing actionable profiling data. The use of build-time configuration for the interval boundaries allows tuning for different workloads.

---

## Summary

The patch is technically sound with no correctness bugs identified. The main items are minor style preferences (definition list in RST documentation, double-indent for continuation lines) that would improve consistency with DPDK conventions. The v13 changes correctly address the cache alignment issue noted in the changelog.