First parameter of galDivide is always 1

[cristaloleg]

Hi, I was scrolling through the code and found that galDivide 1st param is always 1 (4 usages in code and 1 in tests).

https://github.com/klauspost/reedsolomon/blob/3db84f17c9add1f09b1a2bb397b1c624230fea24/galois.go#L873

This func doesn't look like a hot function, but maybe propagating a const might give a small benefit (smaller binary, caching, etc.). WDYT?

Thanks.

[klauspost]

Good observation. It simplifies down to something like this:

// galOneOver is the same as galDivide(1, a).
func galOneOver(a byte) byte {
    if a == 0 {
        panic("Argument 'divisor' is 0")
    }
    logResult := logTable[a]
    if logResult != 0 {
        logResult ^= 255
    }
    return expTable[logResult]
}

If it was super-hot we could eliminate the if with another table, but it doesn't seem worth it.

[cristaloleg]

but it doesn't seem worth it.

Feel free to close the issue, thanks.

[klauspost]

@cristaloleg I am sending in a PR.. I was mainly saying "adding a separate table isn't worth it".

[klauspost]

https://github.com/klauspost/reedsolomon/pull/254

[cristaloleg]

I was mainly saying "adding a separate table isn't worth it".

Ah, my bad.

[klauspost]

@cristaloleg Actually expTable[255] == expTable[0], so we don't need the "if" since 0^255 == 255

[cristaloleg]

Nice! By the way, logResult := logTable[a] ^ 255 looks like a new table to me.

logTable values are in [0, 255) range, so we can store the result of logTable[a] ^ 255 instead, wdyt? (if I'm not missing something)

[klauspost]

The XOR is so fast that I don't really see any point in adding 256 bytes to binaries just for that.

[klauspost]

254 Merged