[zhaoyboo]

Summary

The ring's device-to-device producers chose their grid from four fixed size
tiers (1, 4, 16 or 64 blocks at 64 KB, 4 MB and 32 MB), so mid-size and large
transfers ran on far fewer blocks than the copy needed. Grids now scale with
the bytes moved: one block per 16 KiB, capped at 264 blocks.

Implementation details

• producer.cuh: producer_grid_blocks(nbytes) replaces the tier constants
  (PRODUCER_BYTES_PER_BLOCK = 16 KiB, PRODUCER_MAX_BLOCKS = 264). The
  ceiling division is overflow-free ((n - 1) / B + 1), so UINT64_MAX
  yields the cap.
• producer.cu: every static, prefix, chunked and record producer launch uses
  it (pick_tier_blocks removed). Kernels already grid-stride and publish
  through the last block, so correctness does not depend on the grid size.
• tests/native/ring/bench_producer_grid.cu (make bench_producer_grid):
  old vs new geometry on an RTX 4090: 64 KiB 1.35x, 256 KiB 1.18x, 1.2 MiB
  1.58x (82 -> 129 GB/s), 4 MiB 5.17x (60 -> 312 GB/s), 16 MiB 3.34x
  (209 -> 696 GB/s).

Compatibility and scope

No API or ABI change; launch geometry only.

Validation

• test_producer: 276 assertions (was 154), including the grid formula at
  the boundaries and UINT64_MAX, wide-grid static/wrap copies, prefix rows,
  chunked partitions and mixed launches. All native ring suites pass.
• CPU tests: 2244 passed, 458 skipped (unchanged from main).

[Samfisheryu]

Independent audit — non-GPU stage

Reviewed head a20b6daa3ba270e6c6ef445e9ec16c5af621c1ef against base e43b4046e79063f4f7430589f7206ec7a3903016. This comment records the review boundary for this PR; the dependent PRs receive their own findings and reproduction evidence.

No confirmed new correctness defect found in this PR's geometry-only implementation. This is not a GPU correctness or performance approval. GPU tests and benchmarks have deliberately been deferred until the repository owner explicitly permits them; no GPU workloads were run for this audit.

Code checked:

• Overflow-free grid calculation: the (nbytes - 1) / B + 1 expression avoids overflowing the ceiling division near UINT64_MAX; zero and small sizes still select one block.
• Legacy and record launch sites: static, prefix and chunked producers now use the helper; the existing grid-stride copy and last-arriving-block publication do not require one particular grid size.
• The new wide-grid tests and old/new geometry benchmark were read, not executed on a GPU.

Clean-slate/performance qualification: this is a shared-path geometry change, including record producers, even when none of the later firewall/toggle/gather features is enabled. It therefore needs a baseline-versus-this-commit comparison with those features off. The fixed 264-block cap should not be treated as proof of a single resident wave on every supported GPU; chunked rounding can also exceed 264 (for example, rounding 264 up to a multiple of 64 gives 320). This is a performance/description observation, not a correctness or deadlock finding.

Deferred verification: boundary transfer sizes around 16 KiB, 64 KiB and the old tiers; prefix capture with a large upper bound but 0/1/few actual rows; chunk counts 1/3/64; ring wrap and mixed launch sizes; legacy and record paths; and whole-pipeline latency/throughput with the new features off. A D2D microbenchmark improvement alone does not establish an end-to-end serving improvement.

No defect reproduction is attached to this PR because none was established; I am not assigning a downstream PR's defect to this commit.

[zhaoyboo]

@Samfisheryu thanks for the audit. No defect was assigned here; I took both observations. The branch is now at 38da7fb (one commit on main). The stacked PRs were rebased on top of it.

Chunked rounding past the cap (fixed). producer_chunked_grid_blocks(nbytes, K) rounds the proportional grid up to a multiple of K only while the result stays within PRODUCER_MAX_BLOCKS. Otherwise it uses the largest multiple of K not above the cap. For 64 chunks on a capped grid that gives 256 blocks, not 320. A chunked grid can exceed 264 only when K itself does, and the launcher already limits K to PRODUCER_MAX_K = 64.

The 264 cap (description). producer.cuh and the commit message now describe 264 as a bounded heuristic, about two resident 256-thread blocks per SM on 128/132-SM parts. It is not a single-wave guarantee. Correctness does not depend on residency.

Tests added (CPU-runnable): tests/test_producer_grid_geometry.py compiles tests/native/ring/test_producer_grid_geometry.cpp host-only (no CUDA) and checks:

• the 16 KiB boundaries, the old tier sizes, cap * 16 KiB ± 1 and UINT64_MAX;
• chunk counts 1, 3, 5, 7, 64, cap and cap + 1;
• for every K from 1 to 64, that the chunked grid is a multiple of K and stays within [K, cap].

The native test_producer additions are GPU tests and have not been rerun for this revision: wide-grid static copies with and without wrap, prefix copies selecting 0..512 rows of a capped grid, the chunked block-to-chunk partition past the cap (legacy and record), and mixed grid sizes sharing the arrival counter.

Verification for this revision, CPU only: the native extension builds, and the -m cpu suite passes (2243 passed, 459 skipped). The one failure, test_cpu_native_build.py::test_every_torch_including_compile_requests_cxx20[host], also fails on main (e43b404). No GPU tests or benchmarks were run. Two things wait for the owner's go-ahead: the boundary/prefix/wrap matrix on a GPU, and the end-to-end baseline-vs-this-commit comparison with toggle/gather/handoff off. I agree a D2D microbenchmark alone does not establish a serving improvement.

[Samfisheryu]

Independent re-review of 38da7fbe0a2cfb14a629c4a25079f86267555640 (non-GPU stage).

The two previous observations are addressed:

• The chunked helper now keeps the grid a multiple of K within the 264-block cap for supported chunk counts. The formerly rounded 320-block example now returns 256 for K=64.
• The cap is correctly described as a heuristic, not a single-wave/residency guarantee.

I reviewed the revision diff and independently ran tests/test_producer_grid_geometry.py: 1 passed. That compiles and executes the shared host arithmetic, including size boundaries, UINT64_MAX and the chunk-count checks; it does not execute a CUDA producer.

No new correctness blocker found in this PR's non-GPU review. The global geometry change still affects normal legacy/record paths when all later features are off, so kernel correctness, prefix/wrap behavior and whole-pipeline performance remain pending. No GPU tests or benchmarks were run; they still require the owner's explicit go-ahead. This is not a GPU or performance sign-off.

[Samfisheryu]

Latest-state review: 38da7fbe0a2cfb14a629c4a25079f86267555640.

This head is unchanged since the previous review. Its CPU geometry checks remain applicable, and the current workflow is fully successful, including CPU/package checks, native compilation and the live catalog checks.

No confirmed issue remains from this PR's non-GPU review. The cap/rounding descriptions and implementation changes previously discussed are addressed. Kernel execution and end-to-end performance with the later features disabled remain for the final GPU phase; no GPU testing or benchmark was performed in this review.

[Samfisheryu]

Approved at 38da7fbe0a2cfb14a629c4a25079f86267555640 following the completed independent code and CPU/host review. No confirmed non-GPU issue remains in this PR. The reviewed commit's CI is successful.

Review record: #170 (comment)

GPU execution and end-to-end performance benchmarks remain a separate follow-up, pending the owner's explicit instruction as agreed. This approval records the completed code/CPU review and does not claim those measurements.