Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Try further optimize ReconstructSome for leopard 8+16 #274

Closed
wants to merge 4 commits into from
Closed

Try further optimize ReconstructSome for leopard 8+16 #274

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b4e3cb
Optimize ReconstructSome for leopard 8+16
elias-orijtech Feb 24, 2024
08a1be0
Try optimization: only add required shards errorBits/errLocs
liamsi Mar 4, 2024
2d78890
Fix tests
liamsi Mar 4, 2024
ad60709
should be investigated properly instead: make errorLocs dependent on …
liamsi Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions leopard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,17 +334,17 @@ func (r *leopardFF16) Split(data []byte) ([][]byte, error) {

func (r *leopardFF16) ReconstructSome(shards [][]byte, required []bool) error {
if len(required) == r.totalShards {
return r.reconstruct(shards, true)
return r.reconstruct(shards, true, required)
}
return r.reconstruct(shards, false)
return r.reconstruct(shards, false, required)
}

func (r *leopardFF16) Reconstruct(shards [][]byte) error {
return r.reconstruct(shards, true)
return r.reconstruct(shards, true, nil)
}

func (r *leopardFF16) ReconstructData(shards [][]byte) error {
return r.reconstruct(shards, false)
return r.reconstruct(shards, false, nil)
}

func (r *leopardFF16) Verify(shards [][]byte) (bool, error) {
Expand Down Expand Up @@ -375,8 +375,8 @@ func (r *leopardFF16) Verify(shards [][]byte) (bool, error) {
return true, nil
}

func (r *leopardFF16) reconstruct(shards [][]byte, recoverAll bool) error {
if len(shards) != r.totalShards {
func (r *leopardFF16) reconstruct(shards [][]byte, recoverAll bool, required []bool) error {
if len(shards) != r.totalShards || required != nil && len(required) < r.dataShards {
return ErrTooFewShards
}

Expand All @@ -388,15 +388,19 @@ func (r *leopardFF16) reconstruct(shards [][]byte, recoverAll bool) error {
// nothing to do.
numberPresent := 0
dataPresent := 0
missingRequired := 0
for i := 0; i < r.totalShards; i++ {
if len(shards[i]) != 0 {
numberPresent++
if i < r.dataShards {
dataPresent++
}
} else if required != nil && required[i] {
missingRequired++
}
}
if numberPresent == r.totalShards || !recoverAll && dataPresent == r.dataShards {
if numberPresent == r.totalShards || !recoverAll && dataPresent == r.dataShards ||
required != nil && missingRequired == 0 {
// Cool. All of the shards have data. We don't
// need to do anything.
return nil
Expand Down Expand Up @@ -426,19 +430,19 @@ func (r *leopardFF16) reconstruct(shards [][]byte, recoverAll bool) error {
for i := 0; i < r.parityShards; i++ {
if len(shards[i+r.dataShards]) == 0 {
errLocs[i] = 1
if LEO_ERROR_BITFIELD_OPT && recoverAll {
if LEO_ERROR_BITFIELD_OPT && (recoverAll || (required != nil && required[i])) {
errorBits.set(i)
}
}
}
for i := r.parityShards; i < m; i++ {
errLocs[i] = 1
if LEO_ERROR_BITFIELD_OPT && recoverAll {
if LEO_ERROR_BITFIELD_OPT && (recoverAll || (required != nil && required[i])) {
errorBits.set(i)
}
}
for i := 0; i < r.dataShards; i++ {
if len(shards[i]) == 0 {
if len(shards[i]) == 0 /*&& (recoverAll || (required != nil && required[i]))*/ {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uncommenting this leads to more significant improvements in BenchmarkDecode1K but will lead to a bunch of test failures (I think mostly because ReconstructData expects to reconstruct all original missing data). This can probably be fixed.

errLocs[i+m] = 1
if LEO_ERROR_BITFIELD_OPT {
errorBits.set(i + m)
Expand Down Expand Up @@ -541,7 +545,7 @@ func (r *leopardFF16) reconstruct(shards [][]byte, recoverAll bool) error {
end = r.totalShards
}
for i := 0; i < end; i++ {
if len(shards[i]) != 0 {
if len(shards[i]) != 0 || required != nil && !required[i] {
continue
}
if cap(shards[i]) >= shardSize {
Expand Down
28 changes: 16 additions & 12 deletions leopard8.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,17 +370,17 @@ func (r *leopardFF8) Split(data []byte) ([][]byte, error) {

func (r *leopardFF8) ReconstructSome(shards [][]byte, required []bool) error {
if len(required) == r.totalShards {
return r.reconstruct(shards, true)
return r.reconstruct(shards, true, required)
}
return r.reconstruct(shards, false)
return r.reconstruct(shards, false, required)
}

func (r *leopardFF8) Reconstruct(shards [][]byte) error {
return r.reconstruct(shards, true)
return r.reconstruct(shards, true, nil)
}

func (r *leopardFF8) ReconstructData(shards [][]byte) error {
return r.reconstruct(shards, false)
return r.reconstruct(shards, false, nil)
}

func (r *leopardFF8) Verify(shards [][]byte) (bool, error) {
Expand Down Expand Up @@ -411,8 +411,8 @@ func (r *leopardFF8) Verify(shards [][]byte) (bool, error) {
return true, nil
}

func (r *leopardFF8) reconstruct(shards [][]byte, recoverAll bool) error {
if len(shards) != r.totalShards {
func (r *leopardFF8) reconstruct(shards [][]byte, recoverAll bool, required []bool) error {
if len(shards) != r.totalShards || required != nil && len(required) < r.dataShards {
return ErrTooFewShards
}

Expand All @@ -424,15 +424,19 @@ func (r *leopardFF8) reconstruct(shards [][]byte, recoverAll bool) error {
// nothing to do.
numberPresent := 0
dataPresent := 0
missingRequired := 0
for i := 0; i < r.totalShards; i++ {
if len(shards[i]) != 0 {
numberPresent++
if i < r.dataShards {
dataPresent++
}
} else if required != nil && required[i] {
missingRequired++
}
}
if numberPresent == r.totalShards || !recoverAll && dataPresent == r.dataShards {
if numberPresent == r.totalShards || !recoverAll && dataPresent == r.dataShards ||
required != nil && missingRequired == 0 {
// Cool. All of the shards have data. We don't
// need to do anything.
return nil
Expand Down Expand Up @@ -463,19 +467,19 @@ func (r *leopardFF8) reconstruct(shards [][]byte, recoverAll bool) error {
for i := 0; i < r.parityShards; i++ {
if len(shards[i+r.dataShards]) == 0 {
errLocs[i] = 1
if LEO_ERROR_BITFIELD_OPT && recoverAll {
if LEO_ERROR_BITFIELD_OPT && (recoverAll || (required != nil && required[i])) {
errorBits.set(i)
}
}
}
for i := r.parityShards; i < m; i++ {
errLocs[i] = 1
if LEO_ERROR_BITFIELD_OPT && recoverAll {
if LEO_ERROR_BITFIELD_OPT && (recoverAll || (required != nil && required[i])) {
errorBits.set(i)
}
}
for i := 0; i < r.dataShards; i++ {
if len(shards[i]) == 0 {
if len(shards[i]) == 0 /*&& (recoverAll || (required != nil && required[i]))*/ {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as in leopard.go:

https://github.com/klauspost/reedsolomon/pull/274/files#r1511442291

errLocs[i+m] = 1
if LEO_ERROR_BITFIELD_OPT {
errorBits.set(i + m)
Expand Down Expand Up @@ -566,7 +570,7 @@ func (r *leopardFF8) reconstruct(shards [][]byte, recoverAll bool) error {

// Add output
for i, sh := range shards {
if !recoverAll && i >= r.dataShards {
if !recoverAll && i >= r.dataShards || required != nil && !required[i] {
continue
}
if len(sh) == 0 {
Expand Down Expand Up @@ -657,7 +661,7 @@ func (r *leopardFF8) reconstruct(shards [][]byte, recoverAll bool) error {
}
// Restore
for i := 0; i < end; i++ {
if len(sh[i]) != 0 {
if len(sh[i]) != 0 || required != nil && !required[i] {
continue
}

Expand Down
7 changes: 3 additions & 4 deletions reedsolomon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,8 +877,7 @@ func testReconstructData(t *testing.T, o ...Option) {
}

if shardsCopy[2] != nil || shardsCopy[5] != nil || shardsCopy[6] != nil {
// This is expected in some cases.
t.Log("ReconstructSome reconstructed extra shards")
t.Error("ReconstructSome reconstructed extra shards")
}

// Reconstruct with 10 shards present. Use pre-allocated memory for one of them.
Expand Down Expand Up @@ -1490,12 +1489,12 @@ func BenchmarkReconstruct10x4x1M(b *testing.B) {
benchmarkReconstruct(b, 10, 4, 1024*1024)
}

// Benchmark 5 data slices with 2 parity slices holding 1MB bytes each
// Benchmark 50 data slices with 20 parity slices holding 1MB bytes each
func BenchmarkReconstruct50x20x1M(b *testing.B) {
benchmarkReconstruct(b, 50, 20, 1024*1024)
}

// Benchmark 5 data slices with 2 parity slices holding 1MB bytes each
// Benchmark 50 data slices with 20 parity slices holding 1MB bytes each
func BenchmarkReconstructLeopard50x20x1M(b *testing.B) {
benchmarkReconstruct(b, 50, 20, 1024*1024, WithLeopardGF(true), WithInversionCache(true))
}
Expand Down
Loading