reflect: make Value.MapRange inlineable

This allows the caller to decide whether MapIter should be
stack allocated or heap allocated based on whether it escapes.
In most cases, it does not escape and thus removes the utility
of MapIter.Reset (#46293). In fact, use of sync.Pool with MapIter
and calling MapIter.Reset is likely to be slower.

Change-Id: Ic93e7d39e5dd4c83e7fca9e0bdfbbcd70777f0e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/400675
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/test/inl_test.go b/src/cmd/compile/internal/test/inl_test.go
index b10d37a17cfd0d242775b60ece9eaed8b30bfbb0..211068e1dc3511863a7793514e237954d888ddcf 100644 (file)
--- a/src/cmd/compile/internal/test/inl_test.go
+++ b/src/cmd/compile/internal/test/inl_test.go
@@ -136,6 +136,7 @@ func TestIntendedInlining(t *testing.T) {
                        "Value.CanSet",
                        "Value.CanInterface",
                        "Value.IsValid",
+                       "Value.MapRange",
                        "Value.pointer",
                        "add",
                        "align",

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 5a35d98b51f01c86993b4e46ce2a482f6ca9a122..f7adf2fa1a6db2cda25fe28f56ff6432bf8934c8 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -370,9 +370,11 @@ func TestMapIterSet(t *testing.T) {
                        e.SetIterValue(iter)
                }
        }))
-       // Making a *MapIter allocates. This should be the only allocation.
-       if got != 1 {
-               t.Errorf("wanted 1 alloc, got %d", got)
+       // Calling MapRange should not allocate even though it returns a *MapIter.
+       // The function is inlineable, so if the local usage does not escape
+       // the *MapIter, it can remain stack allocated.
+       if got != 0 {
+               t.Errorf("wanted 0 alloc, got %d", got)
        }
 }
 

diff --git a/src/reflect/value.go b/src/reflect/value.go
index 06f0469edefc96ae88f1fa0d84cf29654210e21e..6fe3cee01717f07d80dea9f7622b7c2a0c87d078 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -1833,10 +1833,20 @@ func (iter *MapIter) Reset(v Value) {
 //             ...
 //     }
 func (v Value) MapRange() *MapIter {
-       v.mustBe(Map)
+       // This is inlinable to take advantage of "function outlining".
+       // The allocation of MapIter can be stack allocated if the caller
+       // does not allow it to escape.
+       // See https://blog.filippo.io/efficient-go-apis-with-the-inliner/
+       if v.kind() != Map {
+               v.panicNotMap()
+       }
        return &MapIter{m: v}
 }
 
+func (f flag) panicNotMap() {
+       f.mustBe(Map)
+}
+
 // copyVal returns a Value containing the map key or value at ptr,
 // allocating a new variable as needed.
 func copyVal(typ *rtype, fl flag, ptr unsafe.Pointer) Value {