implement in-place decode

[karaketir16]

I implement the in-place decode and add a test for it.

[maruel]

The documentation for Decode() already mentions that it happens in place. Why a new function?

[karaketir16]

Decode() uses copy operations, create new array, and copy backs to original array.

New function does not copy the array.

[maruel]

Why not just change the original function and keep the same name?

[karaketir16]

Original function takes two slices. Changed version takes one slice. I don't know changing that is okay.

If you think it is okay, I can just modify the original so that it takes one slice.

[karaketir16]

Also I will update the decoder_test.go

[karaketir16]

Its doing same job without copy operations, just knowing ecc count. We need one slice, because your poly struct has one slice. If we get two slices, we need to concat them to put inside the poly.

If we want to use two slices without copying them, we need to modify poly which I don't know how it works.

Here unmodified Decode

func (d *rSDecoder) Decode(data, ecc []byte) (int, error) {
	// TODO(maruel): Temporary migration code.
	received := make([]byte, len(data)+len(ecc))
	copy(received, data)
	copy(received[len(data):], ecc)
	poly := &poly{d.f, received}
	syndromeCoeffs := make([]byte, len(ecc))

Here modified Decode

func (d *rSDecoder) Decode(dataWithEcc []byte, eccLen int) (int, error) {
	// TODO(maruel): Temporary migration code.
	dataLen := len(dataWithEcc) - eccLen
	poly := &poly{d.f, dataWithEcc}
	syndromeCoeffs := make([]byte, eccLen)

In my case, I store and transmit data with ecc together, its better for me to call Decode(dataWithEcc, eccLen) instead of Decode(dataWithEcc[:dataLen], dataWithEcc[dataLen:])

[maruel]

Using two slices permits to store the ecc in both a discontinuous or continuous stream. The new version is much more limiting here. I'd highly prefer to fix poly instead.

Slices are not arrays. A slice is a view into an array, thus there's no copy in the two slices version. See https://gobyexample.com/slices

[karaketir16]

I don't mean passing slices are copy. I mean, you are calling copy function directly inside the Decode. And as I said we have to copy if we don't modify the poly.

I will look poly, maybe we can modify it so it also takes two slices.

[maruel]

Yes modifying poly would be my preference.

[karaketir16]

When I look the poly, it is using coefficients as a single slice. Converting it to multiple slices probably not a good idea.

type poly struct {
	field        *Field
	coefficients []byte // In reverse order.
}

So, I am offering the code as I opened PR, we can add DecodeInPlace to code, (name can be change). and we can use Decode like the same. it probably does not create much overhead(i can do benchmark if you want), and if we want we can use DecodeInPlace which does 4 less copy operations.

func (d *rSDecoder) Decode(data, ecc []byte) (int, error) {
	received := make([]byte, len(data)+len(ecc))
	copy(received, data)
	copy(received[len(data):], ecc)
	errorCount, decodeError := d.DecodeInPlace(received, len(ecc))
	copy(data, received)
	copy(ecc, received[len(data):])
	return errorCount, decodeError
}

[karaketir16]

Here Benchmark results, but I am not sure I benchmarked correctly and I don't know how to analyze this results.

Old Decode

goos: linux
goarch: amd64
pkg: github.com/maruel/rs
cpu: 12th Gen Intel(R) Core(TM) i5-12450H
BenchmarkDecode16_10-12               	 2445758	       473.5 ns/op	82648331.65 MB/s	      48 B/op	       2 allocs/op
BenchmarkDecode16_10With1Error-12     	 1256457	       949.2 ns/op	21180130.20 MB/s	     528 B/op	      28 allocs/op
BenchmarkDecode128_16-12              	  198171	      6036 ns/op	4202141.26 MB/s	     160 B/op	       2 allocs/op
BenchmarkDecode128_16With1Error-12    	  181016	      6586 ns/op	3518293.74 MB/s	     790 B/op	      28 allocs/op
BenchmarkDecode128_16With2Error-12    	  137966	      7320 ns/op	2412675.47 MB/s	    1176 B/op	      47 allocs/op
PASS
ok  	github.com/maruel/rs	7.444s

New Decode

goos: linux
goarch: amd64
pkg: github.com/maruel/rs
cpu: 12th Gen Intel(R) Core(TM) i5-12450H
BenchmarkDecode16_10-12               	 2521464	       477.9 ns/op	84420616.63 MB/s	      16 B/op	       1 allocs/op
BenchmarkDecode16_10With1Error-12     	 1000000	      1049 ns/op	15254901.10 MB/s	     512 B/op	      27 allocs/op
BenchmarkDecode128_16-12              	  184364	      6157 ns/op	3832831.58 MB/s	      16 B/op	       1 allocs/op
BenchmarkDecode128_16With1Error-12    	  178500	      6763 ns/op	3378207.00 MB/s	     662 B/op	      27 allocs/op
BenchmarkDecode128_16With2Error-12    	  138470	      7644 ns/op	2318786.01 MB/s	    1032 B/op	      45 allocs/op
PASS
ok  	github.com/maruel/rs	6.387s

[karaketir16]

I updated the benchmark with makecopy removed. I also updated the benchmark for the old decode function while running.

Old Decode

goos: linux
goarch: amd64
pkg: github.com/maruel/rs
cpu: 12th Gen Intel(R) Core(TM) i5-12450H
BenchmarkDecode16_10-12               	 2407120	       498.6 ns/op	77244958.62 MB/s	      48 B/op	       2 allocs/op
BenchmarkDecode16_10With1Error-12     	 1305115	       941.2 ns/op	22186455.84 MB/s	     512 B/op	      27 allocs/op
BenchmarkDecode128_16-12              	  196250	      6122 ns/op	4103061.96 MB/s	     160 B/op	       2 allocs/op
BenchmarkDecode128_16With1Error-12    	  181203	      6679 ns/op	3472787.80 MB/s	     662 B/op	      27 allocs/op
BenchmarkDecode128_16With2Error-12    	  164697	      7258 ns/op	2904691.98 MB/s	    1032 B/op	      45 allocs/op
PASS
ok  	github.com/maruel/rs	7.689s

New Decode

goos: linux
goarch: amd64
pkg: github.com/maruel/rs
cpu: 12th Gen Intel(R) Core(TM) i5-12450H
BenchmarkDecode16_10-12               	 2505919	       465.6 ns/op	86111209.87 MB/s	      16 B/op	       1 allocs/op
BenchmarkDecode16_10With1Error-12     	 1312581	       899.8 ns/op	23340185.02 MB/s	     480 B/op	      26 allocs/op
BenchmarkDecode128_16-12              	  202328	      6011 ns/op	4308775.31 MB/s	      16 B/op	       1 allocs/op
BenchmarkDecode128_16With1Error-12    	  179751	      6567 ns/op	3503619.46 MB/s	     518 B/op	      26 allocs/op
BenchmarkDecode128_16With2Error-12    	  153178	      7212 ns/op	2718774.60 MB/s	     888 B/op	      44 allocs/op
PASS
ok  	github.com/maruel/rs	7.481s

[karaketir16]

Closing this one, we can continue on #4.