|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.

## Warnings

### 1. Missing `rte_memory_order_relaxed` on atomic accumulation

**lib/graph/rte_graph_worker_common.h:234-236**

The statistics counters (`total_cycles`, `total_calls`, `total_objs`) are accumulated without atomic operations. If this code path is reachable from multiple threads simultaneously (e.g., per-lcore graph instances sharing a node structure), this is a data race. If the node structure is guaranteed to be per-lcore (not shared), this is acceptable.

**Suggested approach:** If nodes can be shared between threads, use:
```c
rte_atomic_fetch_add_explicit(&node->total_cycles, cycles, rte_memory_order_relaxed);
rte_atomic_fetch_add_explicit(&node->total_calls, 1, rte_memory_order_relaxed);
rte_atomic_fetch_add_explicit(&node->total_objs, rc, rte_memory_order_relaxed);
```

If nodes are per-lcore and never shared, this is fine as-is.

**Note:** The same applies to the new `usage_stats` fields added in the `#ifdef RTE_GRAPH_PROFILE` block (lines 247-262). If nodes are not shared, no change is needed.

---

### 2. Integer division in percentage calculation could be clearer

**lib/graph/graph_debug.c:108-109**

The percentage calculation uses `(double)n->total_cycles / (double)all_total_cycles * 100.0`. While correct, casting both operands to `double` is redundant--only one needs to be cast to promote the division to floating-point.

**Suggested fix:**
```c
(double)n->total_cycles / all_total_cycles * 100.0
```

This is a minor style preference, not an error.

---

### 3. Release note formatting: build-time vs. build time

**doc/guides/rel_notes/release_26_11.rst:60-61**

The term "build-time configurable" uses a hyphenated adjective. Ensure consistency with DPDK documentation style. If the style guide prefers "build time" without a hyphen when used as a noun phrase, adjust accordingly.

**Suggested rewording (if needed):**
```rst
Added build time configurable graph node profiling statistics via
``RTE_GRAPH_PROFILE`` in ``rte_config.h`` (disabled by default).
```

This is a minor documentation style point, not a functional issue.

---

## Info

### 1. Potential for zero-division is already handled

**lib/graph/graph_debug.c:108-110, 150-157**

The code correctly checks for zero denominators before division:
- `all_total_cycles == 0 ? 0.0 : ...`
- `n->total_calls == 0 ? 0.0 : ...`
- `calls == 0 ? 0.0 : ...`

This is correct and prevents divide-by-zero errors.

---

### 2. Loop variable scope is appropriate

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

The loop variable `idx` is declared `unsigned int`, which is appropriate for indexing into arrays. Using `size_t` would also be acceptable but is not required here.

---

### 3. Static assertion placement is correct

**lib/graph/rte_graph_worker_common.h:238-244**

The `static_assert` statements validating the burst size configuration constants are placed inside the `#ifdef RTE_GRAPH_PROFILE` block, which is appropriate since the constants are only meaningful when profiling is enabled.

---

### 4. Alignment annotation is correctly applied

**lib/graph/rte_graph_worker_common.h:126**

The `alignas(RTE_CACHE_LINE_MIN_SIZE)` directive is correctly applied to the `usage_stats_01` array to ensure the profiling statistics start on a cache line boundary. This prevents false sharing between the fast-path data and the statistics.

---

## Summary

The patch correctly implements optional graph profiling statistics with appropriate build-time configuration. The primary consideration is whether the `rte_node` structure is per-lcore (not shared between threads) or can be accessed concurrently. If nodes are per-lcore, the non-atomic statistics updates are acceptable. If nodes can be shared, atomic operations should be used for the counter updates.

The code is otherwise well-structured, includes proper documentation, and follows DPDK coding conventions.