reflect: permit Value.Bytes (but not SetBytes) on addressable byte arrays

[bradfitz]

I'm trying to get a []byte of an array via reflect, without allocations.

reflect.Value.Bytes says:

Bytes returns v's underlying value. It panics if v's underlying value is not a slice of bytes.

I have an array, not a slice. I can slice it, but reflect.Value.Slice call allocates:

func TestArraySliceAllocs(t *testing.T) {
        type T struct {
                X [32]byte
        }
        x := &T{X: [32]byte{1: 1, 2: 2, 3: 3, 4: 4}}

        var b []byte
        n := int(testing.AllocsPerRun(2000, func() {
                v := reflect.ValueOf(x)
                b = v.Elem().Field(0).Slice(0, 5).Bytes()
        }))
        if n != 0 {
                t.Errorf("allocs = %d; want 0", n)
        }
        const want = "\x00\x01\x02\x03\x04"
        if string(b) != want {
                t.Errorf("got %q; want %q", b, want)
        }
}

(fails with 1 alloc, from Slice)

Perhaps reflect.Value.Bytes could also permit getting a []byte of an array of bytes?

Or maybe I'm holding reflect wrong and there's an alloc-free way to do this already.

/cc @josharian

[randall77]

If you have a pointer to an array of bytes in a reflect.Value, you could do

p := v.Interface().(*[8]byte)[:]

(assuming you know 8)

In your code, that's b = v.Elem().Field(0).Addr().Interface().(*[32]byte)[:]

[bradfitz]

I don't know the length, though. I want to do this in a package that can run over anybody's types at runtime.

[randall77]

I see. I think it would be fine to allow Bytes to work on an addressable array (or pointer to array?).

[josharian]

Is it possible to make Slice not allocate when the array is addressable (or when it is a pointer to an array)?

[DeedleFake]

Maybe I'm more tired than I think and I'm completely misreading something here, but doesn't that test code make sure that Slice() doesn't allocate? It marks it as an error if testing.AllocsPerRun() returns anything other than zero. The error message even explicitly says allocs = %d; want 0.

Edit: I'm definitely more tired than I think. Somehow completely missed the

(fails with 1 alloc, from Slice)

underneath.

I read through both reflect.Value.Slice() and reflect.Value.Bytes(). I think special-casing Bytes() to allow calling it with an array makes sense. I'd prefer being able to get rid of the allocation in Slice(), but I'm not sure that that's particularly feasible without changing the structure of reflect.Value. Am I correct that the allocation happens because reflect.Value can only hold at most a word of data, so it stores a pointer to the newly created slice header instead, which causes it to escape to the heap on line 1810? -gcflags='-m' doesn't seem to do anything about stdlib packages, even if I combine it with -a.

It should also probably be noted that this is pretty easy to workaround, especially in Go 1.17:

package main

import (
    "fmt"
    "reflect"
    "testing"
    "unsafe"
)

func main() {
    v := reflect.ValueOf(&[...]byte{3, 5, 7}).Elem()
    var s []byte
    n := testing.AllocsPerRun(1024, func() {
        s = unsafe.Slice((*byte)(unsafe.Pointer(v.Addr().Pointer())), v.Len())
    })
    fmt.Println(n)
}

[dsnet]

I like @randall77's suggestion of allow Bytes on an addressable array or non-nil pointer to array.

In some code, I have:

if t.Kind() == reflect.Array {
    b = va.Slice(0, t.Len()).Bytes()
} else {
    b = va.Bytes()
}

I would like to reduce it to simply:

b = va.Bytes()

and avoid the allocation that occurs in Slice.

[dsnet]

Analyzing the module proxy, I found:

• 655 cases of x.Slice(...).Bytes()
• 648 cases of x.Slice(0, ...).Bytes() (98.9% of total)
• 276 cases of x.Slice(0, x.Len()).Bytes() (42.1% of total)

Of the 372 cases of x.Slice(0, ...).Bytes() that aren't x.Slice(0, x.Len()).Bytes(), many were like x.Slice(0, n).Bytes(), where n was the length of the array, but obtained elsewhere to avoid repeatedly calling reflect.Value.Len.

[rsc]

Making v.Bytes() work when v is an addressable byte array seems OK. Or can we optimize away the slice conversion in v.Slice().Bytes()?

[rsc]

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group

[randall77]

Optimizing the allocation out of v.Slice() sounds hard. The tricky part is to keep track of the args to Slice somewhere. We could conceivably introduce another flag to reflect.Value.flag which means "this is a slice of a ptr to an array with slice bounds 0 and len(v.Type.Elem())", and set that flag when we're slicing a pointer-to-array with the right Slice args. Then everywhere that accepts a slice type needs a special case to recognize this flag.

All to avoid just letting Bytes work on addressable arrays, that seems overkill.

[dsnet]

We could conceivably introduce another flag to reflect.Value.flag which means "this is a slice of a ptr to an array with slice bounds 0 and len(v.Type.Elem())"

This would unfortunately still not help the common patterns where reflect.Value.Slice and reflect.Append (#48000) always allocate.

[dsnet]

Making v.Bytes() work when v is an addressable byte array seems OK.

In addition, we should probably update Value.SetBytes should stay consistent with Value.Bytes.

[rsc]

SetBytes seems more difficult. What happens if you v.SetBytes(x) and v.Len() and len(x) are different sizes?

[gopherbot]

Change https://golang.org/cl/357331 mentions this issue: reflect: allow Value.Bytes and Value.SetBytes on byte arrays

[dsnet]

I mailed out https://golang.org/cl/357331 as a prototype.

What happens if you v.SetBytes(x) and v.Len() and len(x) are different sizes?

I would expect it to panic. The Bytes method already makes implicit assumptions about the length of the []byte that it returns for an array. It seems reasonable for SetBytes to have expectations on the length it accepts.

[randall77]

That SetBytes behavior is subtly different though. SetBytes on a []byte just replaces pointers, it does no copying. Whereas SetBytes on an array does a copy of the underlying data.

Unlike Bytes, where the returned []byte is always aliased to the storage for the input slice or array.

Would it make more sense for SetBytes on an addressable *[N]byte to just update the pointer to point to the first N bytes of the input byte slice?

That means you couldn't SetBytes on an addressable [N]byte. You would use Copy for that (which is the behavior your prototype does, I think).

[rsc]

That is in fact subtle, @randall77. It makes me think we should leave SetBytes as is, maybe? It looks like today SetBytes requires a slice. It seems safest to just keep it that way.

[randall77]

Agreed. If you really want the alias behavior, you can use Set, and if you really want the copy behavior, you can use Copy.

var a *[4]byte = ...
var b *[4]byte = ...

// copy b to a
va := reflect.ValueOf(a)
vb := reflect.ValueOf(b)
reflect.Copy(va.Elem(), vb.Elem())

// alias b to a
va := reflect.ValueOf(a)
vb := reflect.ValueOf(&b).Elem()
vb.Set(va)

Unlike with []byte, array pointers can be put in interfaces (and reflect.Values) without allocation, so having a separate Set function is less useful.

[dsnet]

@randall77 Good point about subtly different behavior for []byte with regard to SetBytes.

It feels weird that Bytes and SetBytes are asymmetrical, but maybe that's a necessity.

[rsc]

Indeed - reading and writing are asymmetrical.

[rsc]

Based on the discussion above, this proposal seems like a likely accept. — rsc for the proposal review group

[rsc]

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/408874 mentions this issue: doc: add release note for reflect.Value.{Bytes,Len,Cap}