reflect: allow stack allocation of underlying value for reflect.Value.Interface

[thepudds]

Go version

tip

What did you do?

Run this simple test (playground link):

func TestAllocsReflectInterface(t *testing.T) {
	allocs := testing.AllocsPerRun(100, func() {
		v := reflect.ValueOf(Point{x: 1, y: 1})
		_ = v.Interface() // Causes the Point value to escape to heap.
	})
	if allocs != 0 {
		t.Errorf("allocs = %d, want 0", int(allocs))
	}
}

What did you see happen?

    prog_test.go:18: allocs = 1, want 0
--- FAIL: TestAllocsReflectInterface (0.00s)

What did you expect to see?

Ideally, zero heap allocations in this test, and ideally reflect.Value.Interface would allow the underlying value to not always be heap allocated. This affects the fmt package and can affect other users of reflect like serialization libraries. This issue is a spin out from #8618 (comment).

There are currently two reasons the underlying value escapes to the heap:

1. reflect.packEface causes reflect.Value.Interface to escape its underlying value:

 parameter v leaks to <heap> for packEface with derefs=0:
   flow: <heap> ← v:
     from v.ptr (dot) at .\value.go:145:13
     from e.word = v.ptr (assign) at ./value.go:145:10

2. Within reflect.Value.Interface and its helpers, method values cause conditional allocation of a methodValue struct, which also causes underlying values of other types to also escape:

 parameter v leaks to <heap> for valueInterface with derefs=0:
   flow: {heap} ← v:
     from makeMethodValue("Interface", v) (call parameter) at ./value.go:1492:22

I sent https://go.dev/cl/528535 to hopefully address the first reason in reflect.packEface.

For addressing the second reason above and handling method values, I sent:

• https://go.dev/cl/530095, which changes the runtime so that it manually creates the stack map
  for reflect.methodValueCall so that a stack-based methodValue can be tracked, including for example to properly update pointers during a stack copy operation.
• https://go.dev/cl/530097, which updates reflect.Value.Interface to stack allocate a methodValue if needed and pass a pointer to it down the stack. This is done while side-stepping other reasons the *methodValue might escape, while keeping reflect.Value.Interface within the inlining budget so that a caller of reflect.Value.Interface can stack allocate a methodValue (which then ends up avoiding the need to always store all underlying values in the heap, regardless of whether or not they are a method value).
• https://go.dev/cl/530096, which updates the compiler's escape analysis to recognize more self-assignment patterns. (This was originally used to help squeak past the inlining budget without escaping, but 530096 might end up not being needed for this issue).

Together, these changes allow the test to pass without heap allocations.

---

I am hopefully returning to this shortly, and filing this issue to re-summarize the approach and to help with tracking & discussion. (Previously, a short overview was in #8618 (comment), in addition to more details in the individual CLs).

[gabyhelp]

Related Issues

• reflect: possible saved allocation in Value.Interface #4949 (closed)
• cmd/compile: avoid always escaping the receivers in interface method calls #62653

Related Code Changes

• reflect: avoid always escaping in Value.Interface
• reflect: build interfaces without leak to heap in Value.Interface
• reflect: leak packEface input to result rather than heap
• fmt: sidestep reflect.Value.Interface to help escape analysis
• runtime: manually represent reflect.methodValue in stack map for methodValueCall assembly
• reflect: ensure correct scanning of return values
• cmd/compile/internal/escape: analyze receivers in interface method calls
• reflect: fix out-of-bounds pointers calling no-result method

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[dsnet]

While I support this optimization, I wonder if #62121 absolves some of necessity for this?

For most of my use-cases, I was directly type-asserting a Interface() call into a concrete type.

[gopherbot]

Change https://go.dev/cl/528535 mentions this issue: reflect: leak packEface input to result rather than heap