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.

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

proposal: encoding: provide append-like variants #53693

Open
dsnet opened this issue Jul 5, 2022 · 1 comment
Open

proposal: encoding: provide append-like variants #53693

dsnet opened this issue Jul 5, 2022 · 1 comment

Comments

@dsnet
Copy link
Member

dsnet commented Jul 5, 2022

There's been a shift in recent years to provide Append-like variants of APIs:

as these are generally easier to work with than Put-like APIs where you need to know the size of the buffer beforehand, prepare a buffer of the right size, and then call the Put-like operation.

Such APIs are missing for the encoding/hex, encoding/base32, and encoding/base64 packages. I propose we add the following:

package hex
func AppendEncoded(dst, src []byte) []byte
func AppendDecoded(dst, src []byte) ([]byte, error)
package base32
func (*Encoding) AppendEncoded(dst, src []byte) []byte
func (*Encoding) AppendDecoded(dst, src []byte) ([]byte, error)
package base64
func (*Encoding) AppendEncoded(dst, src []byte) []byte
func (*Encoding) AppendDecoded(dst, src []byte) ([]byte, error)

This would simplify many callers of these packages, where the only reason to call the Len method is to obtain the size of the expected output buffer, do some manual memory management, and then call the Encode or Decode equivalent.


For example, this logic in the json package:

e.WriteByte('"')
encodedLen := base64.StdEncoding.EncodedLen(len(s))
if encodedLen <= len(e.scratch) {
// If the encoded bytes fit in e.scratch, avoid an extra
// allocation and use the cheaper Encoding.Encode.
dst := e.scratch[:encodedLen]
base64.StdEncoding.Encode(dst, s)
e.Write(dst)
} else if encodedLen <= 1024 {
// The encoded bytes are short enough to allocate for, and
// Encoding.Encode is still cheaper.
dst := make([]byte, encodedLen)
base64.StdEncoding.Encode(dst, s)
e.Write(dst)
} else {
// The encoded bytes are too long to cheaply allocate, and
// Encoding.Encode is no longer noticeably cheaper.
enc := base64.NewEncoder(base64.StdEncoding, e)
enc.Write(s)
enc.Close()
}
e.WriteByte('"')

could be simplified as:

b := e.AvailableBuffer()
b = append(b, '"')
b = base64.StdEncoding.AppendEncoded(b, s)
b = append(b, '"')
e.Write(b)

This assumes we had some way to unify bytes.Buffer with append-like APIs; see #53685 (comment).

The resulting code is both simpler and more performant since we can append directly into the underlying bytes.Buffer without going through an intermediate encodeState.scratch buffer. Also, the caller doesn't have to do manual memory management checking whether the encoded output fits within the encodeState.scratch.

@dsnet dsnet added the Proposal label Jul 5, 2022
@gopherbot gopherbot added this to the Proposal milestone Jul 5, 2022
@mvdan
Copy link
Member

mvdan commented Jul 5, 2022

As far as names go, we could also consider EncodeAppend and DecodeAppend, even if they aren't as consistent with the usual "verb then object" method names like MarshalJSON or AppendUint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

3 participants