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

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Errors

### 1. Duplicate struct member in `usage_stats`

**File:** `lib/graph/rte_graph_worker_common.h` (line 139)

```c
struct __rte_cache_aligned {
	uint64_t calls;     /**< Calls done. */
	uint64_t cycles;    /**< Cycles spent. */
	uint64_t cycles;    /**< Objects processed. */  // ERROR: duplicate member name
} usage_stats[4];
```

**Issue:** The struct has `cycles` declared twice. The second should be `objs` based on the usage in the code.

**Fix:**
```c
struct __rte_cache_aligned {
	uint64_t calls;     /**< Calls done. */
	uint64_t cycles;    /**< Cycles spent. */
	uint64_t objs;      /**< Objects processed. */
} usage_stats[4];
```

---

### 2. Division by zero in percentage calculation

**File:** `lib/graph/graph_debug.c` (line 108)

```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,  // should be *100.0
	n->total_calls == 0 ? 0.0 :
	(double)n->total_cycles / (double)n->total_calls);
```

**Issue:** Percentage calculation divides without multiplying by 100, resulting in values like 0.15 instead of 15.0%.

**Fix:**
```c
all_total_cycles == 0 ? 0.0 :
	(double)n->total_cycles / (double)all_total_cycles * 100.0,
```

Apply the same fix to line 149 where the same pattern occurs for per-interval percentages.

---

### 3. Missing bounds check for array index

**File:** `lib/graph/graph_debug.c` (line 117)

```c
for (int idx = 0; idx < RTE_DIM(sample_sizes); idx++) {
	// ...
	} else {
		calls = n->usage_stats[idx - 2].calls;  // idx=2 accesses [0], but idx=5 accesses [3]
```

**Issue:** When `idx=5`, this accesses `usage_stats[3]`, which is valid. When `idx=0` or `idx=1`, the code takes the first branch. This appears correct, but the static_assert on line 119 should use `==` instead of `!=` for clarity since it's checking equality.

**Actually, this is correct** -- the static_assert on line 119 already validates the array dimensions match. The indexing is safe.

---

## Warnings

### 1. Missing initialization of `objs` field in `usage_stats_01`

**File:** `lib/graph/rte_graph_worker_common.h` (line 125)

The `usage_stats_01` struct only has `calls` and `cycles`, but the profiling code at line 219 increments `usage_stats[0].objs` for the interval `[2, SMALL[`. However, `usage_stats_01[0]` and `usage_stats_01[1]` do NOT have an `objs` field.

This is intentional per the code -- for 0 or 1 objects, the object count is fixed (0 or 1), so only `calls` and `cycles` are tracked. For 2+ objects, `objs` is accumulated.

**No action needed** -- design is consistent.

---

### 2. Format string consistency

**File:** `lib/graph/graph_debug.c` (line 145)

```c
fprintf(f, "         calls=%" PRIu64,
	calls);
if (calls != 0) {
	fprintf(f, " (%.1f%% of this node)\n",
		n->total_calls == 0 ? 0.0 :
		(double)calls / (double)n->total_calls);  // should be *100.0
```

Same percentage calculation issue as Error #2 above -- should multiply by 100.0.

---

### 3. Potential confusing output for zero-call case

**File:** `lib/graph/graph_debug.c` (lines 145-160)

When `calls == 0`, the code prints only `"calls=0\n"` with no other stats. This is fine but could be more explicit:

```c
if (calls == 0) {
	fprintf(f, "\n");
} else {
	// existing percentage/cycles output
}
```

This is a minor style point -- current behavior is acceptable.

---

### 4. Missing release notes update for config options

**File:** `doc/guides/rel_notes/release_26_11.rst`

The release notes mention the three new config options but do not explicitly state their default values or that `RTE_GRAPH_PROFILE` is disabled by default. Consider adding:

```rst
* **Added optional graph profiling statistics.**

  Added build-time configurable graph node profiling statistics via
  ``RTE_GRAPH_PROFILE`` in ``rte_config.h`` (disabled by default).
  When enabled, tracks cycles spent processing various burst size intervals
  per node. The interval edges default to 8, 32, and 256 objects, and are
  configurable via ``RTE_GRAPH_PROFILE_BURST_SMALL``,
  ``RTE_GRAPH_PROFILE_BURST_MEDIUM``, and ``RTE_GRAPH_PROFILE_BURST_LARGE``.
```

---

## Summary

**Critical fixes required:**
1. Fix duplicate `cycles` member in `usage_stats` struct (should be `objs`)
2. Fix percentage calculations -- multiply by 100.0

**Recommendations:**
- Clarify release notes with default values
- Verify percentage output formatting matches intent (15.0% vs 0.15)

The patch demonstrates good design: the profiling stats are optional, the cache alignment is explicit, and the histogram buckets are configurable. Once the duplicate member and percentage calculation are fixed, the patch should be correct.