Allow decompose to make use of the buffer

[Gilthoniel]

The comment of Decompose specifies that buf can be used to fill the coefficient if the size is big enough however the current implementation simply ignores it. This is problematic when attempting to encode without allocation. This patch attempts to provide the feature.

[Gilthoniel]

@petermattis maybe ?

[nvanbenschoten]

Hi @Gilthoniel, thanks for the contribution! Out of curiosity, is this an API that you are using or intend to use?

[nvanbenschoten]

cap(buf)-len(buf)

What does it mean for len(buf) != 0? Should we preserve the data in the slice and only use the unused capacity? The contract here, which comes from golang/go#30870, is ambiguous.

The usual convention for when a function like this takes a scratch space buffer is that it is permitted to overwrite any data in the buffer. So the function checks the capacity (if cap(buf) >= sizeInBytes), sets the length to the needed length if there's room (buf = buf[:sizeInBytes]), and then writes into the buffer from the beginning (coefficient = d.Coeff.FillBytes(buf)).

[Gilthoniel]

Yeah that makes sense, I will change to this approach instead.

[nvanbenschoten]

Since you made that change, I don't understand why the buf = append(buf, make([]byte, sizeInBytes-len(buf))...) logic is needed, instead of just buf = buf[:sizeInBytes].

[Gilthoniel]

Yes you're right that's not necessary. I actually learned today that it's allowed to increase the length if the capacity is big enough without append.

[Gilthoniel]

@nvanbenschoten thanks a lot for the review, I think I resolved your points. To answer your question, I'm already using the interface to marshal the decimal for transfers over the network. This is much more efficient than a string encoding and reduce the load on the garbage collector, even more with this PR.

[Gilthoniel]

@nvanbenschoten ping

[nvanbenschoten]

Looks great, thanks @Gilthoniel.