Do all reconstruction in one pass

[klauspost]

Instead of doing data + parity recovery in separate passes do everything in one pass.

benchmark                                      old MB/s      new MB/s      speedup
BenchmarkReconstruct50x5x50000-32              74908.09      109926.03     1.47x
BenchmarkReconstruct10x2x1M-32                 165523.19     249533.53     1.51x
BenchmarkReconstruct5x2x1M-32                  141100.72     217592.22     1.54x
BenchmarkReconstruct10x4x1M-32                 144233.98     239901.83     1.66x
BenchmarkReconstruct50x20x1M-32                39208.33      52027.88      1.33x
BenchmarkReconstruct10x4x16M-32                40617.55      54814.64      1.35x

Will apply whenever parity is recovered. Pure data recovery remains the same.

Summary by CodeRabbit

• Refactor
  • Improved performance and efficiency of the recovery process through internal code optimization.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The reconstruct method in reedsolomon.go is refactored to consolidate multi-step per-shard reconstruction into a unified single codeSomeShards call. A new multiplyRowWithMatrix helper function computes row-by-matrix products for parity recovery. Missing parity shards are now reconstructed via matrix multiplication rather than separate reconstruction paths.

Changes

Cohort / File(s)	Summary
Reed-Solomon Reconstruction Logic reedsolomon.go	Consolidates per-shard reconstruction into unified codeSomeShards call; adds multiplyRowWithMatrix helper for parity row computation; refactors missing shard handling to build single outputs/matrixRows set for data and parity shards; eliminates per-shard branching in favor of streamlined control flow

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 With multiplied rows and matrix dance,
One unified call gives shards a chance,
Parity flows through vectors aligned,
Reconstruction's now streamlined and refined! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Do all reconstruction in one pass' accurately summarizes the main change: consolidating data and parity recovery from multiple passes into a single codeSomeShards call, as confirmed by the detailed changes in reedsolomon.go.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch single-pass-recover

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4916c9c and 3a303e7.

📒 Files selected for processing (1)

• reedsolomon.go

🧰 Additional context used

🧬 Code graph analysis (1)

reedsolomon.go (2)

unsafe_disabled.go (1)

• AllocAligned (17-27)

unsafe.go (1)

• AllocAligned (19-43)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: build (1.25.x, macos-latest)
• GitHub Check: build (1.23.x, ubuntu-latest)
• GitHub Check: build (1.25.x, ubuntu-latest)
• GitHub Check: build (1.24.x, windows-latest)
• GitHub Check: build (1.23.x, macos-latest)
• GitHub Check: build (1.24.x, ubuntu-latest)
• GitHub Check: build (1.24.x, macos-latest)
• GitHub Check: build (1.23.x, windows-latest)
• GitHub Check: build (1.25.x, windows-latest)
• GitHub Check: build-special

🔇 Additional comments (2)

reedsolomon.go (2)

599-612: LGTM! Correct GF(2^8) row-by-matrix multiplication.

The implementation correctly uses XOR for field addition and galMultiply for field multiplication in GF(2^8). The allocation per call is acceptable given this is only invoked for each missing parity shard during reconstruction (infrequent hot path).

---

1522-1563: LGTM! Single-pass reconstruction consolidation is correct.

The mathematical approach is sound:

• For missing data shards: dataDecodeMatrix[iShard] correctly maps from available shards to original data.
• For missing parity shards: multiplyRowWithMatrix(r.parity[parityIdx], dataDecodeMatrix) correctly composes the mapping from available shards → original data → parity.

The edge case where all data shards are present also works correctly since dataDecodeMatrix becomes the identity matrix, leaving parity rows unchanged.

Existing tests cover mixed data and parity shard loss scenarios (e.g., reedsolomon_test.go lines 715-718 and 906-909).

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}