ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.85k stars 295 forks source link

No way to zero-copy decode a byte slice #312

Closed cvermilion closed 5 years ago

cvermilion commented 5 years ago

The primer says that decoding strives for zero-copy, so I was surprised that in the simplest case, decoding a Msgpack byte array into a nil byte slice, a new slice is allocated rather than simply take the slice from the input. I can see the downside of doing this by default, since slice mutability means that if you copy the input bytes by reference, changes to either input or output will affect the other. But often, the input bytes will be discarded as soon as decoding is done, so there is no risk and avoiding the extra allocations is desirable. Perhaps a decoder option (UnsafeBytes, maybe) would be the best solution? I'm happy to put together a PR if you think this would be a useful and acceptable option.

Here's a small example program that illustrates what I mean:

package main

import (
        "fmt"

        "github.com/ugorji/go/codec"
)

func main() {
        MH := &codec.MsgpackHandle{}

        encoded := []byte{0xc4, 0x05, 0x68, 0x65, 0x6c, 0x6c, 0x6f} // "hello" as bytes
        var out []byte

        dec := codec.NewDecoderBytes(encoded, MH)
        _ = dec.Decode(&out)

        // Prints "hello"
        fmt.Println(string(out))

        encoded[3] = 0x75 // change input bytes to be "hullo"

        // Prints "hello" even though we changed the input slice
        fmt.Println(string(out))
}
ugorji commented 5 years ago

Initially, my first reaction was that

UnsafeBytes option is a non-starter. Most folks will stumble upon it and have a negative experience, for what is really an edge case (a non-general case i.e applicable to bytes within msgpack which are mutable). Also, I am naturally resistant to adding more options; the bar has to be higher, like needed performance or fixing a bug.

However, on thinking about it some more, I like the option. It seems like a naturally expected option, and you nicely articulated all the pros and cons of it, making me think less.

So - fix coming soon.

cvermilion commented 5 years ago

Thanks for the fast response, this is perfect!