@dr2chase

the compiler should able to prove that a string header passed into the variadic interfaces neither escaped nor is modified.

The escape analysis does not track whether things are modified. So it cannot make an interface with its data field directly pointing to s.

That said, it is possible to have it point to a copy of s on stack. However, currently the escape analysis doesn't distinguish the interface and its underlying value, i.e. x[0].(string) escaping is seen as x[0] escaping, which causes the string header allocated on heap.

If the caller knew that that the last use of the string was the function call itself, then it wouldn't even need to make a copy on the stack.

If the caller knew that that the last use of the string was the function call itself, then it wouldn't even need to make a copy on the stack.

Once we fix the escape problem, this will probably not matter. s can be SSA'd. To make the interface it needs to store s to a temp string header on stack so its address can be taken. There will be only one string header on stack.

Isn't this issue a duplicate of #8618 ?

It looks this is a problem of inlining:

package main

import "testing"

//go:noinline
func NewA(x ...interface{}) string {
	if len(x) == 0 {
		return ""
	}
	return x[0].(string)
}

//go:noinline
func NewB(x []interface{}) string {
	if len(x) == 0 {
		return ""
	}
	return x[0].(string)
}

//go:noinline
func NewC(x ...interface{}) string {
	return NewB(x)
}

var sink string

func BenchmarkA(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s = "hello"
		sink = NewA(s)
	}
}

func BenchmarkB(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s = "hello"
		sink = NewB([]interface{}{s})
	}
}

func BenchmarkC(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s = "hello"
		sink = NewC(s)
	}
}

Output:

BenchmarkA-4   	278548590	         4.154 ns/op	       0 B/op	       0 allocs/op
BenchmarkB-4   	280900167	         4.201 ns/op	       0 B/op	       0 allocs/op
BenchmarkC-4   	211030160	         5.666 ns/op	       0 B/op	       0 allocs/op

After removing the //go:noinline directive lines, the output becomes into:

BenchmarkA-4   	26151573	       101.1 ns/op	      16 B/op	       1 allocs/op
BenchmarkB-4   	418234288	         2.631 ns/op	       0 B/op	       0 allocs/op
BenchmarkC-4   	27927691	       118.6 ns/op	      16 B/op	       1 allocs/op

And this is not a string specified problem:

package main

import "testing"

func NewA(x ...interface{}) int {
	if len(x) == 0 {
		return 0
	}
	return x[0].(int)
}

func NewB(x []interface{}) int {
	if len(x) == 0 {
		return 0
	}
	return x[0].(int)
}

func NewC(x ...interface{}) int {
	return NewB(x)
}

var sink int

func BenchmarkA(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewA(s)
	}
}

func BenchmarkB(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewB([]interface{}{s})
	}
}

func BenchmarkC(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewC(s)
	}
}

The output:

BenchmarkA-4   	60167695	        49.68 ns/op	       8 B/op	       1 allocs/op
BenchmarkB-4   	629795134	         1.882 ns/op	       0 B/op	       0 allocs/op
BenchmarkC-4   	51212834	        43.10 ns/op	       8 B/op	       1 allocs/op

The smallest case:

package main

import "testing"

func NewA(x ...interface{}) int {
	return 0
}

var sink int

func BenchmarkA(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewA(s)
	}
}

func BenchmarkB(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		var s int = 99999
		sink = NewA([]interface{}{s}...)
	}
}

It looks the code inlining misleads the escape analysis, so that the arguments in the auto-constructed slice parameter are all allocated on heap.

{update]: if the value 99999 is changed to a value between [0, 255], then there will be no heap allocations.

if the value 99999 is changed to a value between [0, 255]

It seems a optimization introduced by CL 216401.

It looks this has been fixed on tip.

@dsnet this can be closed, right?

Correct. Confirmed that it is fixed.