From 67f8a8131648346f6bf9b525cd989bd2f7293b3f Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Mon, 22 Dec 2014 22:31:55 +0300 Subject: [PATCH] reflect: cache call frames Call frame allocations can account for significant portion of all allocations in a program, if call is executed in an inner loop (e.g. to process every line in a log). On the other hand, the allocation is easy to remove using sync.Pool since the allocation is strictly scoped. benchmark old ns/op new ns/op delta BenchmarkCall 634 338 -46.69% BenchmarkCall-4 496 167 -66.33% benchmark old allocs new allocs delta BenchmarkCall 1 0 -100.00% BenchmarkCall-4 1 0 -100.00% Update #7818 Change-Id: Icf60cce0a9be82e6171f0c0bd80dee2393db54a7 Reviewed-on: https://go-review.googlesource.com/1954 Reviewed-by: Keith Randall --- src/reflect/all_test.go | 11 ++++++++ src/reflect/export_test.go | 4 +-- src/reflect/makefunc.go | 4 +-- src/reflect/type.go | 13 ++++++--- src/reflect/value.go | 55 +++++++++++++++++++++++++++----------- src/runtime/stubs.go | 5 ++++ 6 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go index 278848bc00..7d40f9a8b6 100644 --- a/src/reflect/all_test.go +++ b/src/reflect/all_test.go @@ -1506,6 +1506,17 @@ func TestCallWithStruct(t *testing.T) { } } +func BenchmarkCall(b *testing.B) { + fv := ValueOf(func(a, b string) {}) + b.ReportAllocs() + b.RunParallel(func(pb *testing.PB) { + args := []Value{ValueOf("a"), ValueOf("b")} + for pb.Next() { + fv.Call(args) + } + }) +} + func TestMakeFunc(t *testing.T) { f := dummy fv := MakeFunc(TypeOf(f), func(in []Value) []Value { return in }) diff --git a/src/reflect/export_test.go b/src/reflect/export_test.go index 82a8a10930..9f06324bf8 100644 --- a/src/reflect/export_test.go +++ b/src/reflect/export_test.go @@ -26,9 +26,9 @@ func FuncLayout(t Type, rcvr Type) (frametype Type, argSize, retOffset uintptr, var ft *rtype var s *bitVector if rcvr != nil { - ft, argSize, retOffset, s = funcLayout(t.(*rtype), rcvr.(*rtype)) + ft, argSize, retOffset, s, _ = funcLayout(t.(*rtype), rcvr.(*rtype)) } else { - ft, argSize, retOffset, s = funcLayout(t.(*rtype), nil) + ft, argSize, retOffset, s, _ = funcLayout(t.(*rtype), nil) } frametype = ft for i := uint32(0); i < s.n; i += 2 { diff --git a/src/reflect/makefunc.go b/src/reflect/makefunc.go index d89f7f6811..4471805248 100644 --- a/src/reflect/makefunc.go +++ b/src/reflect/makefunc.go @@ -56,7 +56,7 @@ func MakeFunc(typ Type, fn func(args []Value) (results []Value)) Value { code := **(**uintptr)(unsafe.Pointer(&dummy)) // makeFuncImpl contains a stack map for use by the runtime - _, _, _, stack := funcLayout(t, nil) + _, _, _, stack, _ := funcLayout(t, nil) impl := &makeFuncImpl{code: code, stack: stack, typ: ftyp, fn: fn} @@ -104,7 +104,7 @@ func makeMethodValue(op string, v Value) Value { code := **(**uintptr)(unsafe.Pointer(&dummy)) // methodValue contains a stack map for use by the runtime - _, _, _, stack := funcLayout(funcType, nil) + _, _, _, stack, _ := funcLayout(funcType, nil) fv := &methodValue{ fn: code, diff --git a/src/reflect/type.go b/src/reflect/type.go index 040b9c06ec..ae7d165a68 100644 --- a/src/reflect/type.go +++ b/src/reflect/type.go @@ -1809,6 +1809,7 @@ type layoutType struct { argSize uintptr // size of arguments retOffset uintptr // offset of return values. stack *bitVector + framePool *sync.Pool } var layoutCache struct { @@ -1822,7 +1823,7 @@ var layoutCache struct { // The returned type exists only for GC, so we only fill out GC relevant info. // Currently, that's just size and the GC program. We also fill in // the name for possible debugging use. -func funcLayout(t *rtype, rcvr *rtype) (frametype *rtype, argSize, retOffset uintptr, stack *bitVector) { +func funcLayout(t *rtype, rcvr *rtype) (frametype *rtype, argSize, retOffset uintptr, stack *bitVector, framePool *sync.Pool) { if t.Kind() != Func { panic("reflect: funcLayout of non-func type") } @@ -1833,13 +1834,13 @@ func funcLayout(t *rtype, rcvr *rtype) (frametype *rtype, argSize, retOffset uin layoutCache.RLock() if x := layoutCache.m[k]; x.t != nil { layoutCache.RUnlock() - return x.t, x.argSize, x.retOffset, x.stack + return x.t, x.argSize, x.retOffset, x.stack, x.framePool } layoutCache.RUnlock() layoutCache.Lock() if x := layoutCache.m[k]; x.t != nil { layoutCache.Unlock() - return x.t, x.argSize, x.retOffset, x.stack + return x.t, x.argSize, x.retOffset, x.stack, x.framePool } tt := (*funcType)(unsafe.Pointer(t)) @@ -1903,14 +1904,18 @@ func funcLayout(t *rtype, rcvr *rtype) (frametype *rtype, argSize, retOffset uin if layoutCache.m == nil { layoutCache.m = make(map[layoutKey]layoutType) } + framePool = &sync.Pool{New: func() interface{} { + return unsafe_New(x) + }} layoutCache.m[k] = layoutType{ t: x, argSize: argSize, retOffset: retOffset, stack: stack, + framePool: framePool, } layoutCache.Unlock() - return x, argSize, retOffset, stack + return x, argSize, retOffset, stack, framePool } // ifaceIndir reports whether t is stored indirectly in an interface value. diff --git a/src/reflect/value.go b/src/reflect/value.go index 3255a697d5..4060206eac 100644 --- a/src/reflect/value.go +++ b/src/reflect/value.go @@ -393,9 +393,18 @@ func (v Value) call(op string, in []Value) []Value { } nout := t.NumOut() - // Compute frame type, allocate a chunk of memory for frame - frametype, _, retOffset, _ := funcLayout(t, rcvrtype) - args := unsafe_New(frametype) + // Compute frame type. + frametype, _, retOffset, _, framePool := funcLayout(t, rcvrtype) + + // Allocate a chunk of memory for frame. + var args unsafe.Pointer + if nout == 0 { + args = framePool.Get().(unsafe.Pointer) + } else { + // Can't use pool if the function has return values. + // We will leak pointer to args in ret, so its lifetime is not scoped. + args = unsafe_New(frametype) + } off := uintptr(0) // Copy inputs into args. @@ -427,16 +436,26 @@ func (v Value) call(op string, in []Value) []Value { runtime.GC() } - // Copy return values out of args. - ret := make([]Value, nout) - off = retOffset - for i := 0; i < nout; i++ { - tv := t.Out(i) - a := uintptr(tv.Align()) - off = (off + a - 1) &^ (a - 1) - fl := flagIndir | flag(tv.Kind()) - ret[i] = Value{tv.common(), unsafe.Pointer(uintptr(args) + off), fl} - off += tv.Size() + var ret []Value + if nout == 0 { + memclr(args, frametype.size) + framePool.Put(args) + } else { + // Zero the now unused input area of args, + // because the Values returned by this function contain pointers to the args object, + // and will thus keep the args object alive indefinitely. + memclr(args, retOffset) + // Copy return values out of args. + ret = make([]Value, nout) + off = retOffset + for i := 0; i < nout; i++ { + tv := t.Out(i) + a := uintptr(tv.Align()) + off = (off + a - 1) &^ (a - 1) + fl := flagIndir | flag(tv.Kind()) + ret[i] = Value{tv.common(), unsafe.Pointer(uintptr(args) + off), fl} + off += tv.Size() + } } return ret @@ -596,10 +615,10 @@ func align(x, n uintptr) uintptr { func callMethod(ctxt *methodValue, frame unsafe.Pointer) { rcvr := ctxt.rcvr rcvrtype, t, fn := methodReceiver("call", rcvr, ctxt.method) - frametype, argSize, retOffset, _ := funcLayout(t, rcvrtype) + frametype, argSize, retOffset, _, framePool := funcLayout(t, rcvrtype) // Make a new frame that is one word bigger so we can store the receiver. - args := unsafe_New(frametype) + args := framePool.Get().(unsafe.Pointer) // Copy in receiver and rest of args. storeRcvr(rcvr, args) @@ -622,6 +641,9 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) { unsafe.Pointer(uintptr(args)+retOffset), retOffset, frametype.size-retOffset) + + memclr(args, frametype.size) + framePool.Put(args) } // funcName returns the name of f, for use in error messages. @@ -2448,6 +2470,9 @@ func typedmemmovepartial(t *rtype, dst, src unsafe.Pointer, off, size uintptr) //go:noescape func typedslicecopy(elemType *rtype, dst, src sliceHeader) int +//go:noescape +func memclr(ptr unsafe.Pointer, n uintptr) + // Dummy annotation marking that the value x escapes, // for use in cases where the reflect code is so clever that // the compiler cannot follow. diff --git a/src/runtime/stubs.go b/src/runtime/stubs.go index d198f02e60..9aa83ef587 100644 --- a/src/runtime/stubs.go +++ b/src/runtime/stubs.go @@ -61,6 +61,11 @@ func badsystemstack() { //go:noescape func memclr(ptr unsafe.Pointer, n uintptr) +//go:linkname reflect_memclr reflect.memclr +func reflect_memclr(ptr unsafe.Pointer, n uintptr) { + memclr(ptr, n) +} + // memmove copies n bytes from "from" to "to". // in memmove_*.s //go:noescape